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

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 [off-list ref]

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/ (local)

quoted

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

[ ... ]

quoted

@@ -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]`.

quoted

+        // 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.

quoted

+    }
 }