Re: [PATCH v4 14/27] rust: usb: make Driver trait lifetime-parameterized

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Critical] The new `'bound` lifetime parameter incorrectly allows drivers to capture and store the dynamic `usb_device_id` pointer (`id: &'bound DeviceId`), which can be freed by userspace at any time, leading to a Use-After-Free.
- [Critical] The patch relies on unsound lifetime transmutation when storing and retrieving the lifetime-parameterized driver data via `drvdata`.
- [Low] The commit message incorrectly refers to the `unbind()` callback instead of `disconnect()`.
- [Low] Stale comments refer to `T::Data` without its new lifetime parameter.
--

commit ca8bfff774a157ce3cf21956820d617a046e6a08
Author: Danilo Krummrich <dakr@kernel.org>

rust: usb: make Driver trait lifetime-parameterized

Add a 'bound lifetime to the associated Data, changing type Data to type
Data<'bound>. This allows the driver's bus device private data to capture
the device / driver bound lifetime.

> The probe() and unbind() callbacks thus gain a 'bound lifetime parameter
> on the methods themselves; avoiding a global lifetime on the trait impl.

[Severity: Low]
The commit message states "unbind()", but the usb::Driver trait defines
a disconnect() method. Should this say disconnect() instead?

> diff --git a/rust/kernel/usb.rs b/rust/kernel/usb.rs
> index 1dbb8387b4636..616e22e34c6fd 100644
> --- a/rust/kernel/usb.rs
> +++ b/rust/kernel/usb.rs

[ ... ]

> @@ -110,7 +110,7 @@ extern "C" fn disconnect_callback(intf: *mut bindings::usb_interface) {
>          // SAFETY: `disconnect_callback` is only ever called after a successful call to
>          // `probe_callback`, hence it's guaranteed that `Device::set_drvdata()` has been called
>          // and stored a `Pin<KBox<T::Data>>`.

[Severity: Low]
This comment refers to T::Data without the new lifetime parameter.
Should it be updated to Pin<KBox<T::Data<'_>>>?

> -        let data = unsafe { dev.drvdata_borrow::<T::Data>() };
> +        let data = unsafe { dev.drvdata_borrow::<T::Data<'_>>() };

[Severity: Critical]
Does this create an unsound lifetime transmutation?

The driver's data T::Data<'a> is instantiated with an ephemeral lifetime 'a
in probe_callback and passed to dev.set_drvdata(), erasing it to a raw
pointer.

Here, it is retrieved using a new, disjoint ephemeral lifetime. If T::Data
is invariant over 'bound (for example, using Mutex<Cell<&'bound T>>), these
represent completely different types.

ForeignOwnable::borrow requires an exact type match. Violating this might
allow safe Rust code to write a short-lived reference into the driver data
during disconnect, which would then dangle when disconnect returns.

Could this lead to a use-after-free when the Drop implementation runs during
post_unbind_callback?

[ ... ]

> @@ -315,16 +319,19 @@ pub trait Driver {
>      ///
>      /// Called when a new USB interface is bound to this driver.
>      /// Implementers should attempt to initialize the interface here.
> -    fn probe(
> -        interface: &Interface<device::Core<'_>>,
> -        id: &DeviceId,
> -        id_info: &Self::IdInfo,
> -    ) -> impl PinInit<Self::Data, Error>;
> +    fn probe<'bound>(
> +        interface: &'bound Interface<device::Core<'_>>,
> +        id: &'bound DeviceId,
> +        id_info: &'bound Self::IdInfo,
> +    ) -> impl PinInit<Self::Data<'bound>, Error> + 'bound;

[Severity: Critical]
Can this lead to a use-after-free if userspace removes a dynamic ID?

The probe method receives id: &'bound DeviceId, allowing the driver to
capture this reference into its T::Data<'bound>.

However, this id pointer can come from a dynamically allocated struct
usb_dynid via usb_match_dynamic_id(). Userspace can trigger
remove_id_store() via sysfs, which immediately frees the usb_dynid
structure without unbinding existing devices.

If the driver's private data holds this reference, it seems it would be
left with a dangling pointer.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260521233501.1191842-1-dakr@kernel.org?part=14