All of lore.kernel.org
 help / color / mirror / Atom feed
From: Markus Probst <markus.probst@posteo.de>
To: "Miguel Ojeda" <ojeda@kernel.org>,
	"Boqun Feng" <boqun@kernel.org>, "Gary Guo" <gary@garyguo.net>,
	"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
	"Benno Lossin" <lossin@kernel.org>,
	"Andreas Hindborg" <a.hindborg@kernel.org>,
	"Alice Ryhl" <aliceryhl@google.com>,
	"Trevor Gross" <tmgross@umich.edu>,
	"Danilo Krummrich" <dakr@kernel.org>,
	"Daniel Almeida" <daniel.almeida@collabora.com>,
	"Tamir Duberstein" <tamird@kernel.org>,
	"Alexandre Courbot" <acourbot@nvidia.com>,
	"Onur Özkan" <work@onurozkan.dev>,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	"Rafael J. Wysocki" <rafael@kernel.org>
Cc: linux-serial@vger.kernel.org, rust-for-linux@vger.kernel.org,
	 linux-kernel@vger.kernel.org, driver-core@lists.linux.dev,
	 Markus Probst <markus.probst@posteo.de>,
	 Sashiko Bot <sashiko-bot@kernel.org>
Subject: [PATCH v2 1/2] rust: serdev: Fix race condition on driver unbind
Date: Sat, 05 Sep 2026 13:30:10 +0000	[thread overview]
Message-ID: <20260905-rust_serdev_fix-v2-1-35dfcd06ef2e@posteo.de> (raw)
In-Reply-To: <20260905-rust_serdev_fix-v2-0-35dfcd06ef2e@posteo.de>

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


  reply	other threads:[~2026-09-05 13:30 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-05 13:30 [PATCH v2 0/2] rust: serdev: Mitigate race conditions Markus Probst
2026-09-05 13:30 ` Markus Probst [this message]
2026-09-05 13:44   ` [PATCH v2 1/2] rust: serdev: Fix race condition on driver unbind sashiko-bot
2026-09-05 13:47     ` Markus Probst
2026-09-05 14:16   ` Gary Guo
2026-09-05 17:44     ` Markus Probst
2026-09-05 13:30 ` [PATCH v2 2/2] rust: serdev: Fix race condition on driver probe Markus Probst
2026-09-05 13:49   ` sashiko-bot
2026-09-05 13:57     ` Markus Probst

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260905-rust_serdev_fix-v2-1-35dfcd06ef2e@posteo.de \
    --to=markus.probst@posteo.de \
    --cc=a.hindborg@kernel.org \
    --cc=acourbot@nvidia.com \
    --cc=aliceryhl@google.com \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun@kernel.org \
    --cc=dakr@kernel.org \
    --cc=daniel.almeida@collabora.com \
    --cc=driver-core@lists.linux.dev \
    --cc=gary@garyguo.net \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-serial@vger.kernel.org \
    --cc=lossin@kernel.org \
    --cc=ojeda@kernel.org \
    --cc=rafael@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=sashiko-bot@kernel.org \
    --cc=tamird@kernel.org \
    --cc=tmgross@umich.edu \
    --cc=work@onurozkan.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.