* [PATCH] rust: serdev: Synchronize receive callback before calling unbind
@ 2026-09-03 22:03 Markus Probst
2026-09-03 22:21 ` sashiko-bot
2026-09-03 22:34 ` Markus Probst
0 siblings, 2 replies; 4+ messages in thread
From: Markus Probst @ 2026-09-03 22:03 UTC (permalink / raw)
To: Ayush Singh, Johan Hovold, Alex Elder, Greg Kroah-Hartman,
Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Daniel Almeida, Tamir Duberstein,
Alexandre Courbot, Onur Özkan, Eric Biggers, Ard Biesheuvel,
Lorenzo Stoakes, Vlastimil Babka, Liam R. Howlett,
Uladzislau Rezki
Cc: greybus-dev, linux-serial, rust-for-linux, linux-kernel,
Markus Probst
The receive callback and unbind callback now have exclusive access to
the drivers private data. Provide mutable references in callbacks to
avoid the need for locks in the private data. Remove the Sync
requirement.
Signed-off-by: Markus Probst <markus.probst@posteo.de>
---
This patch avoids the need for a SpinLock in the patch series
https://lore.kernel.org/rust-for-linux/20260827-gb-uart-transport-v2-7-a03bb1f5fbd1@beagleboard.org/
.
---
rust/kernel/serdev.rs | 51 ++++++++++++++++++++++----------------
samples/rust/rust_driver_serdev.rs | 2 +-
2 files changed, 31 insertions(+), 22 deletions(-)
diff --git a/rust/kernel/serdev.rs b/rust/kernel/serdev.rs
index 17ca504b7f8d..44f029ed93fd 100644
--- a/rust/kernel/serdev.rs
+++ b/rust/kernel/serdev.rs
@@ -106,7 +106,7 @@ pub struct PrivateData<'bound, T: Driver> {
/// Whether `receive_buf_callback` is allowed to call `Driver::receive`.
///
/// If locked, the receive_buf_callback will be blocked on data reception.
- /// This is the case while the driver is being probed or while [`PrivateData`] is being dropped.
+ /// This is the case while the driver is being probed or removed.
/// This is necessary, because we need to open the serdev device before the driver has been
/// probed in order to allow it to be configured, which allows `receive_buf_callback` to be
/// called. Thus we need to block data until probe completes and the driver data becomes
@@ -127,16 +127,6 @@ pub struct PrivateData<'bound, T: Driver> {
#[pinned_drop]
impl<T: Driver> PinnedDrop for PrivateData<'_, T> {
fn drop(self: Pin<&mut Self>) {
- let mut active = self.active.lock();
- if *active {
- // SAFETY:
- // - We have exclusive access to `self.driver`.
- // - `self.driver` is guaranteed to be initialized.
- unsafe { (*self.driver.get()).assume_init_drop() };
- *active = false;
- }
- drop(active);
-
// SAFETY: We have exclusive access to `self.open`.
if unsafe { *self.open.get() } {
// SAFETY: `self.sdev.as_raw()` is guaranteed to be a pointer to a valid
@@ -176,7 +166,20 @@ 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>>() });
+ let private_data = unsafe {
+ sdev.as_ref()
+ .drvdata_obtain::<PrivateData<'_, T>>()
+ .unwrap_unchecked()
+ };
+
+ let mut active = private_data.active.lock();
+ if *active {
+ // SAFETY:
+ // - We have exclusive access to `private_data.driver`.
+ // - `private_data.driver` is guaranteed to be initialized.
+ unsafe { (*private_data.driver.get()).assume_init_drop() };
+ *active = false;
+ }
});
let mut active = private_data.active.lock();
@@ -222,15 +225,21 @@ extern "C" fn remove_callback(sdev: *mut bindings::serdev_device) {
// and stored a `Pin<KBox<PrivateData<'_, T>>>`.
let private_data = unsafe { sdev.as_ref().drvdata_borrow::<PrivateData<'_, T>>() };
- // SAFETY: No one has exclusive access to `private_data.driver`.
- let data = unsafe { &*private_data.driver.get() };
+ let mut active = private_data.active.lock();
+
+ // SAFETY: We have exclusive access to `private_data.driver`.
+ let data = unsafe { &mut *private_data.driver.get() };
// SAFETY:
// - `private_data.driver` is pinned.
// - `remove_callback` is only ever called after a successful call to `probe_callback`,
// hence it's guaranteed that `private_data.driver` was initialized.
- let data_pinned = unsafe { Pin::new_unchecked(data.assume_init_ref()) };
+ let data_pinned = unsafe { Pin::new_unchecked(data.assume_init_mut()) };
T::unbind(sdev, data_pinned);
+
+ // SAFETY: We already established that `data` is guaranteed to be initialized.
+ unsafe { data.assume_init_drop() };
+ *active = false;
}
extern "C" fn receive_buf_callback(
@@ -254,13 +263,13 @@ extern "C" fn receive_buf_callback(
return length;
}
- // SAFETY: No one has exclusive access to `private_data.driver`.
- let data = unsafe { &*private_data.driver.get() };
+ // SAFETY: We have exclusive access to `private_data.driver`.
+ let data = unsafe { &mut *private_data.driver.get() };
// SAFETY:
// - `private_data.driver` is pinned.
// - `receive_buf_callback` is only ever called after a successful call to `probe_callback`,
// hence it's guaranteed that `private_data.driver` was initialized.
- let data_pinned = unsafe { Pin::new_unchecked(data.assume_init_ref()) };
+ let data_pinned = unsafe { Pin::new_unchecked(data.assume_init_mut()) };
// SAFETY: `buf` is guaranteed to be non-null and has the size of `length`.
let buf = unsafe { core::slice::from_raw_parts(buf, length) };
@@ -365,7 +374,7 @@ pub trait Driver {
type IdInfo: 'static;
/// The type of the driver's bus device private data.
- type Data<'bound>: Send + Sync + 'bound;
+ type Data<'bound>: Send + 'bound;
/// The table of OF device ids supported by the driver.
const OF_ID_TABLE: Option<of::IdTable<Self::IdInfo>> = None;
@@ -391,7 +400,7 @@ fn probe<'bound>(
/// `&Device<Core>` or `&Device<Bound>` reference. For instance.
///
/// Otherwise, release operations for driver resources should be performed in `Drop`.
- fn unbind<'bound>(sdev: &'bound Device<device::Core<'_>>, this: Pin<&Self::Data<'bound>>) {
+ fn unbind<'bound>(sdev: &'bound Device<device::Core<'_>>, this: Pin<&mut Self::Data<'bound>>) {
let _ = (sdev, this);
}
@@ -402,7 +411,7 @@ fn unbind<'bound>(sdev: &'bound Device<device::Core<'_>>, this: Pin<&Self::Data<
/// Returns the number of bytes accepted.
fn receive<'bound>(
sdev: &'bound Device<device::Bound>,
- this: Pin<&Self::Data<'bound>>,
+ this: Pin<&mut Self::Data<'bound>>,
data: &[u8],
) -> usize {
let _ = (sdev, this, data);
diff --git a/samples/rust/rust_driver_serdev.rs b/samples/rust/rust_driver_serdev.rs
index 51b4898cd855..d00d547234c8 100644
--- a/samples/rust/rust_driver_serdev.rs
+++ b/samples/rust/rust_driver_serdev.rs
@@ -63,7 +63,7 @@ fn probe<'bound>(
fn receive<'bound>(
sdev: &'bound serdev::Device<Bound>,
- _this: Pin<&Self>,
+ _this: Pin<&mut Self>,
data: &[u8],
) -> usize {
sdev.write(data).unwrap_or_default() as usize
---
base-commit: e5e04726cdd043e309677071ab1b65a4b18f422b
change-id: 20260903-rust_serdev_ref_mut-4d2285776ae1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] rust: serdev: Synchronize receive callback before calling unbind
2026-09-03 22:03 [PATCH] rust: serdev: Synchronize receive callback before calling unbind Markus Probst
@ 2026-09-03 22:21 ` sashiko-bot
2026-09-03 22:31 ` Markus Probst
2026-09-03 22:34 ` Markus Probst
1 sibling, 1 reply; 4+ messages in thread
From: sashiko-bot @ 2026-09-03 22:21 UTC (permalink / raw)
To: Markus Probst; +Cc: ojeda, linux-serial
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues:
- [High] Holding the `active` mutex during `T::unbind` causes an inescapable deadlock for drivers that synchronously communicate with the device.
- [Low] Logically dead code inside the error-handling `ScopeGuard` in `probe_callback`.
Pre-existing issues:
- [High] Race condition between drvdata_obtain and receive_buf_callback causes NULL pointer dereference or Use-After-Free.
--
commit 6f561db35778bb34c19e46b1a5df116263c0adbc
Author: Markus Probst <markus.probst@posteo.de>
Subject: rust: serdev: Synchronize receive callback before calling unbind
The commit changes the receive and unbind callbacks to provide exclusive
access to driver private data by handing out mutable references. It removes
the Sync requirement for Data and introduces locking for synchronization.
> diff --git a/rust/kernel/serdev.rs b/rust/kernel/serdev.rs
> index 17ca504b7f8d6..44f029ed93fd1 100644
> --- a/rust/kernel/serdev.rs
> +++ b/rust/kernel/serdev.rs
[ ... ]
> @@ -127,16 +127,6 @@ pub struct PrivateData<'bound, T: Driver> {
> #[pinned_drop]
> impl<T: Driver> PinnedDrop for PrivateData<'_, T> {
> fn drop(self: Pin<&mut Self>) {
> - let mut active = self.active.lock();
> - if *active {
> - // SAFETY:
> - // - We have exclusive access to `self.driver`.
> - // - `self.driver` is guaranteed to be initialized.
> - unsafe { (*self.driver.get()).assume_init_drop() };
> - *active = false;
> - }
> - drop(active);
> -
> // SAFETY: We have exclusive access to `self.open`.
> if unsafe { *self.open.get() } {
[Severity: High]
This isn't a bug introduced by this patch, but does the timing of closing the
device cause a use-after-free or NULL pointer dereference?
If probe fails after the device is opened, or when the driver is unbound,
drvdata_obtain() clears the underlying C driver_data pointer to NULL and
drops the data. However, bindings::serdev_device_close(self.sdev.as_raw())
is only called later inside PrivateData::drop().
Because the device is still open while the pointer is cleared, incoming
serial data can trigger receive_buf_callback() in the TTY workqueue.
receive_buf_callback() calls drvdata_borrow(), which reads the NULL pointer
and blindly dereferences it, or reads it right before it's cleared and
accesses freed memory.
Should the device be closed before clearing the drvdata pointer?
[ ... ]
> @@ -176,7 +166,20 @@ 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>>() });
> + let private_data = unsafe {
> + sdev.as_ref()
> + .drvdata_obtain::<PrivateData<'_, T>>()
> + .unwrap_unchecked()
> + };
> +
> + let mut active = private_data.active.lock();
> + if *active {
[Severity: Low]
Is this block of code logically dead?
The ScopeGuard closure checks if *active is true to selectively call
assume_init_drop(). However, *active is only set to true if the driver
initialization succeeds.
Immediately after setting *active = true, private_data.dismiss() is called,
which consumes the guard and prevents this closure from running.
Therefore, this closure only runs on the error path, where *active is
strictly false, meaning the assume_init_drop() block will never execute.
> + // SAFETY:
> + // - We have exclusive access to `private_data.driver`.
> + // - `private_data.driver` is guaranteed to be initialized.
> + unsafe { (*private_data.driver.get()).assume_init_drop() };
> + *active = false;
> + }
> });
> let mut active = private_data.active.lock();
[ ... ]
> @@ -222,15 +225,21 @@ extern "C" fn remove_callback(sdev: *mut bindings::serdev_device) {
> // and stored a `Pin<KBox<PrivateData<'_, T>>>`.
> let private_data = unsafe { sdev.as_ref().drvdata_borrow::<PrivateData<'_, T>>() };
>
> - // SAFETY: No one has exclusive access to `private_data.driver`.
> - let data = unsafe { &*private_data.driver.get() };
> + let mut active = private_data.active.lock();
> +
> + // SAFETY: We have exclusive access to `private_data.driver`.
> + let data = unsafe { &mut *private_data.driver.get() };
> // SAFETY:
> // - `private_data.driver` is pinned.
> // - `remove_callback` is only ever called after a successful call to `probe_callback`,
> // hence it's guaranteed that `private_data.driver` was initialized.
> - let data_pinned = unsafe { Pin::new_unchecked(data.assume_init_ref()) };
> + let data_pinned = unsafe { Pin::new_unchecked(data.assume_init_mut()) };
>
> T::unbind(sdev, data_pinned);
[Severity: High]
Does holding the active mutex during T::unbind() cause a deadlock for
drivers that synchronously communicate with the device?
If the driver's unbind() callback sends a shutdown command to the device and
synchronously waits for an acknowledgment, the incoming serial data for the
acknowledgment will be routed to receive_buf_callback().
receive_buf_callback() attempts to acquire this same active mutex to process
the response, completing the deadlock because remove_callback() is blocked
waiting for the acknowledgment.
> +
> + // SAFETY: We already established that `data` is guaranteed to be initialized.
> + unsafe { data.assume_init_drop() };
> + *active = false;
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260904-rust_serdev_ref_mut-v1-1-245db7eb09af@posteo.de?part=1
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] rust: serdev: Synchronize receive callback before calling unbind
2026-09-03 22:21 ` sashiko-bot
@ 2026-09-03 22:31 ` Markus Probst
0 siblings, 0 replies; 4+ messages in thread
From: Markus Probst @ 2026-09-03 22:31 UTC (permalink / raw)
To: sashiko-reviews; +Cc: ojeda, linux-serial
[-- Attachment #1: Type: text/plain, Size: 6647 bytes --]
On Thu, 2026-09-03 at 22:21 +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
>
> New issues:
> - [High] Holding the `active` mutex during `T::unbind` causes an inescapable deadlock for drivers that synchronously communicate with the device.
> - [Low] Logically dead code inside the error-handling `ScopeGuard` in `probe_callback`.
>
> Pre-existing issues:
> - [High] Race condition between drvdata_obtain and receive_buf_callback causes NULL pointer dereference or Use-After-Free.
> --
>
> commit 6f561db35778bb34c19e46b1a5df116263c0adbc
> Author: Markus Probst <markus.probst@posteo.de>
> Subject: rust: serdev: Synchronize receive callback before calling unbind
>
> The commit changes the receive and unbind callbacks to provide exclusive
> access to driver private data by handing out mutable references. It removes
> the Sync requirement for Data and introduces locking for synchronization.
>
> > diff --git a/rust/kernel/serdev.rs b/rust/kernel/serdev.rs
> > index 17ca504b7f8d6..44f029ed93fd1 100644
> > --- a/rust/kernel/serdev.rs
> > +++ b/rust/kernel/serdev.rs
>
> [ ... ]
>
> > @@ -127,16 +127,6 @@ pub struct PrivateData<'bound, T: Driver> {
> > #[pinned_drop]
> > impl<T: Driver> PinnedDrop for PrivateData<'_, T> {
> > fn drop(self: Pin<&mut Self>) {
> > - let mut active = self.active.lock();
> > - if *active {
> > - // SAFETY:
> > - // - We have exclusive access to `self.driver`.
> > - // - `self.driver` is guaranteed to be initialized.
> > - unsafe { (*self.driver.get()).assume_init_drop() };
> > - *active = false;
> > - }
> > - drop(active);
> > -
> > // SAFETY: We have exclusive access to `self.open`.
> > if unsafe { *self.open.get() } {
>
> [Severity: High]
> This isn't a bug introduced by this patch, but does the timing of closing the
> device cause a use-after-free or NULL pointer dereference?
>
> If probe fails after the device is opened, or when the driver is unbound,
> drvdata_obtain() clears the underlying C driver_data pointer to NULL and
> drops the data. However, bindings::serdev_device_close(self.sdev.as_raw())
> is only called later inside PrivateData::drop().
>
> Because the device is still open while the pointer is cleared, incoming
> serial data can trigger receive_buf_callback() in the TTY workqueue.
>
> receive_buf_callback() calls drvdata_borrow(), which reads the NULL pointer
> and blindly dereferences it, or reads it right before it's cleared and
> accesses freed memory.
>
> Should the device be closed before clearing the drvdata pointer?
This definitely needs to be addressed, yes.
>
> [ ... ]
>
> > @@ -176,7 +166,20 @@ 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>>() });
> > + let private_data = unsafe {
> > + sdev.as_ref()
> > + .drvdata_obtain::<PrivateData<'_, T>>()
> > + .unwrap_unchecked()
> > + };
> > +
> > + let mut active = private_data.active.lock();
> > + if *active {
>
> [Severity: Low]
> Is this block of code logically dead?
>
> The ScopeGuard closure checks if *active is true to selectively call
> assume_init_drop(). However, *active is only set to true if the driver
> initialization succeeds.
>
> Immediately after setting *active = true, private_data.dismiss() is called,
> which consumes the guard and prevents this closure from running.
>
> Therefore, this closure only runs on the error path, where *active is
> strictly false, meaning the assume_init_drop() block will never execute.
True.
>
> > + // SAFETY:
> > + // - We have exclusive access to `private_data.driver`.
> > + // - `private_data.driver` is guaranteed to be initialized.
> > + unsafe { (*private_data.driver.get()).assume_init_drop() };
> > + *active = false;
> > + }
> > });
> > let mut active = private_data.active.lock();
>
> [ ... ]
>
> > @@ -222,15 +225,21 @@ extern "C" fn remove_callback(sdev: *mut bindings::serdev_device) {
> > // and stored a `Pin<KBox<PrivateData<'_, T>>>`.
> > let private_data = unsafe { sdev.as_ref().drvdata_borrow::<PrivateData<'_, T>>() };
> >
> > - // SAFETY: No one has exclusive access to `private_data.driver`.
> > - let data = unsafe { &*private_data.driver.get() };
> > + let mut active = private_data.active.lock();
> > +
> > + // SAFETY: We have exclusive access to `private_data.driver`.
> > + let data = unsafe { &mut *private_data.driver.get() };
> > // SAFETY:
> > // - `private_data.driver` is pinned.
> > // - `remove_callback` is only ever called after a successful call to `probe_callback`,
> > // hence it's guaranteed that `private_data.driver` was initialized.
> > - let data_pinned = unsafe { Pin::new_unchecked(data.assume_init_ref()) };
> > + let data_pinned = unsafe { Pin::new_unchecked(data.assume_init_mut()) };
> >
> > T::unbind(sdev, data_pinned);
>
> [Severity: High]
> Does holding the active mutex during T::unbind() cause a deadlock for
> drivers that synchronously communicate with the device?
>
> If the driver's unbind() callback sends a shutdown command to the device and
> synchronously waits for an acknowledgment, the incoming serial data for the
> acknowledgment will be routed to receive_buf_callback().
>
> receive_buf_callback() attempts to acquire this same active mutex to process
> the response, completing the deadlock because remove_callback() is blocked
> waiting for the acknowledgment.
That would be a driver issue.
And there shouldn't be a reason for a driver to wait for an
acknowledgement.
>
> > +
> > + // SAFETY: We already established that `data` is guaranteed to be initialized.
> > + unsafe { data.assume_init_drop() };
> > + *active = false;
> > }
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 870 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] rust: serdev: Synchronize receive callback before calling unbind
2026-09-03 22:03 [PATCH] rust: serdev: Synchronize receive callback before calling unbind Markus Probst
2026-09-03 22:21 ` sashiko-bot
@ 2026-09-03 22:34 ` Markus Probst
1 sibling, 0 replies; 4+ messages in thread
From: Markus Probst @ 2026-09-03 22:34 UTC (permalink / raw)
To: Ayush Singh, Johan Hovold, Alex Elder, Greg Kroah-Hartman,
Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Daniel Almeida, Tamir Duberstein,
Alexandre Courbot, Onur Özkan, Eric Biggers, Ard Biesheuvel,
Lorenzo Stoakes, Vlastimil Babka, Liam R. Howlett,
Uladzislau Rezki
Cc: greybus-dev, linux-serial, rust-for-linux, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 7867 bytes --]
On Thu, 2026-09-03 at 22:03 +0000, Markus Probst wrote:
> The receive callback and unbind callback now have exclusive access to
> the drivers private data. Provide mutable references in callbacks to
> avoid the need for locks in the private data. Remove the Sync
> requirement.
>
> Signed-off-by: Markus Probst <markus.probst@posteo.de>
> ---
> This patch avoids the need for a SpinLock in the patch series
> https://lore.kernel.org/rust-for-linux/20260827-gb-uart-transport-v2-7-a03bb1f5fbd1@beagleboard.org/
> .
> ---
> rust/kernel/serdev.rs | 51 ++++++++++++++++++++++----------------
> samples/rust/rust_driver_serdev.rs | 2 +-
> 2 files changed, 31 insertions(+), 22 deletions(-)
>
> diff --git a/rust/kernel/serdev.rs b/rust/kernel/serdev.rs
> index 17ca504b7f8d..44f029ed93fd 100644
> --- a/rust/kernel/serdev.rs
> +++ b/rust/kernel/serdev.rs
> @@ -106,7 +106,7 @@ pub struct PrivateData<'bound, T: Driver> {
> /// Whether `receive_buf_callback` is allowed to call `Driver::receive`.
> ///
> /// If locked, the receive_buf_callback will be blocked on data reception.
> - /// This is the case while the driver is being probed or while [`PrivateData`] is being dropped.
> + /// This is the case while the driver is being probed or removed.
> /// This is necessary, because we need to open the serdev device before the driver has been
> /// probed in order to allow it to be configured, which allows `receive_buf_callback` to be
> /// called. Thus we need to block data until probe completes and the driver data becomes
> @@ -127,16 +127,6 @@ pub struct PrivateData<'bound, T: Driver> {
> #[pinned_drop]
> impl<T: Driver> PinnedDrop for PrivateData<'_, T> {
> fn drop(self: Pin<&mut Self>) {
> - let mut active = self.active.lock();
> - if *active {
> - // SAFETY:
> - // - We have exclusive access to `self.driver`.
> - // - `self.driver` is guaranteed to be initialized.
> - unsafe { (*self.driver.get()).assume_init_drop() };
> - *active = false;
> - }
> - drop(active);
> -
> // SAFETY: We have exclusive access to `self.open`.
> if unsafe { *self.open.get() } {
> // SAFETY: `self.sdev.as_raw()` is guaranteed to be a pointer to a valid
> @@ -176,7 +166,20 @@ 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>>() });
> + let private_data = unsafe {
> + sdev.as_ref()
> + .drvdata_obtain::<PrivateData<'_, T>>()
> + .unwrap_unchecked()
> + };
> +
> + let mut active = private_data.active.lock();
> + if *active {
> + // SAFETY:
> + // - We have exclusive access to `private_data.driver`.
> + // - `private_data.driver` is guaranteed to be initialized.
> + unsafe { (*private_data.driver.get()).assume_init_drop() };
> + *active = false;
> + }
Ignore this hunk.
As Sashiko correctly noticed, this introduces dead code.
Thanks
- Markus Probst
> });
> let mut active = private_data.active.lock();
>
> @@ -222,15 +225,21 @@ extern "C" fn remove_callback(sdev: *mut bindings::serdev_device) {
> // and stored a `Pin<KBox<PrivateData<'_, T>>>`.
> let private_data = unsafe { sdev.as_ref().drvdata_borrow::<PrivateData<'_, T>>() };
>
> - // SAFETY: No one has exclusive access to `private_data.driver`.
> - let data = unsafe { &*private_data.driver.get() };
> + let mut active = private_data.active.lock();
> +
> + // SAFETY: We have exclusive access to `private_data.driver`.
> + let data = unsafe { &mut *private_data.driver.get() };
> // SAFETY:
> // - `private_data.driver` is pinned.
> // - `remove_callback` is only ever called after a successful call to `probe_callback`,
> // hence it's guaranteed that `private_data.driver` was initialized.
> - let data_pinned = unsafe { Pin::new_unchecked(data.assume_init_ref()) };
> + let data_pinned = unsafe { Pin::new_unchecked(data.assume_init_mut()) };
>
> T::unbind(sdev, data_pinned);
> +
> + // SAFETY: We already established that `data` is guaranteed to be initialized.
> + unsafe { data.assume_init_drop() };
> + *active = false;
> }
>
> extern "C" fn receive_buf_callback(
> @@ -254,13 +263,13 @@ extern "C" fn receive_buf_callback(
> return length;
> }
>
> - // SAFETY: No one has exclusive access to `private_data.driver`.
> - let data = unsafe { &*private_data.driver.get() };
> + // SAFETY: We have exclusive access to `private_data.driver`.
> + let data = unsafe { &mut *private_data.driver.get() };
> // SAFETY:
> // - `private_data.driver` is pinned.
> // - `receive_buf_callback` is only ever called after a successful call to `probe_callback`,
> // hence it's guaranteed that `private_data.driver` was initialized.
> - let data_pinned = unsafe { Pin::new_unchecked(data.assume_init_ref()) };
> + let data_pinned = unsafe { Pin::new_unchecked(data.assume_init_mut()) };
>
> // SAFETY: `buf` is guaranteed to be non-null and has the size of `length`.
> let buf = unsafe { core::slice::from_raw_parts(buf, length) };
> @@ -365,7 +374,7 @@ pub trait Driver {
> type IdInfo: 'static;
>
> /// The type of the driver's bus device private data.
> - type Data<'bound>: Send + Sync + 'bound;
> + type Data<'bound>: Send + 'bound;
>
> /// The table of OF device ids supported by the driver.
> const OF_ID_TABLE: Option<of::IdTable<Self::IdInfo>> = None;
> @@ -391,7 +400,7 @@ fn probe<'bound>(
> /// `&Device<Core>` or `&Device<Bound>` reference. For instance.
> ///
> /// Otherwise, release operations for driver resources should be performed in `Drop`.
> - fn unbind<'bound>(sdev: &'bound Device<device::Core<'_>>, this: Pin<&Self::Data<'bound>>) {
> + fn unbind<'bound>(sdev: &'bound Device<device::Core<'_>>, this: Pin<&mut Self::Data<'bound>>) {
> let _ = (sdev, this);
> }
>
> @@ -402,7 +411,7 @@ fn unbind<'bound>(sdev: &'bound Device<device::Core<'_>>, this: Pin<&Self::Data<
> /// Returns the number of bytes accepted.
> fn receive<'bound>(
> sdev: &'bound Device<device::Bound>,
> - this: Pin<&Self::Data<'bound>>,
> + this: Pin<&mut Self::Data<'bound>>,
> data: &[u8],
> ) -> usize {
> let _ = (sdev, this, data);
> diff --git a/samples/rust/rust_driver_serdev.rs b/samples/rust/rust_driver_serdev.rs
> index 51b4898cd855..d00d547234c8 100644
> --- a/samples/rust/rust_driver_serdev.rs
> +++ b/samples/rust/rust_driver_serdev.rs
> @@ -63,7 +63,7 @@ fn probe<'bound>(
>
> fn receive<'bound>(
> sdev: &'bound serdev::Device<Bound>,
> - _this: Pin<&Self>,
> + _this: Pin<&mut Self>,
> data: &[u8],
> ) -> usize {
> sdev.write(data).unwrap_or_default() as usize
>
> ---
> base-commit: e5e04726cdd043e309677071ab1b65a4b18f422b
> change-id: 20260903-rust_serdev_ref_mut-4d2285776ae1
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 870 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-09-03 22:34 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-03 22:03 [PATCH] rust: serdev: Synchronize receive callback before calling unbind Markus Probst
2026-09-03 22:21 ` sashiko-bot
2026-09-03 22:31 ` Markus Probst
2026-09-03 22:34 ` Markus Probst
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox