Re: DIY allocation or embedding of URBs in larger structures

[Takashi Iwai <tiwai@suse.de>]

On Thu, 03 Sep 2026 13:07:57 +0200,
Michal Pecio wrote:
> 
> On Thu, 03 Sep 2026 11:44:45 +0200, Takashi Iwai wrote:
> > On Thu, 03 Sep 2026 11:24:17 +0200,
> > Michal Pecio wrote:
> > > So what happens here is that USB core continues to use a URB after
> > > completion to implement things like usb_kill_urb(), so URBs are
> > > reference counted. Then core decrements the count - one more use.
> > > 
> > > If a driver waits for completion or even usb_kill_urb() to return
> > > and then proceeds to free the URB's storage, this becomes a UAF.
> > > This driver embeds 2 URBs in its priv and does just that.
> > > 
> > > A URB can only exist as an independent allocation, core will free
> > > it if upon finding zero reference count in such case.
> >
> > So, IIUC, now the URB *must* be always allocated via usb_alloc_urb()
> > and an embedded URB isn't allowed?  If so, we'd need to address other
> > drivers, too.
> 
> Yes, that's basically the case and actually has been for a long time.
> 
> The only thing that works is for both USB and the driver to call
> usb_free_urb() and whoever does last will actually free the storage.
> This means URB can't share storage with anything else.
> 
> Some drivers got away with making URB the first member of a struct
> which is freed together with it. AFAIK this pattern doesn't crash, but
> it's deprecated too because it puts a flexible member (in the URB) in
> the middle of (the outer) struct.
> 
> This pattern here in caiaq has always been one race away from UAF.
> Maybe it wasn't very likely to happen, but stuff like PREEMPT_RT and
> hypervisors can insert unexpected delays anywhere these days.
> 
> Greg KH puts it thusly:
> https://lore.kernel.org/linux-usb/2025120716-sway-hypnotic-8cb6@gregkh/

OK, I'll send the fix patches for the rest of sound drivers.


thanks,

Takashi