[PATCH v2 0/2] rust: serdev: Mitigate race conditions

[PATCH v2 0/2] rust: serdev: Mitigate race conditions

[Markus Probst]

For details, see the commit messages.

I will submit a patch (for the next merge cycle) soon, which will
make the probe and unbind code less convoluted. This will make bugs like
these more unlikely.

Signed-off-by: Markus Probst <markus.probst@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

---
Markus Probst (2):
      rust: serdev: Fix race condition on driver unbind
      rust: serdev: Fix race condition on driver probe

 rust/kernel/device.rs | 16 +++++++++-------
 rust/kernel/driver.rs |  2 +-
 rust/kernel/serdev.rs |  2 +-
 3 files changed, 11 insertions(+), 9 deletions(-)
---
base-commit: e5e04726cdd043e309677071ab1b65a4b18f422b
change-id: 20260904-rust_serdev_fix-be3ff9c8a5e8

[PATCH v2 1/2] rust: serdev: Fix race condition on driver unbind

[Markus Probst]

On device unbind, 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 function `drvdata_drop` that leaves the pointer to the driver data
valid until the Drop has completed. Use it in the post unbind callback.

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/
Signed-off-by: Markus Probst <markus.probst@posteo.de>
---
 rust/kernel/device.rs | 27 +++++++++++++++++++++++++++
 rust/kernel/driver.rs |  2 +-
 2 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
