* [PATCH] usb: raw-gadget: fix double-free race during UDC unbind and rebind
@ 2026-10-24 18:37 Igor Putko
2026-10-24 19:07 ` [PATCH v2] " Igor Putko
0 siblings, 1 reply; 3+ messages in thread
From: Igor Putko @ 2026-10-24 18:37 UTC (permalink / raw)
To: Andrey Konovalov, Greg Kroah-Hartman
Cc: linux-usb, linux-kernel, syzbot+b7534c9b380f81a3b4be, Igor Putko
raw-gadget defers endpoint disabling, request freeing, and descriptor
cleanup to dev_free(). However, dev_free() is only called after
dev->count drops to zero in raw_release(), which happens after
usb_gadget_unregister_driver() has already returned.
Once usb_gadget_unregister_driver() finishes, the UDC can be immediately
rebound by another raw_gadget instance. If that new instance enables an
endpoint, it assigns its newly allocated descriptor to ep->ep->desc on
the shared UDC endpoint. When dev_free() from the old instance runs
concurrently, it calls usb_ep_disable() and frees dev->eps[i].ep->desc,
which now belongs to the new instance. The new instance's usb_ep_enable()
subsequently fails with -ESHUTDOWN, and its error path frees the descriptor
again, causing a KASAN double-free.
Tear down all endpoints and requests synchronously in gadget_unbind()
before the UDC can be bound again. Keep the descriptor pointer inside
struct raw_ep and clear ep->ep->desc on error and disable paths so no
dangling pointers are left on the UDC endpoints. Additionally, make sure
raw_release() always unregisters the driver if registered, and release
the ep0 request if gadget_bind() encounters an error.
Reported-by: syzbot+b7534c9b380f81a3b4be@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=b7534c9b380f81a3b4be
Fixes: f2c2e717642c ("usb: gadget: add raw-gadget interface")
Signed-off-by: Igor Putko <igorpetindev@gmail.com>
---
drivers/usb/gadget/legacy/raw_gadget.c | 69 +++++++++++++++++++-------
1 file changed, 52 insertions(+), 17 deletions(-)
diff --git a/drivers/usb/gadget/legacy/raw_gadget.c b/drivers/usb/gadget/legacy/raw_gadget.c
index 4febf8dac7ca..379351a010f8 100644
--- a/drivers/usb/gadget/legacy/raw_gadget.c
+++ b/drivers/usb/gadget/legacy/raw_gadget.c
@@ -141,6 +141,7 @@ struct raw_ep {
enum ep_state state;
struct usb_ep *ep;
u8 addr;
+ struct usb_endpoint_descriptor *desc;
struct usb_request *req;
bool urb_queued;
bool disabling;
@@ -205,7 +206,6 @@ static struct raw_dev *dev_new(void)
static void dev_free(struct kref *kref)
{
struct raw_dev *dev = container_of(kref, struct raw_dev, count);
- int i;
kfree(dev->udc_name);
kfree(dev->driver.udc_name);
@@ -218,14 +218,6 @@ static void dev_free(struct kref *kref)
usb_ep_free_request(dev->gadget->ep0, dev->req);
}
raw_event_queue_destroy(&dev->queue);
- for (i = 0; i < dev->eps_num; i++) {
- if (dev->eps[i].state == STATE_EP_DISABLED)
- continue;
- usb_ep_disable(dev->eps[i].ep);
- usb_ep_free_request(dev->eps[i].ep, dev->eps[i].req);
- kfree(dev->eps[i].ep->desc);
- dev->eps[i].state = STATE_EP_DISABLED;
- }
kfree(dev);
}
@@ -316,6 +308,12 @@ static int gadget_bind(struct usb_gadget *gadget,
ret = raw_queue_event(dev, USB_RAW_EVENT_CONNECT, 0, NULL);
if (ret < 0) {
dev_err(&gadget->dev, "failed to queue connect event\n");
+ usb_ep_free_request(gadget->ep0, req);
+ spin_lock_irqsave(&dev->lock, flags);
+ dev->req = NULL;
+ dev->gadget = NULL;
+ dev->eps_num = 0;
+ spin_unlock_irqrestore(&dev->lock, flags);
set_gadget_data(gadget, NULL);
return ret;
}
@@ -328,6 +326,43 @@ static int gadget_bind(struct usb_gadget *gadget,
static void gadget_unbind(struct usb_gadget *gadget)
{
struct raw_dev *dev = get_gadget_data(gadget);
+ int i;
+ unsigned long flags;
+
+ if (!dev)
+ return;
+
+ for (i = 0; i < dev->eps_num; i++) {
+ struct raw_ep *ep = &dev->eps[i];
+
+ spin_lock_irqsave(&dev->lock, flags);
+ if (ep->state == STATE_EP_DISABLED) {
+ spin_unlock_irqrestore(&dev->lock, flags);
+ continue;
+ }
+ ep->state = STATE_EP_DISABLED;
+ spin_unlock_irqrestore(&dev->lock, flags);
+
+ usb_ep_disable(ep->ep);
+
+ spin_lock_irqsave(&dev->lock, flags);
+ usb_ep_free_request(ep->ep, ep->req);
+ kfree(ep->desc);
+ ep->desc = NULL;
+ ep->ep->desc = NULL;
+ spin_unlock_irqrestore(&dev->lock, flags);
+ }
+
+ spin_lock_irqsave(&dev->lock, flags);
+ if (dev->req) {
+ if (dev->ep0_urb_queued)
+ usb_ep_dequeue(dev->gadget->ep0, dev->req);
+ usb_ep_free_request(dev->gadget->ep0, dev->req);
+ dev->req = NULL;
+ }
+ dev->gadget = NULL;
+ dev->eps_num = 0;
+ spin_unlock_irqrestore(&dev->lock, flags);
set_gadget_data(gadget, NULL);
/* Matches kref_get() in gadget_bind(). */
@@ -450,10 +485,6 @@ static int raw_release(struct inode *inode, struct file *fd)
spin_lock_irqsave(&dev->lock, flags);
dev->state = STATE_DEV_CLOSED;
- if (!dev->gadget) {
- spin_unlock_irqrestore(&dev->lock, flags);
- goto out_put;
- }
if (dev->gadget_registered)
unregister = true;
dev->gadget_registered = false;
@@ -469,7 +500,6 @@ static int raw_release(struct inode *inode, struct file *fd)
kref_put(&dev->count, dev_free);
}
-out_put:
/* Matches dev_new() in raw_open(). */
kref_put(&dev->count, dev_free);
return ret;
@@ -886,6 +916,7 @@ static int raw_ioctl_ep_enable(struct raw_dev *dev, unsigned long value)
if (ret < 0) {
dev_err(&dev->gadget->dev,
"fail, usb_ep_enable returned %d\n", ret);
+ ep->ep->desc = NULL;
goto out_free;
}
ep->req = usb_ep_alloc_request(ep->ep, GFP_ATOMIC);
@@ -893,10 +924,12 @@ static int raw_ioctl_ep_enable(struct raw_dev *dev, unsigned long value)
dev_err(&dev->gadget->dev,
"fail, usb_ep_alloc_request failed\n");
usb_ep_disable(ep->ep);
+ ep->ep->desc = NULL;
ret = -ENOMEM;
goto out_free;
}
ep->state = STATE_EP_ENABLED;
+ ep->desc = desc;
ep->ep->driver_data = ep;
ret = i;
goto out_unlock;
@@ -962,7 +995,9 @@ static int raw_ioctl_ep_disable(struct raw_dev *dev, unsigned long value)
spin_lock_irqsave(&dev->lock, flags);
usb_ep_free_request(dev->eps[i].ep, dev->eps[i].req);
- kfree(dev->eps[i].ep->desc);
+ kfree(dev->eps[i].desc);
+ dev->eps[i].desc = NULL;
+ dev->eps[i].ep->desc = NULL;
dev->eps[i].state = STATE_EP_DISABLED;
dev->eps[i].disabling = false;
@@ -1010,7 +1045,7 @@ static int raw_ioctl_ep_set_clear_halt_wedge(struct raw_dev *dev,
ret = -EINVAL;
goto out_unlock;
}
- if (usb_endpoint_xfer_isoc(dev->eps[i].ep->desc)) {
+ if (usb_endpoint_xfer_isoc(dev->eps[i].desc)) {
dev_dbg(&dev->gadget->dev,
"fail, can't halt/wedge ISO endpoint\n");
ret = -EINVAL;
@@ -1096,7 +1131,7 @@ static int raw_process_ep_io(struct raw_dev *dev, struct usb_raw_ep_io *io,
ret = -EBUSY;
goto out_unlock;
}
- if (in != usb_endpoint_dir_in(ep->ep->desc)) {
+ if (in != usb_endpoint_dir_in(ep->desc)) {
dev_dbg(&dev->gadget->dev, "fail, wrong direction\n");
ret = -EINVAL;
goto out_unlock;
--
2.47.3
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [PATCH v2] usb: raw-gadget: fix double-free race during UDC unbind and rebind
2026-10-24 18:37 [PATCH] usb: raw-gadget: fix double-free race during UDC unbind and rebind Igor Putko
@ 2026-10-24 19:07 ` Igor Putko
2026-11-06 1:16 ` Andrey Konovalov
0 siblings, 1 reply; 3+ messages in thread
From: Igor Putko @ 2026-10-24 19:07 UTC (permalink / raw)
To: Andrey Konovalov, Greg Kroah-Hartman
Cc: linux-usb, linux-kernel, syzbot+b7534c9b380f81a3b4be, Igor Putko
raw-gadget defers endpoint disabling, request freeing, and descriptor
cleanup to dev_free(). However, dev_free() is only called after
dev->count drops to zero in raw_release(), which happens after
usb_gadget_unregister_driver() has already returned.
Once usb_gadget_unregister_driver() finishes, the UDC can be immediately
rebound by another raw_gadget instance. If that new instance enables an
endpoint, it assigns its newly allocated descriptor to ep->ep->desc on
the shared UDC endpoint. When dev_free() from the old instance runs
concurrently, it calls usb_ep_disable() and frees dev->eps[i].ep->desc,
which now belongs to the new instance. The new instance's usb_ep_enable()
subsequently fails with -ESHUTDOWN, and its error path frees the descriptor
again, causing a KASAN double-free.
Tear down active endpoints and requests synchronously in gadget_unbind()
before the UDC can be bound again. Keep the descriptor pointer inside
struct raw_ep and clear ep->ep->desc on error and disable paths so no
dangling pointers are left on the UDC endpoints.
Reported-by: syzbot+b7534c9b380f81a3b4be@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=b7534c9b380f81a3b4be
Fixes: f2c2e717642c ("usb: gadget: add raw-gadget interface")
Signed-off-by: Igor Putko <igorpetindev@gmail.com>
---
v1 -> v2:
- Keep ep0 request handling in dev_free() to avoid any lock recursion.
- Synchronously disable and clean up active endpoints in gadget_unbind()
to prevent use after UDC unbind.
- Update endpoint cleanup in dev_free() to free raw_ep->desc.
---
---
drivers/usb/gadget/legacy/raw_gadget.c | 30 ++++++++++++++++++++++----
1 file changed, 26 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/gadget/legacy/raw_gadget.c b/drivers/usb/gadget/legacy/raw_gadget.c
index 4febf8dac7ca..a6b42679acc7 100644
--- a/drivers/usb/gadget/legacy/raw_gadget.c
+++ b/drivers/usb/gadget/legacy/raw_gadget.c
@@ -141,6 +141,7 @@ struct raw_ep {
enum ep_state state;
struct usb_ep *ep;
u8 addr;
+ struct usb_endpoint_descriptor *desc;
struct usb_request *req;
bool urb_queued;
bool disabling;
@@ -223,7 +224,9 @@ static void dev_free(struct kref *kref)
continue;
usb_ep_disable(dev->eps[i].ep);
usb_ep_free_request(dev->eps[i].ep, dev->eps[i].req);
- kfree(dev->eps[i].ep->desc);
+ kfree(dev->eps[i].desc);
+ dev->eps[i].desc = NULL;
+ dev->eps[i].ep->desc = NULL;
dev->eps[i].state = STATE_EP_DISABLED;
}
kfree(dev);
@@ -328,6 +331,20 @@ static int gadget_bind(struct usb_gadget *gadget,
static void gadget_unbind(struct usb_gadget *gadget)
{
struct raw_dev *dev = get_gadget_data(gadget);
+ int i;
+
+ if (dev) {
+ for (i = 0; i < dev->eps_num; i++) {
+ if (dev->eps[i].state == STATE_EP_DISABLED)
+ continue;
+ usb_ep_disable(dev->eps[i].ep);
+ usb_ep_free_request(dev->eps[i].ep, dev->eps[i].req);
+ kfree(dev->eps[i].desc);
+ dev->eps[i].desc = NULL;
+ dev->eps[i].ep->desc = NULL;
+ dev->eps[i].state = STATE_EP_DISABLED;
+ }
+ }
set_gadget_data(gadget, NULL);
/* Matches kref_get() in gadget_bind(). */
@@ -886,6 +903,7 @@ static int raw_ioctl_ep_enable(struct raw_dev *dev, unsigned long value)
if (ret < 0) {
dev_err(&dev->gadget->dev,
"fail, usb_ep_enable returned %d\n", ret);
+ ep->ep->desc = NULL;
goto out_free;
}
ep->req = usb_ep_alloc_request(ep->ep, GFP_ATOMIC);
@@ -893,10 +911,12 @@ static int raw_ioctl_ep_enable(struct raw_dev *dev, unsigned long value)
dev_err(&dev->gadget->dev,
"fail, usb_ep_alloc_request failed\n");
usb_ep_disable(ep->ep);
+ ep->ep->desc = NULL;
ret = -ENOMEM;
goto out_free;
}
ep->state = STATE_EP_ENABLED;
+ ep->desc = desc;
ep->ep->driver_data = ep;
ret = i;
goto out_unlock;
@@ -962,7 +982,9 @@ static int raw_ioctl_ep_disable(struct raw_dev *dev, unsigned long value)
spin_lock_irqsave(&dev->lock, flags);
usb_ep_free_request(dev->eps[i].ep, dev->eps[i].req);
- kfree(dev->eps[i].ep->desc);
+ kfree(dev->eps[i].desc);
+ dev->eps[i].desc = NULL;
+ dev->eps[i].ep->desc = NULL;
dev->eps[i].state = STATE_EP_DISABLED;
dev->eps[i].disabling = false;
@@ -1010,7 +1032,7 @@ static int raw_ioctl_ep_set_clear_halt_wedge(struct raw_dev *dev,
ret = -EINVAL;
goto out_unlock;
}
- if (usb_endpoint_xfer_isoc(dev->eps[i].ep->desc)) {
+ if (usb_endpoint_xfer_isoc(dev->eps[i].desc)) {
dev_dbg(&dev->gadget->dev,
"fail, can't halt/wedge ISO endpoint\n");
ret = -EINVAL;
@@ -1096,7 +1118,7 @@ static int raw_process_ep_io(struct raw_dev *dev, struct usb_raw_ep_io *io,
ret = -EBUSY;
goto out_unlock;
}
- if (in != usb_endpoint_dir_in(ep->ep->desc)) {
+ if (in != usb_endpoint_dir_in(ep->desc)) {
dev_dbg(&dev->gadget->dev, "fail, wrong direction\n");
ret = -EINVAL;
goto out_unlock;
--
2.47.3
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v2] usb: raw-gadget: fix double-free race during UDC unbind and rebind
2026-10-24 19:07 ` [PATCH v2] " Igor Putko
@ 2026-11-06 1:16 ` Andrey Konovalov
0 siblings, 0 replies; 3+ messages in thread
From: Andrey Konovalov @ 2026-11-06 1:16 UTC (permalink / raw)
To: Igor Putko
Cc: Greg Kroah-Hartman, linux-usb, linux-kernel,
syzbot+b7534c9b380f81a3b4be
On Sat, Oct 24, 2026 at 9:08 PM Igor Putko <igorpetindev@gmail.com> wrote:
>
> raw-gadget defers endpoint disabling, request freeing, and descriptor
> cleanup to dev_free(). However, dev_free() is only called after
> dev->count drops to zero in raw_release(), which happens after
> usb_gadget_unregister_driver() has already returned.
>
> Once usb_gadget_unregister_driver() finishes, the UDC can be immediately
> rebound by another raw_gadget instance. If that new instance enables an
> endpoint, it assigns its newly allocated descriptor to ep->ep->desc on
> the shared UDC endpoint. When dev_free() from the old instance runs
> concurrently, it calls usb_ep_disable() and frees dev->eps[i].ep->desc,
> which now belongs to the new instance. The new instance's usb_ep_enable()
> subsequently fails with -ESHUTDOWN, and its error path frees the descriptor
> again, causing a KASAN double-free.
>
> Tear down active endpoints and requests synchronously in gadget_unbind()
> before the UDC can be bound again. Keep the descriptor pointer inside
> struct raw_ep and clear ep->ep->desc on error and disable paths so no
> dangling pointers are left on the UDC endpoints.
>
> Reported-by: syzbot+b7534c9b380f81a3b4be@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=b7534c9b380f81a3b4be
Please also ask syzbot to test your patch - it has a reproducer for this issue.
> Fixes: f2c2e717642c ("usb: gadget: add raw-gadget interface")
> Signed-off-by: Igor Putko <igorpetindev@gmail.com>
> ---
> v1 -> v2:
> - Keep ep0 request handling in dev_free() to avoid any lock recursion.
> - Synchronously disable and clean up active endpoints in gadget_unbind()
> to prevent use after UDC unbind.
> - Update endpoint cleanup in dev_free() to free raw_ep->desc.
> ---
> ---
> drivers/usb/gadget/legacy/raw_gadget.c | 30 ++++++++++++++++++++++----
> 1 file changed, 26 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/usb/gadget/legacy/raw_gadget.c b/drivers/usb/gadget/legacy/raw_gadget.c
> index 4febf8dac7ca..a6b42679acc7 100644
> --- a/drivers/usb/gadget/legacy/raw_gadget.c
> +++ b/drivers/usb/gadget/legacy/raw_gadget.c
> @@ -141,6 +141,7 @@ struct raw_ep {
> enum ep_state state;
> struct usb_ep *ep;
> u8 addr;
> + struct usb_endpoint_descriptor *desc;
Why do we need this extra field? Why is ep->desc not enough?
> struct usb_request *req;
> bool urb_queued;
> bool disabling;
> @@ -223,7 +224,9 @@ static void dev_free(struct kref *kref)
> continue;
> usb_ep_disable(dev->eps[i].ep);
> usb_ep_free_request(dev->eps[i].ep, dev->eps[i].req);
> - kfree(dev->eps[i].ep->desc);
> + kfree(dev->eps[i].desc);
> + dev->eps[i].desc = NULL;
> + dev->eps[i].ep->desc = NULL;
> dev->eps[i].state = STATE_EP_DISABLED;
> }
> kfree(dev);
> @@ -328,6 +331,20 @@ static int gadget_bind(struct usb_gadget *gadget,
> static void gadget_unbind(struct usb_gadget *gadget)
> {
> struct raw_dev *dev = get_gadget_data(gadget);
> + int i;
> +
> + if (dev) {
> + for (i = 0; i < dev->eps_num; i++) {
> + if (dev->eps[i].state == STATE_EP_DISABLED)
> + continue;
> + usb_ep_disable(dev->eps[i].ep);
> + usb_ep_free_request(dev->eps[i].ep, dev->eps[i].req);
> + kfree(dev->eps[i].desc);
> + dev->eps[i].desc = NULL;
> + dev->eps[i].ep->desc = NULL;
> + dev->eps[i].state = STATE_EP_DISABLED;
> + }
> + }
Any of the other things dev_free() does need to be repeated here too?
Also, if we release/free all these things here during unbind, do we
still need to do the same during dev_free()?
>
> set_gadget_data(gadget, NULL);
> /* Matches kref_get() in gadget_bind(). */
> @@ -886,6 +903,7 @@ static int raw_ioctl_ep_enable(struct raw_dev *dev, unsigned long value)
> if (ret < 0) {
> dev_err(&dev->gadget->dev,
> "fail, usb_ep_enable returned %d\n", ret);
> + ep->ep->desc = NULL;
> goto out_free;
> }
> ep->req = usb_ep_alloc_request(ep->ep, GFP_ATOMIC);
> @@ -893,10 +911,12 @@ static int raw_ioctl_ep_enable(struct raw_dev *dev, unsigned long value)
> dev_err(&dev->gadget->dev,
> "fail, usb_ep_alloc_request failed\n");
> usb_ep_disable(ep->ep);
> + ep->ep->desc = NULL;
usb_ep_disable() should already do this for UDC drivers that care
about value of ep->desc for disabled endpoints.
> ret = -ENOMEM;
> goto out_free;
> }
> ep->state = STATE_EP_ENABLED;
> + ep->desc = desc;
> ep->ep->driver_data = ep;
> ret = i;
> goto out_unlock;
> @@ -962,7 +982,9 @@ static int raw_ioctl_ep_disable(struct raw_dev *dev, unsigned long value)
>
> spin_lock_irqsave(&dev->lock, flags);
> usb_ep_free_request(dev->eps[i].ep, dev->eps[i].req);
> - kfree(dev->eps[i].ep->desc);
> + kfree(dev->eps[i].desc);
> + dev->eps[i].desc = NULL;
> + dev->eps[i].ep->desc = NULL;
> dev->eps[i].state = STATE_EP_DISABLED;
> dev->eps[i].disabling = false;
>
> @@ -1010,7 +1032,7 @@ static int raw_ioctl_ep_set_clear_halt_wedge(struct raw_dev *dev,
> ret = -EINVAL;
> goto out_unlock;
> }
> - if (usb_endpoint_xfer_isoc(dev->eps[i].ep->desc)) {
> + if (usb_endpoint_xfer_isoc(dev->eps[i].desc)) {
> dev_dbg(&dev->gadget->dev,
> "fail, can't halt/wedge ISO endpoint\n");
> ret = -EINVAL;
> @@ -1096,7 +1118,7 @@ static int raw_process_ep_io(struct raw_dev *dev, struct usb_raw_ep_io *io,
> ret = -EBUSY;
> goto out_unlock;
> }
> - if (in != usb_endpoint_dir_in(ep->ep->desc)) {
> + if (in != usb_endpoint_dir_in(ep->desc)) {
> dev_dbg(&dev->gadget->dev, "fail, wrong direction\n");
> ret = -EINVAL;
> goto out_unlock;
> --
> 2.47.3
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-11-06 1:16 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-24 18:37 [PATCH] usb: raw-gadget: fix double-free race during UDC unbind and rebind Igor Putko
2026-10-24 19:07 ` [PATCH v2] " Igor Putko
2026-11-06 1:16 ` Andrey Konovalov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox