rust/kernel/serdev.rs | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-)
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.
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/
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-be3ff9c8a5e8On 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
>
> 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/
> 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
On Sat Sep 5, 2026 at 1:22 AM BST, Markus Probst wrote: > 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). What would go wrong if the device is closed before destroying the driver data? Best, Gary
On Sat, 2026-09-05 at 15:24 +0100, Gary Guo wrote: > On Sat Sep 5, 2026 at 1:22 AM BST, Markus Probst wrote: > > 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). > > What would go wrong if the device is closed before destroying the driver data? The driver would still have a handle to the Device and could call `Device::set_baudrate` for instance. This would be a use-after-free by itself. I am currently investigating, if I can only stop rx and leave the device for transmission open. There seems to be the `CREAD` c_cflag I can remove from the tty. But I am not sure yet if it guarantees that receive_buf isn't called anymore. Thanks - Markus Probst > > Best, > Gary
© 2016 - 2026 Red Hat, Inc.