Re: [PATCH] rust: serdev: Fix race condition on driver probe fail
flat view
From: Markus Probst <markus.probst@posteo.de>
Date: 2026-09-05 00:22:15
Also in:
lkml, rust-for-linux
On Fri, 2026-09-04 at 23:54 +0000, Markus Probst wrote:
If `Driver::probe` fails, the pointer to the driver data (`PrivateData`) will first be set to NULL by `drvdata_obtain` and only after that the serdev device will be closed by Drop. Thus there is a small window in which the serdev device is still open, but the pointer to the driver data is NULL. Therefore it is possible that `receive_buf_callback` might try to access the `active` mutex on a null pointer.
It seems, Sashiko found the same issue in a different unrelated place too (while reviewing this patch): https://sashiko.dev/#/patchset/20260905-rust_serdev_fix-v1-1-2ea92b154a6b%40posteo.de Unfortunately, I cannot fix this one so easily, because the serdev device needs to be open for the entire lifetime of the "real" driver data (Driver::Data). Thanks - Markus Probst
quoted hunk ↗ jump to hunk
Add a separate `ScopeGuard` that will close the device before the driver data will be set to NULL on probe failure. Fixes: 99f59aa82341 ("rust: add basic serial device bus abstractions") Reported-by: Sashiko Bot <sashiko-bot@kernel.org> Closes: https://lore.kernel.org/linux-serial/20260903222159.70A911F000E9@smtp.kernel.org/ (local) Signed-off-by: Markus Probst <markus.probst@posteo.de> --- Unfortunately, this also makes the code in probe even more convoluted than it already is. I will submit a patch (for the next merge cycle) soon, which will address this concern. --- rust/kernel/serdev.rs | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-)diff --git a/rust/kernel/serdev.rs b/rust/kernel/serdev.rs index 17ca504b7f8d..4f57f4c45329 100644 --- a/rust/kernel/serdev.rs +++ b/rust/kernel/serdev.rs@@ -190,6 +190,16 @@ extern "C" fn probe_callback(sdev: *mut bindings::serdev_device) -> kernel::ffi: // SAFETY: We have exclusive access to `private_data.open`. unsafe { *private_data.open.get() = true }; + let open_guard = ScopeGuard::new(|| { + // SAFETY: + // - `private_data.sdev.as_raw()` is guaranteed to be a pointer to a valid + // `struct serdev_device`. + // - We just opened the device, thus it is guaranteed to be open. + unsafe { bindings::serdev_device_close(private_data.sdev.as_raw()) }; + // SAFETY: We have exclusive access to `private_data.open`. + unsafe { *private_data.open.get() = false }; + }); + let data = T::probe(sdev, info); // SAFETY: We have exclusive access to `private_data.driver`.@@ -203,10 +213,12 @@ extern "C" fn probe_callback(sdev: *mut bindings::serdev_device) -> kernel::ffi: drop(active); - result.map(|()| { - private_data.dismiss(); - 0 - }) + result?; + + open_guard.dismiss(); + private_data.dismiss(); + + Ok(0) }) }--- base-commit: e5e04726cdd043e309677071ab1b65a4b18f422b change-id: 20260904-rust_serdev_fix-be3ff9c8a5e8
Attachments
- signature.asc [application/pgp-signature] 870 bytes