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: <DD08HWM0M68R.2R74OSODBIWSZ@kernel.org>
Date: Tue, 23 Sep 2025 16:03:01 +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 Tue Sep 23, 2025 at 3:31 PM CEST, Daniel Almeida wrote:
>>> +/// 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.
>
> What I was brainstorming with Greg is to submit this initial support, and then
> follow up with all the other abstractions needed to implement a Rust version of
> usb-skeleton.c. IIUC, the plan is to submit any fixes as follow-ups, as we're
> close to the merge window.
>
> struct usb_device would be used for the skeleton driver, so we should keep it if
> we're following the plan above, IMHO.

Yes, it's clearly required for the raw accessors for submitting URBs, e.g.
usb_fill_bulk_urb(), usb_submit_urb(), etc.

But I'm not sure you actually have to expose a representation of a struct
usb_device (with device context information) publically for that. It seems to me
that this can all be contained within the abstraction.

For instance, the public API could look like this:

	let urb = intf.urb_create()?;
	urb.fill_bulk(buffer, callback_fn, ...)?;
	urb.submit();

The urb_create() method of a usb::Interface can derive the struct usb_device
from the struct usb_interface internally and store it in the Urb structure, i.e.
no need to let drivers mess with this.

So, I think for this part it makes more sense to first work out the other
APIs before exposing things speculatively.

I also just spotted this:

	impl<Ctx: device::DeviceContext> AsRef<Device<Ctx>> for Interface<Ctx> {
	    fn as_ref(&self) -> &Device<Ctx> {
	        // SAFETY: `self.as_raw()` is valid by the type invariants. For a valid interface,
	        // the helper should always return a valid USB device pointer.
	        let usb_dev = unsafe { bindings::interface_to_usbdev(self.as_raw()) };
	
	        // SAFETY: The helper returns a valid interface pointer that shares the
	        // same `DeviceContext`.
	        unsafe { &*(usb_dev.cast()) }
	    }
	}

which I think is wrong. You can't derive the device context of a usb::Interface
for a usb::Device generically. You probably can for the Bound context, but not
for the Core context.

But honestly, I'm even unsure for the Bound context.

@Greg: Can we guarantee that a struct usb_device is always bound as long as one
of its interfaces is still bound?