Re: [PATCH v2 1/2] rust: serdev: Fix race condition on driver unbind
flat view
From: sashiko-bot@kernel.org
Date: 2026-09-05 13:44:58
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 hunk ↗ jump to hunk
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 hunk ↗ jump to hunk
@@ -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 hunk ↗ jump to hunk
+ // 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