index 2291d85b6849..3886cc713c28 100644
--- a/rust/kernel/device.rs
+++ b/rust/kernel/device.rs
@@ -219,6 +219,7 @@ pub fn set_drvdata<T>(&self, data: impl PinInit<T, Error>) -> Result {
     ///
     /// - The type `T` must match the type of the `ForeignOwnable` previously stored by
     ///   [`Device::set_drvdata`].
+    /// - Must only be called before the device is fully unbound.
     pub(crate) unsafe fn drvdata_obtain<T>(&self) -> Option<Pin<KBox<T>>> {
         // SAFETY: By the type invariants, `self.as_raw()` is a valid pointer to a `struct device`.
         let ptr = unsafe { bindings::dev_get_drvdata(self.as_raw()) };
@@ -236,6 +237,32 @@ pub(crate) unsafe fn drvdata_obtain<T>(&self) -> Option<Pin<KBox<T>>> {
         //   in `into_foreign()`.
         Some(unsafe { Pin::<KBox<T>>::from_foreign(ptr.cast()) })
     }
+
+    /// Drop the private data stored in this [`Device`].
+    ///
+    /// The pointer to the private data remains valid until the drop is complete.
+    ///
+    /// # Safety
+    ///
+    /// - The type `T` must match the type of the `ForeignOwnable` previously stored by
+    ///   [`Device::set_drvdata`].
+    pub(crate) unsafe fn drvdata_drop<T>(&self) {
+        // SAFETY: By the type invariants, `self.as_raw()` is a valid pointer to a `struct device`.
+        let ptr = unsafe { bindings::dev_get_drvdata(self.as_raw()) };
+
+        if ptr.is_null() {
+            return;
+        }
+
+        // SAFETY:
+        // - If `ptr` is not NULL, it comes from a previous call to `into_foreign()`.
+        // - `dev_get_drvdata()` guarantees to return the same pointer given to `dev_set_drvdata()`
+        //   in `into_foreign()`.
+        drop(unsafe { Pin::<KBox<T>>::from_foreign(ptr.cast()) });
+
+        // SAFETY: By the type invariants, `self.as_raw()` is a valid pointer to a `struct device`.
+        unsafe { bindings::dev_set_drvdata(self.as_raw(), core::ptr::null_mut()) };
+    }
 }
 
 impl<Ctx: InternalBoundContext> Device<Ctx> {
diff --git a/rust/kernel/driver.rs b/rust/kernel/driver.rs
index c9c74c4dde8f..83410141ef1c 100644
--- a/rust/kernel/driver.rs
+++ b/rust/kernel/driver.rs
@@ -204,7 +204,7 @@ extern "C" fn post_unbind_callback(dev: *mut bindings::device) {
         //
         // SAFETY: By the safety requirements of the `Driver` trait, `T::DriverData` is the
         // driver's bus device private data type.
-        drop(unsafe { dev.drvdata_obtain::<T::DriverData<'_>>() });
+        unsafe { dev.drvdata_drop::<T::DriverData<'_>>() };
     }
 
     /// Attach generic `struct device_driver` callbacks.

-- 
2.55.0

[PATCH v2 2/2] rust: serdev: Fix race condition on driver probe

[Markus Probst]

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.

Use previously added `drvdata_drop` instead of `drvdata_obtain`, so the
serdev device will first be closed with Drop and after that the pointer to
the driver data will be set to NULL.

Remove `drvdata_obtain`, as it is now dead code.

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>
---
 rust/kernel/device.rs | 25 -------------------------
 rust/kernel/serdev.rs |  2 +-
 2 files changed, 1 insertion(+), 26 deletions(-)

diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
index 3886cc713c28..834fea0eb0a7 100644
--- a/rust/kernel/device.rs
+++ b/rust/kernel/device.rs
@@ -213,31 +213,6 @@ pub fn set_drvdata<T>(&self, data: impl PinInit<T, Error>) -> Result {
         Ok(())
     }
 
-    /// Take ownership of the private data stored in this [`Device`].
-    ///
-    /// # Safety
-    ///
-    /// - The type `T` must match the type of the `ForeignOwnable` previously stored by
-    ///   [`Device::set_drvdata`].
-    /// - Must only be called before the device is fully unbound.
-    pub(crate) unsafe fn drvdata_obtain<T>(&self) -> Option<Pin<KBox<T>>> {
-        // SAFETY: By the type invariants, `self.as_raw()` is a valid pointer to a `struct device`.
-        let ptr = unsafe { bindings::dev_get_drvdata(self.as_raw()) };
-
-        // SAFETY: By the type invariants, `self.as_raw()` is a valid pointer to a `struct device`.
-        unsafe { bindings::dev_set_drvdata(self.as_raw(), core::ptr::null_mut()) };
-
-        if ptr.is_null() {
-            return None;
-        }
-
-        // SAFETY:
-        // - If `ptr` is not NULL, it comes from a previous call to `into_foreign()`.
-        // - `dev_get_drvdata()` guarantees to return the same pointer given to `dev_set_drvdata()`
-        //   in `into_foreign()`.
-        Some(unsafe { Pin::<KBox<T>>::from_foreign(ptr.cast()) })
-    }
-
     /// Drop the private data stored in this [`Device`].
     ///
     /// The pointer to the private data remains valid until the drop is complete.
diff --git a/rust/kernel/serdev.rs b/rust/kernel/serdev.rs
index 17ca504b7f8d..a12b1dea12aa 100644
--- a/rust/kernel/serdev.rs
+++ b/rust/kernel/serdev.rs
@@ -176,7 +176,7 @@ extern "C" fn probe_callback(sdev: *mut bindings::serdev_device) -> kernel::ffi:
             let private_data = unsafe { sdev.as_ref().drvdata_borrow::<PrivateData<'_, T>>() };
             let private_data = ScopeGuard::new_with_data(private_data, |_| {
                 // SAFETY: We just set drvdata to `PrivateData<'_, T>`.
-                drop(unsafe { sdev.as_ref().drvdata_obtain::<PrivateData<'_, T>>() });
+                unsafe { sdev.as_ref().drvdata_drop::<PrivateData<'_, T>>() };
             });
             let mut active = private_data.active.lock();
 

-- 
2.55.0

Re: [PATCH v2 1/2] rust: serdev: Fix race condition on driver unbind

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The function `drvdata_drop` introduces a Use-After-Free window and mutable aliasing vulnerability by dropping the driver data before clearing `dev->driver_data`.
- [Low] The new helper function `drvdata_drop` is missing the `#[inline]` attribute.
--

commit 26ab6ef2b4c35bdc9d899d45004d0c1a7f3fca80
Author: Markus Probst <markus.probst@posteo.de>

rust: serdev: Fix race condition on driver unbind

This commit attempts to fix a race condition where the driver data pointer is
NULL while the serdev device is still open. It does this by introducing a new
function drvdata_drop() that leaves the pointer valid until the memory drop
is complete.

Closes: https://lore.kernel.org/linux-serial/20260905000836.C8FC91F00A3D@smtp.kernel.org/

> diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
> index 2291d85b6849e..3886cc713c28b 100644
> --- a/rust/kernel/device.rs
> +++ b/rust/kernel/device.rs

[ ... ]

> @@ -236,6 +237,32 @@ pub(crate) unsafe fn drvdata_obtain<T>(&self) -> Option<Pin<KBox<T>>> {
>          //   in `into_foreign()`.
>          Some(unsafe { Pin::<KBox<T>>::from_foreign(ptr.cast()) })
>      }
> +
> +    /// Drop the private data stored in this [`Device`].
> +    ///
> +    /// The pointer to the private data remains valid until the drop is complete.
> +    ///
> +    /// # Safety
> +    ///
> +    /// - The type `T` must match the type of the `ForeignOwnable` previously stored by
> +    ///   [`Device::set_drvdata`].
> +    pub(crate) unsafe fn drvdata_drop<T>(&self) {

[Severity: Low]
This isn't a bug, but should this abstraction function be annotated with
`#[inline]`?

The Rust subsystem coding guidelines mandate that small functions forwarding
to C bindings or performing small abstraction tasks should use `#[inline]`.

> +        // SAFETY: By the type invariants, `self.as_raw()` is a valid pointer to a `struct device`.
> +        let ptr = unsafe { bindings::dev_get_drvdata(self.as_raw()) };
> +
> +        if ptr.is_null() {
> +            return;
> +        }
> +
> +        // SAFETY:
> +        // - If `ptr` is not NULL, it comes from a previous call to `into_foreign()`.
> +        // - `dev_get_drvdata()` guarantees to return the same pointer given to `dev_set_drvdata()`
> +        //   in `into_foreign()`.
> +        drop(unsafe { Pin::<KBox<T>>::from_foreign(ptr.cast()) });
> +
> +        // SAFETY: By the type invariants, `self.as_raw()` is a valid pointer to a `struct device`.
> +        unsafe { bindings::dev_set_drvdata(self.as_raw(), core::ptr::null_mut()) };

[Severity: High]
Does this create a Use-After-Free window?

By dropping the driver data before clearing `dev->driver_data`, the pointer
remains valid while and after the memory is freed.

If a concurrent context, such as a hardware interrupt triggering
receive_buf_callback() in rust/kernel/serdev.rs, runs at this exact
moment, it will borrow a dangling pointer:

rust/kernel/serdev.rs:receive_buf_callback() {
    ...
    let private_data = unsafe { sdev.as_ref().drvdata_borrow::<PrivateData<'_, T>>() };
    ...
}

Could this lead to a memory corruption vulnerability during the unbind path?

> +    }
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260905-rust_serdev_fix-v2-0-35dfcd06ef2e@posteo.de?part=1

Re: [PATCH v2 1/2] rust: serdev: Fix race condition on driver unbind

[Markus Probst]

[-- Attachment #1: Type: text/plain, Size: 3784 bytes --]

On Sat, 2026-09-05 at 13:44 +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] The function `drvdata_drop` introduces a Use-After-Free window and mutable aliasing vulnerability by dropping the driver data before clearing `dev->driver_data`.
> - [Low] The new helper function `drvdata_drop` is missing the `#[inline]` attribute.
> --
> 
> commit 26ab6ef2b4c35bdc9d899d45004d0c1a7f3fca80
> Author: Markus Probst <markus.probst@posteo.de>
> 
> rust: serdev: Fix race condition on driver unbind
> 
> This commit attempts to fix a race condition where the driver data pointer is
> NULL while the serdev device is still open. It does this by introducing a new
> function drvdata_drop() that leaves the pointer valid until the memory drop
> is complete.
> 
> Closes: https://lore.kernel.org/linux-serial/20260905000836.C8FC91F00A3D@smtp.kernel.org/
> 
> > diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
> > index 2291d85b6849e..3886cc713c28b 100644
> > --- a/rust/kernel/device.rs
> > +++ b/rust/kernel/device.rs
> 
> [ ... ]
> 
> > @@ -236,6 +237,32 @@ pub(crate) unsafe fn drvdata_obtain<T>(&self) -> Option<Pin<KBox<T>>> {
> >          //   in `into_foreign()`.
> >          Some(unsafe { Pin::<KBox<T>>::from_foreign(ptr.cast()) })
> >      }
> > +
> > +    /// Drop the private data stored in this [`Device`].
> > +    ///
> > +    /// The pointer to the private data remains valid until the drop is complete.
> > +    ///
> > +    /// # Safety
> > +    ///
> > +    /// - The type `T` must match the type of the `ForeignOwnable` previously stored by
> > +    ///   [`Device::set_drvdata`].
> > +    pub(crate) unsafe fn drvdata_drop<T>(&self) {
> 
> [Severity: Low]
> This isn't a bug, but should this abstraction function be annotated with
> `#[inline]`?
> 
> The Rust subsystem coding guidelines mandate that small functions forwarding
> to C bindings or performing small abstraction tasks should use `#[inline]`.
> 
> > +        // SAFETY: By the type invariants, `self.as_raw()` is a valid pointer to a `struct device`.
> > +        let ptr = unsafe { bindings::dev_get_drvdata(self.as_raw()) };
> > +
> > +        if ptr.is_null() {
> > +            return;
> > +        }
> > +
> > +        // SAFETY:
> > +        // - If `ptr` is not NULL, it comes from a previous call to `into_foreign()`.
> > +        // - `dev_get_drvdata()` guarantees to return the same pointer given to `dev_set_drvdata()`
> > +        //   in `into_foreign()`.
> > +        drop(unsafe { Pin::<KBox<T>>::from_foreign(ptr.cast()) });
> > +
> > +        // SAFETY: By the type invariants, `self.as_raw()` is a valid pointer to a `struct device`.
> > +        unsafe { bindings::dev_set_drvdata(self.as_raw(), core::ptr::null_mut()) };

drvdata_obtain didn't have it.

> 
> [Severity: High]
> Does this create a Use-After-Free window?
> 
> By dropping the driver data before clearing `dev->driver_data`, the pointer
> remains valid while and after the memory is freed.
> 
> If a concurrent context, such as a hardware interrupt triggering
> receive_buf_callback() in rust/kernel/serdev.rs, runs at this exact
> moment, it will borrow a dangling pointer:
> 
> rust/kernel/serdev.rs:receive_buf_callback() {
>     ...
>     let private_data = unsafe { sdev.as_ref().drvdata_borrow::<PrivateData<'_, T>>() };
>     ...
> }
> 
> Could this lead to a memory corruption vulnerability during the unbind path?
Under a normal case, no one should access this pointer anymore. For
serdev, this isn't a normal case. Before it becomes a dangling pointer,
PinnedDrop ensures it won't be used afterwards.


> 
> > +    }
> >  }

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 870 bytes --]

Re: [PATCH v2 2/2] rust: serdev: Fix race condition on driver probe

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [Critical] Use-After-Free and Mutable Aliasing in `drvdata_drop` because memory is freed before clearing the driver data pointer.
- [High] `T::receive()` can run concurrently with `T::unbind()`, leading to a Use-After-Unbind race condition.
- [High] Circular deadlock between `probe_callback` and `receive_buf_callback`.
--

commit 67f5ed19e07ddf9c3ef694dbb2c1e962df051525
Author: Markus Probst <markus.probst@posteo.de>

rust: serdev: Fix race condition on driver probe

This commit fixes a race condition on driver probe failures by using
drvdata_drop() instead of drvdata_obtain(), ensuring the serdev device
is closed before the pointer to the driver data is set to null.

Closes: https://lore.kernel.org/linux-serial/20260903222159.70A911F000E9@smtp.kernel.org/

> diff --git a/rust/kernel/serdev.rs b/rust/kernel/serdev.rs
> index 17ca504b7f8d6..a12b1dea12aa1 100644
> --- a/rust/kernel/serdev.rs
> +++ b/rust/kernel/serdev.rs
> @@ -176,7 +176,7 @@ extern "C" fn probe_callback(sdev: *mut bindings::serdev_device) -> kernel::ffi:
>              let private_data = unsafe { sdev.as_ref().drvdata_borrow::<PrivateData<'_, T>>() };
>              let private_data = ScopeGuard::new_with_data(private_data, |_| {
>                  // SAFETY: We just set drvdata to `PrivateData<'_, T>`.
> -                drop(unsafe { sdev.as_ref().drvdata_obtain::<PrivateData<'_, T>>() });
> +                unsafe { sdev.as_ref().drvdata_drop::<PrivateData<'_, T>>() };

[Severity: Critical]
This is a pre-existing issue, but does calling drvdata_drop() here introduce
a use-after-free and mutable aliasing vulnerability?

Since drvdata_drop() executes drop() on the driver's private data and
deallocates the memory before calling dev_set_drvdata(..., null), the global
device pointer still references the freed memory.

If receive_buf_callback() is invoked concurrently by incoming serial data:

rust/kernel/serdev.rs:receive_buf_callback() {
    ...
    let private_data = unsafe { sdev.as_ref().drvdata_borrow::<PrivateData<'_, T>>() };
    ...
}

It can fetch the still-non-null dangling pointer before it is cleared, and
block on the active mutex. Once drop() completes, receive_buf_callback()
would wake up and operate on freed memory.

>              });
>              let mut active = private_data.active.lock();

[Severity: High]
This isn't a bug introduced by this patch, but can holding this active mutex
across the T::probe() call result in a circular deadlock?

In probe_callback(), the active mutex is locked before invoking T::probe().
If a driver writes a command to the device and synchronously waits for a
response during probe, the incoming response would trigger
receive_buf_callback():

rust/kernel/serdev.rs:receive_buf_callback() {
    ...
    let active = private_data.active.lock();
    ...
}

Since probe_callback() holds the lock and waits for the response, and
receive_buf_callback() receives the response but blocks indefinitely waiting
for the lock, does this structurally guarantee a deadlock for synchronous
probes?

[ ... ]

[Severity: High]
This is a pre-existing issue, but can T::receive() run concurrently with
T::unbind(), leading to a use-after-unbind race condition?

During device removal, remove_callback() calls T::unbind():

rust/kernel/serdev.rs:remove_callback() {
    ...
    T::unbind(sdev, data_pinned);
}

It does not lock the active mutex, set it to false, or close the serdev
device prior to unbinding.

Consequently, incoming serial data can trigger receive_buf_callback(), which
evaluates the active check as true (since active is not cleared during
unbind):

rust/kernel/serdev.rs:receive_buf_callback() {
    ...
    if !*active {
        return length;
    }
    ...
    T::receive(sdev, data_pinned, buf)
}

This forwards the data to T::receive(), which can execute concurrently with
or after T::unbind(), accessing driver resources that are actively being
destroyed.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260905-rust_serdev_fix-v2-0-35dfcd06ef2e@posteo.de?part=2

Re: [PATCH v2 2/2] rust: serdev: Fix race condition on driver probe

[Markus Probst]

[-- Attachment #1: Type: text/plain, Size: 5021 bytes --]

On Sat, 2026-09-05 at 13:49 +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [Critical] Use-After-Free and Mutable Aliasing in `drvdata_drop` because memory is freed before clearing the driver data pointer.
> - [High] `T::receive()` can run concurrently with `T::unbind()`, leading to a Use-After-Unbind race condition.
> - [High] Circular deadlock between `probe_callback` and `receive_buf_callback`.
> --
> 
> commit 67f5ed19e07ddf9c3ef694dbb2c1e962df051525
> Author: Markus Probst <markus.probst@posteo.de>
> 
> rust: serdev: Fix race condition on driver probe
> 
> This commit fixes a race condition on driver probe failures by using
> drvdata_drop() instead of drvdata_obtain(), ensuring the serdev device
> is closed before the pointer to the driver data is set to null.
> 
> Closes: https://lore.kernel.org/linux-serial/20260903222159.70A911F000E9@smtp.kernel.org/
> 
> > diff --git a/rust/kernel/serdev.rs b/rust/kernel/serdev.rs
> > index 17ca504b7f8d6..a12b1dea12aa1 100644
> > --- a/rust/kernel/serdev.rs
> > +++ b/rust/kernel/serdev.rs
> > @@ -176,7 +176,7 @@ extern "C" fn probe_callback(sdev: *mut bindings::serdev_device) -> kernel::ffi:
> >              let private_data = unsafe { sdev.as_ref().drvdata_borrow::<PrivateData<'_, T>>() };
> >              let private_data = ScopeGuard::new_with_data(private_data, |_| {
> >                  // SAFETY: We just set drvdata to `PrivateData<'_, T>`.
> > -                drop(unsafe { sdev.as_ref().drvdata_obtain::<PrivateData<'_, T>>() });
> > +                unsafe { sdev.as_ref().drvdata_drop::<PrivateData<'_, T>>() };
> 
> [Severity: Critical]
> This is a pre-existing issue, but does calling drvdata_drop() here introduce
> a use-after-free and mutable aliasing vulnerability?
> 
> Since drvdata_drop() executes drop() on the driver's private data and
> deallocates the memory before calling dev_set_drvdata(..., null), the global
> device pointer still references the freed memory.
> 
> If receive_buf_callback() is invoked concurrently by incoming serial data:
> 
> rust/kernel/serdev.rs:receive_buf_callback() {
>     ...
>     let private_data = unsafe { sdev.as_ref().drvdata_borrow::<PrivateData<'_, T>>() };
>     ...
> }
> 
> It can fetch the still-non-null dangling pointer before it is cleared, and
> block on the active mutex. Once drop() completes, receive_buf_callback()
> would wake up and operate on freed memory.
> 
> >              });
> >              let mut active = private_data.active.lock();
False positive.

The PinnedDrop implementation closes the serdev device and therefore
synchronizes this callback before the data is freed.

> 
> [Severity: High]
> This isn't a bug introduced by this patch, but can holding this active mutex
> across the T::probe() call result in a circular deadlock?
> 
> In probe_callback(), the active mutex is locked before invoking T::probe().
> If a driver writes a command to the device and synchronously waits for a
> response during probe, the incoming response would trigger
> receive_buf_callback():
> 
> rust/kernel/serdev.rs:receive_buf_callback() {
>     ...
>     let active = private_data.active.lock();
>     ...
> }
> 
> Since probe_callback() holds the lock and waits for the response, and
> receive_buf_callback() receives the response but blocks indefinitely waiting
> for the lock, does this structurally guarantee a deadlock for synchronous
> probes?
False positive.

On failure, the drop order guarantees that the lock in probe will be
unlocked first. It is still possible for receive_buf_callback or
PinnedDrop to acquire the mutex then. In any case, there is no
deadlock.

> 
> [ ... ]
> 
> [Severity: High]
> This is a pre-existing issue, but can T::receive() run concurrently with
> T::unbind(), leading to a use-after-unbind race condition?
> 
> During device removal, remove_callback() calls T::unbind():
> 
> rust/kernel/serdev.rs:remove_callback() {
>     ...
>     T::unbind(sdev, data_pinned);
> }
> 
> It does not lock the active mutex, set it to false, or close the serdev
> device prior to unbinding.
> 
> Consequently, incoming serial data can trigger receive_buf_callback(), which
> evaluates the active check as true (since active is not cleared during
> unbind):
> 
> rust/kernel/serdev.rs:receive_buf_callback() {
>     ...
>     if !*active {
>         return length;
>     }
>     ...
>     T::receive(sdev, data_pinned, buf)
> }
> 
> This forwards the data to T::receive(), which can execute concurrently with
> or after T::unbind(), accessing driver resources that are actively being
> destroyed.
It can for now, but this shouldn't be an issue (as the driver should be
aware of that). But this will change anyway through:
https://lore.kernel.org/linux-serial/fe6a21fa8604bb35e32971b80cd75efb8ee3d202.camel@posteo.de/T/#t


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 870 bytes --]

Re: [PATCH v2 1/2] rust: serdev: Fix race condition on driver unbind

[Gary Guo]

On Sat Sep 5, 2026 at 2:30 PM BST, Markus Probst wrote:
> On device unbind, 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 function `drvdata_drop` that leaves the pointer to the driver data
> valid until the Drop has completed. Use it in the post unbind callback.
>
> 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/
> Signed-off-by: Markus Probst <markus.probst@posteo.de>
> ---
>  rust/kernel/device.rs | 27 +++++++++++++++++++++++++++
>  rust/kernel/driver.rs |  2 +-
>  2 files changed, 28 insertions(+), 1 deletion(-)
>
> diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
> index 2291d85b6849..3886cc713c28 100644
> --- a/rust/kernel/device.rs
> +++ b/rust/kernel/device.rs
> @@ -219,6 +219,7 @@ pub fn set_drvdata<T>(&self, data: impl PinInit<T, Error>) -> Result {
>      ///
>      /// - The type `T` must match the type of the `ForeignOwnable` previously stored by
>      ///   [`Device::set_drvdata`].
> +    /// - Must only be called before the device is fully unbound.
>      pub(crate) unsafe fn drvdata_obtain<T>(&self) -> Option<Pin<KBox<T>>> {
>          // SAFETY: By the type invariants, `self.as_raw()` is a valid pointer to a `struct device`.
>          let ptr = unsafe { bindings::dev_get_drvdata(self.as_raw()) };
> @@ -236,6 +237,32 @@ pub(crate) unsafe fn drvdata_obtain<T>(&self) -> Option<Pin<KBox<T>>> {
>          //   in `into_foreign()`.
>          Some(unsafe { Pin::<KBox<T>>::from_foreign(ptr.cast()) })
>      }
> +
> +    /// Drop the private data stored in this [`Device`].
> +    ///
> +    /// The pointer to the private data remains valid until the drop is complete.
> +    ///
> +    /// # Safety
> +    ///
> +    /// - The type `T` must match the type of the `ForeignOwnable` previously stored by
> +    ///   [`Device::set_drvdata`].
> +    pub(crate) unsafe fn drvdata_drop<T>(&self) {
> +        // SAFETY: By the type invariants, `self.as_raw()` is a valid pointer to a `struct device`.
> +        let ptr = unsafe { bindings::dev_get_drvdata(self.as_raw()) };
> +
> +        if ptr.is_null() {
> +            return;
> +        }

How does this help the problem? While drop is running, other code should not
attempt to obtain a reference to the data anymore. Otherwise this still have UB
potential by accessing fields that are just destroyed (not to mention that Rust
alias model also forbid it).

I think the existing actually catches it better, because *if* NULL pointer can
be observed by callbacks, a synchronization is missing in the subsystem. The bus
should first perform a synchronization to ensure callbacks are no longer fired,
and then proceed to clean up resources.

Best,
Gary

> +
> +        // SAFETY:
> +        // - If `ptr` is not NULL, it comes from a previous call to `into_foreign()`.
> +        // - `dev_get_drvdata()` guarantees to return the same pointer given to `dev_set_drvdata()`
> +        //   in `into_foreign()`.
> +        drop(unsafe { Pin::<KBox<T>>::from_foreign(ptr.cast()) });
> +
> +        // SAFETY: By the type invariants, `self.as_raw()` is a valid pointer to a `struct device`.
> +        unsafe { bindings::dev_set_drvdata(self.as_raw(), core::ptr::null_mut()) };
> +    }
>  }
>  
>  impl<Ctx: InternalBoundContext> Device<Ctx> {
> diff --git a/rust/kernel/driver.rs b/rust/kernel/driver.rs
> index c9c74c4dde8f..83410141ef1c 100644
> --- a/rust/kernel/driver.rs
> +++ b/rust/kernel/driver.rs
> @@ -204,7 +204,7 @@ extern "C" fn post_unbind_callback(dev: *mut bindings::device) {
>          //
>          // SAFETY: By the safety requirements of the `Driver` trait, `T::DriverData` is the
>          // driver's bus device private data type.
> -        drop(unsafe { dev.drvdata_obtain::<T::DriverData<'_>>() });
> +        unsafe { dev.drvdata_drop::<T::DriverData<'_>>() };
>      }
>  
>      /// Attach generic `struct device_driver` callbacks.

Re: [PATCH v2 1/2] rust: serdev: Fix race condition on driver unbind

[Markus Probst]

[-- Attachment #1: Type: text/plain, Size: 5039 bytes --]

On Sat, 2026-09-05 at 15:16 +0100, Gary Guo wrote:
> On Sat Sep 5, 2026 at 2:30 PM BST, Markus Probst wrote:
> > On device unbind, 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 function `drvdata_drop` that leaves the pointer to the driver data
> > valid until the Drop has completed. Use it in the post unbind callback.
> > 
> > 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/
> > Signed-off-by: Markus Probst <markus.probst@posteo.de>
> > ---
> >  rust/kernel/device.rs | 27 +++++++++++++++++++++++++++
> >  rust/kernel/driver.rs |  2 +-
> >  2 files changed, 28 insertions(+), 1 deletion(-)
> > 
> > diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
> > index 2291d85b6849..3886cc713c28 100644
> > --- a/rust/kernel/device.rs
> > +++ b/rust/kernel/device.rs
> > @@ -219,6 +219,7 @@ pub fn set_drvdata<T>(&self, data: impl PinInit<T, Error>) -> Result {
> >      ///
> >      /// - The type `T` must match the type of the `ForeignOwnable` previously stored by
> >      ///   [`Device::set_drvdata`].
> > +    /// - Must only be called before the device is fully unbound.
> >      pub(crate) unsafe fn drvdata_obtain<T>(&self) -> Option<Pin<KBox<T>>> {
> >          // SAFETY: By the type invariants, `self.as_raw()` is a valid pointer to a `struct device`.
> >          let ptr = unsafe { bindings::dev_get_drvdata(self.as_raw()) };
> > @@ -236,6 +237,32 @@ pub(crate) unsafe fn drvdata_obtain<T>(&self) -> Option<Pin<KBox<T>>> {
> >          //   in `into_foreign()`.
> >          Some(unsafe { Pin::<KBox<T>>::from_foreign(ptr.cast()) })
> >      }
> > +
> > +    /// Drop the private data stored in this [`Device`].
> > +    ///
> > +    /// The pointer to the private data remains valid until the drop is complete.
> > +    ///
> > +    /// # Safety
> > +    ///
> > +    /// - The type `T` must match the type of the `ForeignOwnable` previously stored by
> > +    ///   [`Device::set_drvdata`].
> > +    pub(crate) unsafe fn drvdata_drop<T>(&self) {
> > +        // SAFETY: By the type invariants, `self.as_raw()` is a valid pointer to a `struct device`.
> > +        let ptr = unsafe { bindings::dev_get_drvdata(self.as_raw()) };
> > +
> > +        if ptr.is_null() {
> > +            return;
> > +        }
> 
> How does this help the problem? While drop is running, other code should not
> attempt to obtain a reference to the data anymore. Otherwise this still have UB
> potential by accessing fields that are just destroyed (not to mention that Rust
> alias model also forbid it).
> 
> I think the existing actually catches it better, because *if* NULL pointer can
> be observed by callbacks, a synchronization is missing in the subsystem. The bus
> should first perform a synchronization to ensure callbacks are no longer fired,
> and then proceed to clean up resources.
The abstraction has been written, so the serdev device stays open until
the drivers private data has been dropped. Until then, the driver can
still have a reference to the device, which can access calls that are
only valid if open.

I don't think I am allowed to rewrite that logic in a rc period.

Thanks
- Markus Probst

> 
> Best,
> Gary
> 
> > +
> > +        // SAFETY:
> > +        // - If `ptr` is not NULL, it comes from a previous call to `into_foreign()`.
> > +        // - `dev_get_drvdata()` guarantees to return the same pointer given to `dev_set_drvdata()`
> > +        //   in `into_foreign()`.
> > +        drop(unsafe { Pin::<KBox<T>>::from_foreign(ptr.cast()) });
> > +
> > +        // SAFETY: By the type invariants, `self.as_raw()` is a valid pointer to a `struct device`.
> > +        unsafe { bindings::dev_set_drvdata(self.as_raw(), core::ptr::null_mut()) };
> > +    }
> >  }
> >  
> >  impl<Ctx: InternalBoundContext> Device<Ctx> {
> > diff --git a/rust/kernel/driver.rs b/rust/kernel/driver.rs
> > index c9c74c4dde8f..83410141ef1c 100644
> > --- a/rust/kernel/driver.rs
> > +++ b/rust/kernel/driver.rs
> > @@ -204,7 +204,7 @@ extern "C" fn post_unbind_callback(dev: *mut bindings::device) {
> >          //
> >          // SAFETY: By the safety requirements of the `Driver` trait, `T::DriverData` is the
> >          // driver's bus device private data type.
> > -        drop(unsafe { dev.drvdata_obtain::<T::DriverData<'_>>() });
> > +        unsafe { dev.drvdata_drop::<T::DriverData<'_>>() };
> >      }
> >  
> >      /// Attach generic `struct device_driver` callbacks.
> 

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 870 bytes --]