linux-kernel - Re: [PATCH 1/2] rust: usb: add basic USB abstractions

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <DD07LUJXNZN9.3RHH9NJNRFVNN@kernel.org>
Date: Tue, 23 Sep 2025 15:21:09 +0200
From: "Danilo Krummrich" <dakr@...nel.org>
To: "Daniel Almeida" <daniel.almeida@...labora.com>
Cc: "Miguel Ojeda" <ojeda@...nel.org>, "Alex Gaynor"
 <alex.gaynor@...il.com>, "Boqun Feng" <boqun.feng@...il.com>, "Gary Guo"
 <gary@...yguo.net>, Björn Roy Baron
 <bjorn3_gh@...tonmail.com>, "Benno Lossin" <lossin@...nel.org>, "Andreas
 Hindborg" <a.hindborg@...nel.org>, "Alice Ryhl" <aliceryhl@...gle.com>,
 "Trevor Gross" <tmgross@...ch.edu>, "Greg Kroah-Hartman"
 <gregkh@...uxfoundation.org>, <linux-kernel@...r.kernel.org>,
 <rust-for-linux@...r.kernel.org>, <linux-usb@...r.kernel.org>
Subject: Re: [PATCH 1/2] rust: usb: add basic USB abstractions

On Mon Aug 25, 2025 at 8:18 PM CEST, Daniel Almeida wrote:
> +/// The USB driver trait.
> +///
> +/// # Examples
> +///
> +///```
> +/// # use kernel::{bindings, device::Core, usb};
> +/// use kernel::prelude::*;
> +///
> +/// struct MyDriver;
> +///
> +/// kernel::usb_device_table!(
> +///     USB_TABLE,
> +///     MODULE_USB_TABLE,
> +///     <MyDriver as usb::Driver>::IdInfo,
> +///     [
> +///         (usb::DeviceId::from_id(0x1234, 0x5678), ()),
> +///         (usb::DeviceId::from_id(0xabcd, 0xef01), ()),
> +///     ]
> +/// );
> +///
> +/// impl usb::Driver for MyDriver {
> +///     type IdInfo = ();
> +///     const ID_TABLE: usb::IdTable<Self::IdInfo> = &USB_TABLE;
> +///
> +///     fn probe(
> +///         _interface: &usb::Interface<Core>,
> +///         _id: &usb::DeviceId,
> +///         _info: &Self::IdInfo,
> +///     ) -> Result<Pin<KBox<Self>>> {
> +///         Err(ENODEV)
> +///     }
> +///
> +///     fn disconnect(_interface: &usb::Interface<Core>, _data: Pin<&Self>) {}
> +/// }
> +///```
> +pub trait Driver {
> +    /// The type holding information about each one of the device ids supported by the driver.
> +    type IdInfo: 'static;
> +
> +    /// The table of device ids supported by the driver.
> +    const ID_TABLE: IdTable<Self::IdInfo>;
> +
> +    /// USB driver probe.
> +    ///
> +    /// 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,
> +    ) -> Result<Pin<KBox<Self>>>;
> +
> +    /// USB driver disconnect.
> +    ///
> +    /// Called when the USB interface is about to be unbound from this driver.
> +    fn disconnect(interface: &Interface<device::Core>, data: Pin<&Self>);

I think this callback should be optional, like all the other unbind() we have in
other busses.

@Greg: Why is this called disconnect() in the C code instead of remove()?

For Rust, I feel like we should align to the unbind() terminology we use
everywhere else (for the same reasons we chose unbind() over remove() for other
busses as well).

We went with unbind(), since the "raw" remove() (or disconnect()) callback does
more, i.e. it first calls into unbind() and then drops the driver's private data
for this specific device.

So, the unbind() callback that goes to the driver is only meant as a place for
drivers to perform operations to tear down the device. Resources are freed
subsequently when the driver's private data is dropped.

> +/// A USB device.
> +///
> +/// This structure represents the Rust abstraction for a C [`struct usb_device`].
> +/// The implementation abstracts the usage of a C [`struct usb_device`] passed in
> +/// from the C side.
> +///
> +/// # Invariants
> +///
> +/// A [`Device`] instance represents a valid [`struct usb_device`] created by the C portion of the
> +/// kernel.
> +///
> +/// [`struct usb_device`]: https://www.kernel.org/doc/html/latest/driver-api/usb/usb.html#c.usb_device
> +#[repr(transparent)]
> +pub struct Device<Ctx: device::DeviceContext = device::Normal>(
> +    Opaque<bindings::usb_device>,
> +    PhantomData<Ctx>,
> +);

What do you use the struct usb_device abstraction for? I only see the sample
driver probing a USB interface instead.