rust/kernel/serdev.rs | 6 +++--- samples/rust/rust_driver_serdev.rs | 13 +------------ 2 files changed, 4 insertions(+), 15 deletions(-)
There are currently 2 race conditions:
- in probe if `Driver::probe` returns Err
- in unbind
. In those cases the driver data will be set to NULL before the serdev
device was closed. If data is received while the driver data is dropped,
the `receive_buf_callback` might try to access the `active` mutex on a
null pointer. The race conditions can only occur if `Driver::receive` is
implemented.
The issue cannot be cleanly fixed without rewriting some logic, which
might introduce new regressions.
Temporarily disable the use of `Driver::receive`, until fixed in the
next release.
Fixes: 99f59aa82341 ("rust: add basic serial device bus abstractions")
Reported-by: Sashiko Bot <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/linux-serial/20260905000836.C8FC91F00A3D@smtp.kernel.org/
Closes: https://lore.kernel.org/linux-serial/20260903222159.70A911F000E9@smtp.kernel.org/
Signed-off-by: Markus Probst <markus.probst@posteo.de>
---
I will submit a patch (for the next merge cycle) soon, which will
address this issue and make the probe and unbind code less convoluted.
---
Changes in v4:
- remove comments
- Link to v3: https://patch.msgid.link/20260905-rust_serdev_fix-v3-1-b86056c3f7a4@posteo.de
Changes in v3:
- mitigate it
- Link to v2: https://patch.msgid.link/20260905-rust_serdev_fix-v2-0-35dfcd06ef2e@posteo.de
Changes in v2:
- also fix race condition on unbind
- Link to v1: https://patch.msgid.link/20260905-rust_serdev_fix-v1-1-2ea92b154a6b@posteo.de
---
rust/kernel/serdev.rs | 6 +++---
samples/rust/rust_driver_serdev.rs | 13 +------------
2 files changed, 4 insertions(+), 15 deletions(-)
diff --git a/rust/kernel/serdev.rs b/rust/kernel/serdev.rs
index 17ca504b7f8d..f4c0f43b5273 100644
--- a/rust/kernel/serdev.rs
+++ b/rust/kernel/serdev.rs
@@ -148,9 +148,8 @@ fn drop(self: Pin<&mut Self>) {
impl<T: Driver> Adapter<T> {
const OPS: &'static bindings::serdev_device_ops = &bindings::serdev_device_ops {
- receive_buf: if T::HAS_RECEIVE {
- Some(Self::receive_buf_callback)
- } else {
+ receive_buf: {
+ const_assert!(!T::HAS_RECEIVE);
None
},
write_wakeup: Some(bindings::serdev_device_write_wakeup),
@@ -233,6 +232,7 @@ extern "C" fn remove_callback(sdev: *mut bindings::serdev_device) {
T::unbind(sdev, data_pinned);
}
+ #[expect(dead_code)]
extern "C" fn receive_buf_callback(
sdev: *mut bindings::serdev_device,
buf: *const u8,
diff --git a/samples/rust/rust_driver_serdev.rs b/samples/rust/rust_driver_serdev.rs
index 51b4898cd855..3fd3eef15371 100644
--- a/samples/rust/rust_driver_serdev.rs
+++ b/samples/rust/rust_driver_serdev.rs
@@ -4,10 +4,7 @@
use kernel::{
acpi,
- device::{
- Bound,
- Core, //
- },
+ device::Core,
of,
prelude::*,
serdev,
@@ -60,14 +57,6 @@ fn probe<'bound>(
Ok(Self { sdev: sdev.into() })
}
-
- fn receive<'bound>(
- sdev: &'bound serdev::Device<Bound>,
- _this: Pin<&Self>,
- data: &[u8],
- ) -> usize {
- sdev.write(data).unwrap_or_default() as usize
- }
}
impl Drop for SampleDriver {
---
base-commit: e5e04726cdd043e309677071ab1b65a4b18f422b
change-id: 20260904-rust_serdev_fix-be3ff9c8a5e8On Sat Sep 5, 2026 at 7:47 PM BST, Markus Probst wrote:
> There are currently 2 race conditions:
> - in probe if `Driver::probe` returns Err
> - in unbind
> . In those cases the driver data will be set to NULL before the serdev
> device was closed. If data is received while the driver data is dropped,
> the `receive_buf_callback` might try to access the `active` mutex on a
> null pointer. The race conditions can only occur if `Driver::receive` is
> implemented.
>
> The issue cannot be cleanly fixed without rewriting some logic, which
> might introduce new regressions.
>
> Temporarily disable the use of `Driver::receive`, until fixed in the
> next release.
Isn't this basically undoing all the usefulness of having the serdev abstraction
in the first place?
As this is for unbound only, I don't think we need to rush to fix this for rc
anyway. Many C drivers don't get unbind correct (and our misc device). There's
no in-tree user that will hit this API anyway.
I think we'd better just find a proper fix for the next cycle and leave the
current code as is.
Best,
Gary
>
> Fixes: 99f59aa82341 ("rust: add basic serial device bus abstractions")
> Reported-by: Sashiko Bot <sashiko-bot@kernel.org>
> Closes: https://lore.kernel.org/linux-serial/20260905000836.C8FC91F00A3D@smtp.kernel.org/
> Closes: https://lore.kernel.org/linux-serial/20260903222159.70A911F000E9@smtp.kernel.org/
> Signed-off-by: Markus Probst <markus.probst@posteo.de>
> ---
> I will submit a patch (for the next merge cycle) soon, which will
> address this issue and make the probe and unbind code less convoluted.
> ---
> Changes in v4:
> - remove comments
> - Link to v3: https://patch.msgid.link/20260905-rust_serdev_fix-v3-1-b86056c3f7a4@posteo.de
>
> Changes in v3:
> - mitigate it
> - Link to v2: https://patch.msgid.link/20260905-rust_serdev_fix-v2-0-35dfcd06ef2e@posteo.de
>
> Changes in v2:
> - also fix race condition on unbind
> - Link to v1: https://patch.msgid.link/20260905-rust_serdev_fix-v1-1-2ea92b154a6b@posteo.de
> ---
> rust/kernel/serdev.rs | 6 +++---
> samples/rust/rust_driver_serdev.rs | 13 +------------
> 2 files changed, 4 insertions(+), 15 deletions(-)
>
> diff --git a/rust/kernel/serdev.rs b/rust/kernel/serdev.rs
> index 17ca504b7f8d..f4c0f43b5273 100644
> --- a/rust/kernel/serdev.rs
> +++ b/rust/kernel/serdev.rs
> @@ -148,9 +148,8 @@ fn drop(self: Pin<&mut Self>) {
>
> impl<T: Driver> Adapter<T> {
> const OPS: &'static bindings::serdev_device_ops = &bindings::serdev_device_ops {
> - receive_buf: if T::HAS_RECEIVE {
> - Some(Self::receive_buf_callback)
> - } else {
> + receive_buf: {
> + const_assert!(!T::HAS_RECEIVE);
> None
> },
> write_wakeup: Some(bindings::serdev_device_write_wakeup),
> @@ -233,6 +232,7 @@ extern "C" fn remove_callback(sdev: *mut bindings::serdev_device) {
> T::unbind(sdev, data_pinned);
> }
>
> + #[expect(dead_code)]
> extern "C" fn receive_buf_callback(
> sdev: *mut bindings::serdev_device,
> buf: *const u8,
> diff --git a/samples/rust/rust_driver_serdev.rs b/samples/rust/rust_driver_serdev.rs
> index 51b4898cd855..3fd3eef15371 100644
> --- a/samples/rust/rust_driver_serdev.rs
> +++ b/samples/rust/rust_driver_serdev.rs
> @@ -4,10 +4,7 @@
>
> use kernel::{
> acpi,
> - device::{
> - Bound,
> - Core, //
> - },
> + device::Core,
> of,
> prelude::*,
> serdev,
> @@ -60,14 +57,6 @@ fn probe<'bound>(
>
> Ok(Self { sdev: sdev.into() })
> }
> -
> - fn receive<'bound>(
> - sdev: &'bound serdev::Device<Bound>,
> - _this: Pin<&Self>,
> - data: &[u8],
> - ) -> usize {
> - sdev.write(data).unwrap_or_default() as usize
> - }
> }
>
> impl Drop for SampleDriver {
>
> ---
> base-commit: e5e04726cdd043e309677071ab1b65a4b18f422b
> change-id: 20260904-rust_serdev_fix-be3ff9c8a5e8
On Sun, 2026-09-06 at 00:39 +0100, Gary Guo wrote:
> On Sat Sep 5, 2026 at 7:47 PM BST, Markus Probst wrote:
> > There are currently 2 race conditions:
> > - in probe if `Driver::probe` returns Err
> > - in unbind
> > . In those cases the driver data will be set to NULL before the serdev
> > device was closed. If data is received while the driver data is dropped,
> > the `receive_buf_callback` might try to access the `active` mutex on a
> > null pointer. The race conditions can only occur if `Driver::receive` is
> > implemented.
> >
> > The issue cannot be cleanly fixed without rewriting some logic, which
> > might introduce new regressions.
> >
> > Temporarily disable the use of `Driver::receive`, until fixed in the
> > next release.
>
> Isn't this basically undoing all the usefulness of having the serdev abstraction
> in the first place?
It removes 50% of its functionality, so kinda. The initial
"synology_microp" driver [1] in the mailing list for instance currently
doesn't rely on it (it will need it later though).
>
> As this is for unbound only, I don't think we need to rush to fix this for rc
> anyway. Many C drivers don't get unbind correct (and our misc device). There's
> no in-tree user that will hit this API anyway.
Also on probe failure (i.e. Driver returns an error on probe).
>
> I think we'd better just find a proper fix for the next cycle and leave the
> current code as is.
Thats also an option I wouldn't mind.
The proper fix is almost ready.
Thanks
- Markus Probst
[1]
https://lore.kernel.org/rust-for-linux/20260724-synology_microp_initial-v18-0-fb2f49f10e77@posteo.de/
>
> Best,
> Gary
>
> >
> > Fixes: 99f59aa82341 ("rust: add basic serial device bus abstractions")
> > Reported-by: Sashiko Bot <sashiko-bot@kernel.org>
> > Closes: https://lore.kernel.org/linux-serial/20260905000836.C8FC91F00A3D@smtp.kernel.org/
> > Closes: https://lore.kernel.org/linux-serial/20260903222159.70A911F000E9@smtp.kernel.org/
> > Signed-off-by: Markus Probst <markus.probst@posteo.de>
> > ---
> > I will submit a patch (for the next merge cycle) soon, which will
> > address this issue and make the probe and unbind code less convoluted.
> > ---
> > Changes in v4:
> > - remove comments
> > - Link to v3: https://patch.msgid.link/20260905-rust_serdev_fix-v3-1-b86056c3f7a4@posteo.de
> >
> > Changes in v3:
> > - mitigate it
> > - Link to v2: https://patch.msgid.link/20260905-rust_serdev_fix-v2-0-35dfcd06ef2e@posteo.de
> >
> > Changes in v2:
> > - also fix race condition on unbind
> > - Link to v1: https://patch.msgid.link/20260905-rust_serdev_fix-v1-1-2ea92b154a6b@posteo.de
> > ---
> > rust/kernel/serdev.rs | 6 +++---
> > samples/rust/rust_driver_serdev.rs | 13 +------------
> > 2 files changed, 4 insertions(+), 15 deletions(-)
> >
> > diff --git a/rust/kernel/serdev.rs b/rust/kernel/serdev.rs
> > index 17ca504b7f8d..f4c0f43b5273 100644
> > --- a/rust/kernel/serdev.rs
> > +++ b/rust/kernel/serdev.rs
> > @@ -148,9 +148,8 @@ fn drop(self: Pin<&mut Self>) {
> >
> > impl<T: Driver> Adapter<T> {
> > const OPS: &'static bindings::serdev_device_ops = &bindings::serdev_device_ops {
> > - receive_buf: if T::HAS_RECEIVE {
> > - Some(Self::receive_buf_callback)
> > - } else {
> > + receive_buf: {
> > + const_assert!(!T::HAS_RECEIVE);
> > None
> > },
> > write_wakeup: Some(bindings::serdev_device_write_wakeup),
> > @@ -233,6 +232,7 @@ extern "C" fn remove_callback(sdev: *mut bindings::serdev_device) {
> > T::unbind(sdev, data_pinned);
> > }
> >
> > + #[expect(dead_code)]
> > extern "C" fn receive_buf_callback(
> > sdev: *mut bindings::serdev_device,
> > buf: *const u8,
> > diff --git a/samples/rust/rust_driver_serdev.rs b/samples/rust/rust_driver_serdev.rs
> > index 51b4898cd855..3fd3eef15371 100644
> > --- a/samples/rust/rust_driver_serdev.rs
> > +++ b/samples/rust/rust_driver_serdev.rs
> > @@ -4,10 +4,7 @@
> >
> > use kernel::{
> > acpi,
> > - device::{
> > - Bound,
> > - Core, //
> > - },
> > + device::Core,
> > of,
> > prelude::*,
> > serdev,
> > @@ -60,14 +57,6 @@ fn probe<'bound>(
> >
> > Ok(Self { sdev: sdev.into() })
> > }
> > -
> > - fn receive<'bound>(
> > - sdev: &'bound serdev::Device<Bound>,
> > - _this: Pin<&Self>,
> > - data: &[u8],
> > - ) -> usize {
> > - sdev.write(data).unwrap_or_default() as usize
> > - }
> > }
> >
> > impl Drop for SampleDriver {
> >
> > ---
> > base-commit: e5e04726cdd043e309677071ab1b65a4b18f422b
> > change-id: 20260904-rust_serdev_fix-be3ff9c8a5e8
>
© 2016 - 2026 Red Hat, Inc.