Re: [PATCH v2] usb: raw_gadget: fix use-after-free when UDC is removed

Greg Kroah-Hartman <[email protected]>
Newsgroups gmane.linux.kernel,gmane.linux.usb.general
Message-ID <2026080451-exterior-stank-a019@gregkh>
On Tue, Aug 04, 2026 at 12:39:05PM +0530, Anuj Bolewar via B4 Relay wrote:
> From: Anuj Bolewar <[email protected]>
> 
> When the UDC is removed (e.g. dummy_hcd unbind via sysfs) while the raw
> gadget fd is still open, usb_del_gadget() destroys the gadget device
> and its name. raw_gadget keeps a dangling pointer in dev->gadget, and
> ioctls dereference it after releasing dev->lock, leading to a
> use-after-free in dev_err() when usb_ep_queue() fails.
> 
> Take a gadget reference in gadget_bind() and drop it in dev_free() so
> the gadget device and its name stay alive for as long as the fd is open
> and an ioctl may still dereference dev->gadget. gadget_unbind() only
> marks the device as failed under dev->lock; the reference is dropped in
> dev_free(), which runs only after the fd is closed and no ioctl can be
> in flight.
> 
> Reported-by: [email protected]
> Closes: https://syzkaller.appspot.com/bug?extid=9aacea11bc70c3ddaff2
> Fixes: f2c2e717642c ("usb: gadget: add raw-gadget interface")
> Signed-off-by: Anuj Bolewar <[email protected]>
> ---
> The gadget device embedded in the UDC is destroyed when the UDC is
> removed while the raw gadget fd is still open (e.g. unbinding dummy_hcd
> via sysfs). raw_gadget keeps a dangling pointer in dev->gadget and
> ioctls dereference it after dropping dev->lock, which KASAN reports as a
> slab use-after-free in raw_process_ep0_io() (dev_err with a freed device
> name).
> 
> Fix it by holding a gadget reference for the raw device lifetime:
> gadget_bind() takes it, gadget_unbind() only marks the device as failed
> under dev->lock, and the reference is dropped in dev_free() once the fd
> is closed and no ioctl can still be in flight.
> ---
> Changes in v2:
> - Reworked per review: the gadget reference is now held for the raw
>   device lifetime (bind until dev_free()) instead of the bind/unbind
>   window, since ioctls dereference dev->gadget after releasing dev->lock
>   and the UDC core owns the gadget's lifetime during bind/unbind.
> - gadget_unbind() now only marks the device as failed under dev->lock;
>   the reference is dropped in dev_free() after the fd is closed.
> - Dropped the now-unneeded comment and switched the spinlock to guard().
> - Link to v1: https://patch.msgid.link/[email protected]
> ---
>  drivers/usb/gadget/legacy/raw_gadget.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/gadget/legacy/raw_gadget.c b/drivers/usb/gadget/legacy/raw_gadget.c
> index 4febf8dac7c..d6341823f90 100644
> --- a/drivers/usb/gadget/legacy/raw_gadget.c
> +++ b/drivers/usb/gadget/legacy/raw_gadget.c
> @@ -226,6 +226,7 @@ static void dev_free(struct kref *kref)
>  		kfree(dev->eps[i].ep->desc);
>  		dev->eps[i].state = STATE_EP_DISABLED;
>  	}
> +	usb_put_gadget(dev->gadget);
>  	kfree(dev);
>  }
>  
> @@ -302,7 +303,7 @@ static int gadget_bind(struct usb_gadget *gadget,
>  	dev->req = req;
>  	dev->req->context = dev;
>  	dev->req->complete = gadget_ep0_complete;
> -	dev->gadget = gadget;
> +	dev->gadget = usb_get_gadget(gadget);
>  	gadget_for_each_ep(ep, dev->gadget) {
>  		dev->eps[i].ep = ep;
>  		dev->eps[i].addr = get_ep_addr(ep->name);
> @@ -329,6 +330,10 @@ static void gadget_unbind(struct usb_gadget *gadget)
>  {
>  	struct raw_dev *dev = get_gadget_data(gadget);
>  
> +	{
> +		guard(spinlock_irqsave)(&dev->lock);
> +		dev->state = STATE_DEV_FAILED;
> +	}

Please look at how scoped_guard() works.

And again, are you _SURE_ you need to call get/put on the device?  That
still feels wrong...

And you did not answer my question about the need for the Assisted-by:
tag.

greg k-h
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.