* [PATCH v8 1/4] usb: gadget: gadgetfs: fix ep0 request lifetime and buffer races
2026-11-10 4:13 [PATCH v8 0/4] usb: gadget: gadgetfs: fix ep0 request lifetime and event counter Jing Lin via B4 Relay
@ 2026-11-10 4:13 ` Jing Lin via B4 Relay
2026-11-10 4:40 ` sashiko-bot
2026-11-10 4:13 ` [PATCH v8 2/4] usb: gadget: gadgetfs: fix ep0 setup abort and unbind state races Jing Lin via B4 Relay
` (3 subsequent siblings)
4 siblings, 1 reply; 10+ messages in thread
From: Jing Lin via B4 Relay @ 2026-11-10 4:13 UTC (permalink / raw)
To: Neill Kapron, Greg Kroah-Hartman
Cc: Jing Lin, linux-usb, linux-kernel, syzbot+c73e1942197af9f54ca3,
stable
From: Jing Lin <kayano050408@gmail.com>
gadgetfs_unbind() frees the preallocated ep0 request after disconnecting
the gadget, assuming that no I/O remains active.
With controllers such as dummy_hcd, disconnecting does not necessarily
drain ep0, so a delayed userspace response can leave dev->req queued
when it is freed. This triggers the warning in dummy_free_request() and
a use-after-free when dummy_udc_stop() subsequently drains the endpoint
queue.
gadgetfs_setup() and the userspace copy helpers also race on dev->req
and req->buf:
1. ep0_read() versus gadgetfs_setup(). A delegated control OUT request
is queued by gadgetfs_setup() and completed by ep0_complete(), which
leaves the payload in dev->req->buf and sets setup_out_ready.
ep0_read() copies that payload with copy_to_user() after dropping
dev->lock, so the copy can fault and sleep. If a new control
transfer arrives during that window, gadgetfs_setup() reuses dev->req
for it; for example, GET_DESCRIPTOR(config) points req->buf at
dev->config. When ep0_read() resumes, clean_req() frees whatever
req->buf points to, which is now dev->config.
2. ep0_write() versus gadgetfs_setup(). For a delegated control IN
request, ep0_write() prepares dev->req and copies the response from
userspace with copy_from_user() after dropping dev->lock. A control
request that arrives during that copy can point req->buf at dev->dev
or dev->config, so the userspace copy overwrites a descriptor buffer,
or reset req->buf to dev->rbuf so completion calls clean_req() on the
wrong object.
Copy completed control OUT payloads into a private setup_buf in
ep0_complete(), copy control IN payloads into a stack buffer before
preparing dev->req and leaving STATE_DEV_SETUP in ep0_write(), and track
buffers allocated by setup_req() separately in dev->ep0_req_buf so
clean_req() never frees descriptor storage. Ignore zero-length reads
while a non-zero control OUT data stage is pending so they cannot queue
a status request early and clobber setup_out_len.
Submit ep0 requests through a shared queue_ep0_req() helper that tracks
dev->ep0_req_queued and wakes dev->wait whenever the flag clears. In
gadgetfs_unbind(), tear down non-ep0 endpoints first, then dequeue any
active ep0 request and wait under dev->lock with wait_event_lock_irq()
for completion to finish. If the controller rejects ep0 dequeue, keep a
reference on dev and let the completion handler free the request when
udc_stop() drains ep0.
Fixes: 1da177e4c3f41 ("Linux-2.6.12-rc2")
Reported-by: syzbot+c73e1942197af9f54ca3@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=c73e1942197af9f54ca3
Cc: stable@vger.kernel.org
Signed-off-by: Jing Lin <kayano050408@gmail.com>
---
drivers/usb/gadget/legacy/inode.c | 266 +++++++++++++++++++++++++++-----------
1 file changed, 188 insertions(+), 78 deletions(-)
diff --git a/drivers/usb/gadget/legacy/inode.c b/drivers/usb/gadget/legacy/inode.c
index 7f6d84601..1e7e8f4bb 100644
--- a/drivers/usb/gadget/legacy/inode.c
+++ b/drivers/usb/gadget/legacy/inode.c
@@ -140,6 +140,10 @@ struct dev_data {
setup_abort : 1,
gadget_registered : 1;
unsigned setup_wLength;
+ unsigned int setup_out_len;
+ bool ep0_req_queued;
+ bool ep0_req_free;
+ void *ep0_req_buf;
/* the rest is basically write-once */
struct usb_config_descriptor *config, *hs_config;
@@ -153,6 +157,7 @@ struct dev_data {
/* except this scratch i/o buffer for ep0 */
u8 rbuf[RBUF_SIZE];
+ u8 setup_buf[RBUF_SIZE];
};
static inline void get_dev (struct dev_data *data)
@@ -984,54 +989,89 @@ static inline void ep0_readable (struct dev_data *dev)
kill_fasync (&dev->fasync, SIGIO, POLL_IN);
}
-static void clean_req (struct usb_ep *ep, struct usb_request *req)
+static void dev_req_complete(struct usb_ep *ep, struct usb_request *req);
+
+static void clean_req(struct usb_ep *ep, struct usb_request *req)
{
- struct dev_data *dev = ep->driver_data;
+ struct dev_data *dev = req->context;
- if (req->buf != dev->rbuf) {
- kfree(req->buf);
- req->buf = dev->rbuf;
+ /*
+ * req->buf may also point into dev->buf, which holds the
+ * descriptors; only the buffer allocated by setup_req() belongs
+ * to the request and may be freed here.
+ */
+ kfree(dev->ep0_req_buf);
+ dev->ep0_req_buf = NULL;
+ req->buf = dev->rbuf;
+ req->complete = dev_req_complete;
+}
+
+static void complete_ep0_req(struct usb_ep *ep, struct usb_request *req,
+ struct dev_data *dev, unsigned long flags)
+{
+ bool free_req = dev->ep0_req_free;
+
+ clean_req(ep, req);
+ dev->ep0_req_queued = false;
+ dev->ep0_req_free = false;
+ wake_up(&dev->wait);
+ spin_unlock_irqrestore(&dev->lock, flags);
+
+ if (free_req) {
+ usb_ep_free_request(ep, req);
+ put_dev(dev);
}
- req->complete = epio_complete;
- dev->setup_out_ready = 0;
+}
+
+static void dev_req_complete(struct usb_ep *ep, struct usb_request *req)
+{
+ struct dev_data *dev = req->context;
+ unsigned long flags;
+
+ spin_lock_irqsave(&dev->lock, flags);
+ complete_ep0_req(ep, req, dev, flags);
}
static void ep0_complete (struct usb_ep *ep, struct usb_request *req)
{
- struct dev_data *dev = ep->driver_data;
+ struct dev_data *dev = req->context;
unsigned long flags;
- int free = 1;
/* for control OUT, data must still get to userspace */
spin_lock_irqsave(&dev->lock, flags);
- if (!dev->setup_in) {
+ if (!dev->setup_in && dev->setup_wLength) {
dev->setup_out_error = (req->status != 0);
- if (!dev->setup_out_error)
- free = 0;
+ if (dev->setup_out_error) {
+ dev->setup_out_len = 0;
+ } else {
+ dev->setup_out_len = min_t(unsigned int, req->actual,
+ sizeof(dev->setup_buf));
+ memcpy(dev->setup_buf, req->buf, dev->setup_out_len);
+ }
dev->setup_out_ready = 1;
- ep0_readable (dev);
+ ep0_readable(dev);
}
- /* clean up as appropriate */
- if (free && req->buf != &dev->rbuf)
- clean_req (ep, req);
- req->complete = epio_complete;
- spin_unlock_irqrestore(&dev->lock, flags);
+ complete_ep0_req(ep, req, dev, flags);
}
static int setup_req (struct usb_ep *ep, struct usb_request *req, u16 len)
{
struct dev_data *dev = ep->driver_data;
- if (dev->setup_out_ready) {
+ if (dev->ep0_req_queued) {
DBG (dev, "ep0 request busy!\n");
return -EBUSY;
}
- if (len > sizeof (dev->rbuf))
- req->buf = kmalloc(len, GFP_ATOMIC);
- if (req->buf == NULL) {
+ if (len > sizeof(dev->rbuf)) {
+ dev->ep0_req_buf = kmalloc(len, GFP_ATOMIC);
+ if (!dev->ep0_req_buf) {
+ req->buf = dev->rbuf;
+ return -ENOMEM;
+ }
+ req->buf = dev->ep0_req_buf;
+ } else {
req->buf = dev->rbuf;
- return -ENOMEM;
}
req->complete = ep0_complete;
req->length = len;
@@ -1039,12 +1079,42 @@ static int setup_req (struct usb_ep *ep, struct usb_request *req, u16 len)
return 0;
}
+/* Called with dev->lock held. */
+static int queue_ep0_req(struct dev_data *dev, bool process_ctx)
+{
+ struct usb_ep *ep = dev->gadget->ep0;
+ struct usb_request *req = dev->req;
+ int ret;
+
+ ++dev->udc_usage;
+ dev->ep0_req_queued = true;
+ if (process_ctx)
+ spin_unlock_irq(&dev->lock);
+ else
+ spin_unlock(&dev->lock);
+
+ ret = usb_ep_queue(ep, req, process_ctx ? GFP_KERNEL : GFP_ATOMIC);
+
+ if (process_ctx)
+ spin_lock_irq(&dev->lock);
+ else
+ spin_lock(&dev->lock);
+ --dev->udc_usage;
+ if (ret < 0) {
+ clean_req(ep, req);
+ dev->ep0_req_queued = false;
+ wake_up(&dev->wait);
+ }
+ return ret;
+}
+
static ssize_t
ep0_read (struct file *fd, char __user *buf, size_t len, loff_t *ptr)
{
struct dev_data *dev = fd->private_data;
ssize_t retval;
enum ep0_state state;
+ u8 tmp[RBUF_SIZE];
spin_lock_irq (&dev->lock);
if (dev->state <= STATE_DEV_OPENED) {
@@ -1072,14 +1142,14 @@ ep0_read (struct file *fd, char __user *buf, size_t len, loff_t *ptr)
struct usb_ep *ep = dev->gadget->ep0;
struct usb_request *req = dev->req;
- if ((retval = setup_req (ep, req, 0)) == 0) {
- ++dev->udc_usage;
- spin_unlock_irq (&dev->lock);
- retval = usb_ep_queue (ep, req, GFP_KERNEL);
- spin_lock_irq (&dev->lock);
- --dev->udc_usage;
+ if (dev->setup_wLength) {
+ retval = 0;
+ goto done;
}
+ retval = setup_req(ep, req, 0);
dev->state = STATE_DEV_CONNECTED;
+ if (retval == 0)
+ retval = queue_ep0_req(dev, true);
/* assume that was SET_CONFIGURATION */
if (dev->current_config) {
@@ -1109,25 +1179,31 @@ ep0_read (struct file *fd, char __user *buf, size_t len, loff_t *ptr)
if (retval)
goto done;
- if (dev->state != STATE_DEV_SETUP) {
+ if (dev->state != STATE_DEV_SETUP ||
+ dev->setup_in || !dev->setup_out_ready) {
retval = -ECANCELED;
goto done;
}
dev->state = STATE_DEV_CONNECTED;
- if (dev->setup_out_error)
+ if (dev->setup_out_error) {
+ dev->setup_out_ready = 0;
+ dev->setup_out_error = 0;
+ dev->setup_out_len = 0;
retval = -EIO;
- else {
- len = min (len, (size_t)dev->req->actual);
+ } else {
+ len = min_t(size_t, len, dev->setup_out_len);
+ memcpy(tmp, dev->setup_buf, len);
+ dev->setup_out_ready = 0;
+ dev->setup_out_len = 0;
++dev->udc_usage;
spin_unlock_irq(&dev->lock);
- if (copy_to_user (buf, dev->req->buf, len))
+ if (copy_to_user(buf, tmp, len))
retval = -EFAULT;
else
retval = len;
spin_lock_irq(&dev->lock);
--dev->udc_usage;
- clean_req (dev->gadget->ep0, dev->req);
/* NOTE userspace can't yet choose to stall */
}
}
@@ -1267,29 +1343,28 @@ ep0_write (struct file *fd, const char __user *buf, size_t len, loff_t *ptr)
len = min_t(size_t, len, dev->setup_wLength);
if (dev->setup_in) {
- retval = setup_req (dev->gadget->ep0, dev->req, len);
+ u8 tmp[RBUF_SIZE];
+ bool zero = len < dev->setup_wLength;
+
+ spin_unlock_irq(&dev->lock);
+ if (copy_from_user(tmp, buf, len)) {
+ spin_lock_irq(&dev->lock);
+ return -EFAULT;
+ }
+ spin_lock_irq(&dev->lock);
+ if (dev->state != STATE_DEV_SETUP || !dev->setup_in)
+ return -ECANCELED;
+
+ retval = setup_req(dev->gadget->ep0, dev->req, len);
if (retval == 0) {
dev->state = STATE_DEV_CONNECTED;
- ++dev->udc_usage;
- spin_unlock_irq (&dev->lock);
- if (copy_from_user (dev->req->buf, buf, len))
- retval = -EFAULT;
- else {
- if (len < dev->setup_wLength)
- dev->req->zero = 1;
- retval = usb_ep_queue (
- dev->gadget->ep0, dev->req,
- GFP_KERNEL);
- }
- spin_lock_irq(&dev->lock);
- --dev->udc_usage;
- if (retval < 0) {
- clean_req (dev->gadget->ep0, dev->req);
- } else
+ memcpy(dev->req->buf, tmp, len);
+ dev->req->zero = zero;
+ retval = queue_ep0_req(dev, true);
+ if (retval == 0)
retval = len;
-
- return retval;
}
+ return retval;
/* can stall some OUT transfers */
} else if (dev->setup_can_stall) {
@@ -1483,6 +1558,10 @@ gadgetfs_setup (struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
}
spin_lock (&dev->lock);
+ if (dev->ep0_req_queued) {
+ spin_unlock(&dev->lock);
+ return -EBUSY;
+ }
dev->setup_abort = 0;
if (dev->state == STATE_DEV_UNCONNECTED) {
if (gadget_is_dualspeed(gadget)
@@ -1509,7 +1588,6 @@ gadgetfs_setup (struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
dev->setup_abort = 1;
req->buf = dev->rbuf;
- req->context = NULL;
switch (ctrl->bRequest) {
case USB_REQ_GET_DESCRIPTOR:
@@ -1618,24 +1696,18 @@ gadgetfs_setup (struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
dev->setup_wLength = w_length;
dev->setup_out_ready = 0;
dev->setup_out_error = 0;
+ dev->setup_out_len = 0;
/* read DATA stage for OUT right away */
if (unlikely (!dev->setup_in && w_length)) {
- value = setup_req (gadget->ep0, dev->req,
- w_length);
+ value = setup_req(gadget->ep0, dev->req,
+ w_length);
if (value < 0)
break;
- ++dev->udc_usage;
- spin_unlock (&dev->lock);
- value = usb_ep_queue (gadget->ep0, dev->req,
- GFP_KERNEL);
- spin_lock (&dev->lock);
- --dev->udc_usage;
- if (value < 0) {
- clean_req (gadget->ep0, dev->req);
+ value = queue_ep0_req(dev, false);
+ if (value < 0)
break;
- }
/* we can't currently stall these */
dev->setup_can_stall = 0;
@@ -1663,21 +1735,19 @@ gadgetfs_setup (struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
if (value >= 0 && dev->state != STATE_DEV_SETUP) {
req->length = value;
req->zero = value < w_length;
+ req->complete = dev_req_complete;
- ++dev->udc_usage;
- spin_unlock (&dev->lock);
- value = usb_ep_queue (gadget->ep0, req, GFP_KERNEL);
- spin_lock(&dev->lock);
- --dev->udc_usage;
- spin_unlock(&dev->lock);
+ value = queue_ep0_req(dev, false);
if (value < 0) {
- DBG (dev, "ep_queue --> %d\n", value);
+ DBG(dev, "ep_queue --> %d\n", value);
req->status = 0;
}
+ spin_unlock(&dev->lock);
return value;
}
/* device stalls when value < 0 */
+ req->buf = dev->rbuf;
spin_unlock (&dev->lock);
return value;
}
@@ -1771,6 +1841,7 @@ static void
gadgetfs_unbind (struct usb_gadget *gadget)
{
struct dev_data *dev = get_gadget_data (gadget);
+ struct usb_request *req;
DBG (dev, "%s\n", __func__);
@@ -1784,12 +1855,48 @@ gadgetfs_unbind (struct usb_gadget *gadget)
spin_unlock_irq (&dev->lock);
destroy_ep_files (dev);
+
+ spin_lock_irq(&dev->lock);
+ req = dev->req;
+ dev->req = NULL;
+ if (req) {
+ if (dev->ep0_req_queued) {
+ int ret;
+
+ spin_unlock_irq(&dev->lock);
+ ret = usb_ep_dequeue(gadget->ep0, req);
+ spin_lock_irq(&dev->lock);
+ if (!dev->ep0_req_queued || ret == 0) {
+ /*
+ * usb_ep_dequeue() may return before the
+ * completion routine runs; wait under
+ * dev->lock so completion finishes first.
+ */
+ wait_event_lock_irq(dev->wait,
+ !dev->ep0_req_queued,
+ dev->lock);
+ } else {
+ /*
+ * Some UDCs cannot dequeue ep0 requests and
+ * drain ep0 in udc_stop() after unbind()
+ * returns; let completion free req then.
+ */
+ dev->ep0_req_free = true;
+ get_dev(dev);
+ req = NULL;
+ }
+ }
+ if (req)
+ clean_req(gadget->ep0, req);
+ }
+ spin_unlock_irq(&dev->lock);
+
+ if (req)
+ usb_ep_free_request(gadget->ep0, req);
+
gadget->ep0->driver_data = NULL;
set_gadget_data (gadget, NULL);
- /* we've already been disconnected ... no i/o is active */
- if (dev->req)
- usb_ep_free_request (gadget->ep0, dev->req);
DBG (dev, "%s done\n", __func__);
put_dev (dev);
}
@@ -1816,11 +1923,14 @@ static int gadgetfs_bind(struct usb_gadget *gadget,
get_dev (dev);
/* preallocate control response and buffer */
+ dev->ep0_req_queued = false;
+ dev->ep0_req_free = false;
dev->req = usb_ep_alloc_request (gadget->ep0, GFP_KERNEL);
if (!dev->req)
goto enomem;
- dev->req->context = NULL;
- dev->req->complete = epio_complete;
+ dev->req->context = dev;
+ dev->req->complete = dev_req_complete;
+ dev->req->buf = dev->rbuf;
if (activate_ep_files (dev) < 0)
goto enomem;
--
Git-155)
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v8 2/4] usb: gadget: gadgetfs: fix ep0 setup abort and unbind state races
2026-11-10 4:13 [PATCH v8 0/4] usb: gadget: gadgetfs: fix ep0 request lifetime and event counter Jing Lin via B4 Relay
2026-11-10 4:13 ` [PATCH v8 1/4] usb: gadget: gadgetfs: fix ep0 request lifetime and buffer races Jing Lin via B4 Relay
@ 2026-11-10 4:13 ` Jing Lin via B4 Relay
2026-11-10 4:36 ` sashiko-bot
2026-11-10 4:13 ` [PATCH v8 3/4] usb: gadget: gadgetfs: prevent ep0 event counter underflow Jing Lin via B4 Relay
` (2 subsequent siblings)
4 siblings, 1 reply; 10+ messages in thread
From: Jing Lin via B4 Relay @ 2026-11-10 4:13 UTC (permalink / raw)
To: Neill Kapron, Greg Kroah-Hartman
Cc: Jing Lin, linux-usb, linux-kernel, stable
From: Jing Lin <kayano050408@gmail.com>
Several ep0 state transitions lose setup abort notifications or leave
the file descriptor in an inconsistent state when control requests,
disconnects, or unbinds race with userspace I/O:
1. ep0_write() drops dev->lock while copying an IN payload from
userspace. If a new SETUP packet arrives in that window, the stale
response must not be queued for the new transfer.
2. ep0_read() waiting for a delegated control OUT data stage only checks
dev->setup_out_ready, so it does not wake when a new SETUP packet,
disconnect, or unbind aborts the transfer, and can consume a newer
transfer's payload if dev->state re-enters STATE_DEV_SETUP. In
addition, gadgetfs_disconnect() overwrites STATE_DEV_UNCONNECTED
before next_event() checks for STATE_DEV_SETUP, skipping setup_abort.
3. Early returns in gadgetfs_setup() on oversized OUT requests or a busy
ep0 request skip setting setup_abort when aborting STATE_DEV_SETUP,
while ep0_poll() clears setup_abort before ep0_read() or ep0_write()
can report -EIDRM.
4. gadgetfs_unbind() leaves queued events readable in STATE_DEV_UNBOUND.
Delivering a queued GADGETFS_SETUP event after unbind puts dev->state
back into STATE_DEV_SETUP after dev->req and ep0->driver_data have
been cleared, causing a NULL pointer dereference on the next ep0 I/O.
Count setup callbacks in dev->setup_generation, record setup_abort and
return from STATE_DEV_SETUP to STATE_DEV_CONNECTED at the start of
gadgetfs_setup(), and consume setup_abort only in ep0_read() and
ep0_write() via ep0_check_abort(). Include setup_abort, state changes,
and setup_generation in the ep0_read() wait condition, fix the state
update order in gadgetfs_disconnect(), and return -ESRCH from ep0_read()
once the device is unbound.
A new SETUP also supersedes a delegated request whose GADGETFS_SETUP
event userspace has not collected yet. Discard that queued event before
any early return or in-kernel handling, so a later ep0_read() cannot
re-enter STATE_DEV_SETUP for a request the host has already abandoned.
Fixes: 1da177e4c3f41 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Signed-off-by: Jing Lin <kayano050408@gmail.com>
---
drivers/usb/gadget/legacy/inode.c | 139 ++++++++++++++++++++++++++++----------
1 file changed, 105 insertions(+), 34 deletions(-)
diff --git a/drivers/usb/gadget/legacy/inode.c b/drivers/usb/gadget/legacy/inode.c
index 1e7e8f4bb..d2e148054 100644
--- a/drivers/usb/gadget/legacy/inode.c
+++ b/drivers/usb/gadget/legacy/inode.c
@@ -144,6 +144,7 @@ struct dev_data {
bool ep0_req_queued;
bool ep0_req_free;
void *ep0_req_buf;
+ unsigned int setup_generation;
/* the rest is basically write-once */
struct usb_config_descriptor *config, *hs_config;
@@ -1108,6 +1109,23 @@ static int queue_ep0_req(struct dev_data *dev, bool process_ctx)
return ret;
}
+/*
+ * Consume a pending setup abort and report it to userspace. Called with
+ * dev->lock held. Return to the connected state so that the next event
+ * read is not misdirected into the control data path, without overwriting
+ * a state that disconnect or unbind already changed.
+ */
+static bool ep0_check_abort(struct dev_data *dev)
+{
+ if (!dev->setup_abort)
+ return false;
+
+ dev->setup_abort = 0;
+ if (dev->state == STATE_DEV_SETUP)
+ dev->state = STATE_DEV_CONNECTED;
+ return true;
+}
+
static ssize_t
ep0_read (struct file *fd, char __user *buf, size_t len, loff_t *ptr)
{
@@ -1123,8 +1141,7 @@ ep0_read (struct file *fd, char __user *buf, size_t len, loff_t *ptr)
}
/* report fd mode change before acting on it */
- if (dev->setup_abort) {
- dev->setup_abort = 0;
+ if (ep0_check_abort(dev)) {
retval = -EIDRM;
goto done;
}
@@ -1165,6 +1182,8 @@ ep0_read (struct file *fd, char __user *buf, size_t len, loff_t *ptr)
}
} else { /* collect OUT data */
+ unsigned int generation = dev->setup_generation;
+
if ((fd->f_flags & O_NONBLOCK) != 0
&& !dev->setup_out_ready) {
retval = -EAGAIN;
@@ -1172,13 +1191,20 @@ ep0_read (struct file *fd, char __user *buf, size_t len, loff_t *ptr)
}
spin_unlock_irq (&dev->lock);
retval = wait_event_interruptible (dev->wait,
- dev->setup_out_ready != 0);
+ dev->setup_out_ready ||
+ dev->setup_abort ||
+ dev->state != STATE_DEV_SETUP ||
+ dev->setup_generation != generation);
- /* FIXME state could change from under us */
spin_lock_irq (&dev->lock);
if (retval)
goto done;
+ if (ep0_check_abort(dev) ||
+ dev->setup_generation != generation) {
+ retval = -EIDRM;
+ goto done;
+ }
if (dev->state != STATE_DEV_SETUP ||
dev->setup_in || !dev->setup_out_ready) {
retval = -ECANCELED;
@@ -1219,6 +1245,11 @@ ep0_read (struct file *fd, char __user *buf, size_t len, loff_t *ptr)
dev->usermode_setup = 1;
scan:
+ if (dev->state == STATE_DEV_UNBOUND) {
+ retval = -ESRCH;
+ goto done;
+ }
+
/* return queued events right away */
if (dev->ev_next != 0) {
unsigned i, n;
@@ -1273,7 +1304,8 @@ ep0_read (struct file *fd, char __user *buf, size_t len, loff_t *ptr)
/* wait for events */
retval = wait_event_interruptible (dev->wait,
- dev->ev_next != 0);
+ dev->ev_next != 0 ||
+ dev->state == STATE_DEV_UNBOUND);
if (retval < 0)
return retval;
spin_lock_irq (&dev->lock);
@@ -1285,11 +1317,29 @@ ep0_read (struct file *fd, char __user *buf, size_t len, loff_t *ptr)
return retval;
}
+/* Called with dev->lock held. Drop a queued event of @type, if any. */
+static void ep0_discard_event(struct dev_data *dev,
+ enum usb_gadgetfs_event_type type)
+{
+ unsigned int i;
+
+ for (i = 0; i != dev->ev_next; i++) {
+ if (dev->event[i].type != type)
+ continue;
+ DBG(dev, "discard old event[%d] %d\n", i, type);
+ dev->ev_next--;
+ if (i == dev->ev_next)
+ break;
+ /* indices start at zero, for simplicity */
+ memmove(&dev->event[i], &dev->event[i + 1],
+ sizeof(struct usb_gadgetfs_event) * (dev->ev_next - i));
+ }
+}
+
static struct usb_gadgetfs_event *
next_event (struct dev_data *dev, enum usb_gadgetfs_event_type type)
{
struct usb_gadgetfs_event *event;
- unsigned i;
switch (type) {
/* these events purge the queue */
@@ -1303,18 +1353,7 @@ next_event (struct dev_data *dev, enum usb_gadgetfs_event_type type)
case GADGETFS_SETUP: /* previous request timed out */
case GADGETFS_SUSPEND: /* same effect */
/* these events can't be repeated */
- for (i = 0; i != dev->ev_next; i++) {
- if (dev->event [i].type != type)
- continue;
- DBG(dev, "discard old event[%d] %d\n", i, type);
- dev->ev_next--;
- if (i == dev->ev_next)
- break;
- /* indices start at zero, for simplicity */
- memmove (&dev->event [i], &dev->event [i + 1],
- sizeof (struct usb_gadgetfs_event)
- * (dev->ev_next - i));
- }
+ ep0_discard_event(dev, type);
break;
default:
BUG ();
@@ -1334,8 +1373,7 @@ ep0_write (struct file *fd, const char __user *buf, size_t len, loff_t *ptr)
ssize_t retval = -ESRCH;
/* report fd mode change before acting on it */
- if (dev->setup_abort) {
- dev->setup_abort = 0;
+ if (ep0_check_abort(dev)) {
retval = -EIDRM;
/* data and/or status stage for control request */
@@ -1345,6 +1383,7 @@ ep0_write (struct file *fd, const char __user *buf, size_t len, loff_t *ptr)
if (dev->setup_in) {
u8 tmp[RBUF_SIZE];
bool zero = len < dev->setup_wLength;
+ unsigned int generation = dev->setup_generation;
spin_unlock_irq(&dev->lock);
if (copy_from_user(tmp, buf, len)) {
@@ -1352,6 +1391,14 @@ ep0_write (struct file *fd, const char __user *buf, size_t len, loff_t *ptr)
return -EFAULT;
}
spin_lock_irq(&dev->lock);
+ /*
+ * A control request that arrived while dev->lock was
+ * dropped supersedes this data stage; do not queue the
+ * stale response for it.
+ */
+ if (ep0_check_abort(dev) ||
+ dev->setup_generation != generation)
+ return -EIDRM;
if (dev->state != STATE_DEV_SETUP || !dev->setup_in)
return -ECANCELED;
@@ -1434,9 +1481,13 @@ ep0_poll (struct file *fd, poll_table *wait)
spin_lock_irq(&dev->lock);
- /* report fd mode change before acting on it */
+ if (dev->state == STATE_DEV_UNBOUND) {
+ mask = EPOLLHUP;
+ goto out;
+ }
+
+ /* report fd mode change without consuming it before read/write */
if (dev->setup_abort) {
- dev->setup_abort = 0;
mask = EPOLLHUP;
goto out;
}
@@ -1545,6 +1596,30 @@ gadgetfs_setup (struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
u16 w_value = le16_to_cpu(ctrl->wValue);
u16 w_length = le16_to_cpu(ctrl->wLength);
+ spin_lock(&dev->lock);
+ /* invalidate userspace copies started for an older control request */
+ dev->setup_generation++;
+
+ /*
+ * If userspace has not collected the previous GADGETFS_SETUP event,
+ * it describes a request this SETUP supersedes. Drop it before any
+ * early return or in-kernel handling so a later ep0_read() cannot
+ * re-enter STATE_DEV_SETUP for that request. If userspace did
+ * collect it, the state check below reports the abort.
+ */
+ ep0_discard_event(dev, GADGETFS_SETUP);
+
+ /*
+ * Host may have given up waiting for a response to a previous
+ * delegated transfer. Record the abort and return to the connected
+ * state before any early return or in-kernel response handling.
+ */
+ if (dev->state == STATE_DEV_SETUP) {
+ dev->setup_abort = 1;
+ dev->state = STATE_DEV_CONNECTED;
+ ep0_readable(dev);
+ }
+
if (w_length > RBUF_SIZE) {
if (ctrl->bRequestType & USB_DIR_IN) {
/* Cast away the const, we are going to overwrite on purpose. */
@@ -1553,16 +1628,15 @@ gadgetfs_setup (struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
*temp = cpu_to_le16(RBUF_SIZE);
w_length = RBUF_SIZE;
} else {
+ spin_unlock(&dev->lock);
return value;
}
}
- spin_lock (&dev->lock);
if (dev->ep0_req_queued) {
spin_unlock(&dev->lock);
return -EBUSY;
}
- dev->setup_abort = 0;
if (dev->state == STATE_DEV_UNCONNECTED) {
if (gadget_is_dualspeed(gadget)
&& gadget->speed == USB_SPEED_HIGH
@@ -1578,14 +1652,7 @@ gadgetfs_setup (struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
event = next_event (dev, GADGETFS_CONNECT);
event->u.speed = gadget->speed;
ep0_readable (dev);
-
- /* host may have given up waiting for response. we can miss control
- * requests handled lower down (device/endpoint status and features);
- * then ep0_{read,write} will report the wrong status. controller
- * driver will have aborted pending i/o.
- */
- } else if (dev->state == STATE_DEV_SETUP)
- dev->setup_abort = 1;
+ }
req->buf = dev->rbuf;
switch (ctrl->bRequest) {
@@ -1847,6 +1914,8 @@ gadgetfs_unbind (struct usb_gadget *gadget)
spin_lock_irq (&dev->lock);
dev->state = STATE_DEV_UNBOUND;
+ dev->setup_abort = 0;
+ ep0_readable(dev);
while (dev->udc_usage > 0) {
spin_unlock_irq(&dev->lock);
usleep_range(1000, 2000);
@@ -1953,12 +2022,13 @@ gadgetfs_disconnect (struct usb_gadget *gadget)
unsigned long flags;
spin_lock_irqsave (&dev->lock, flags);
- if (dev->state == STATE_DEV_UNCONNECTED)
+ if (dev->state == STATE_DEV_UNCONNECTED ||
+ dev->state == STATE_DEV_UNBOUND)
goto exit;
- dev->state = STATE_DEV_UNCONNECTED;
INFO (dev, "disconnected\n");
next_event (dev, GADGETFS_DISCONNECT);
+ dev->state = STATE_DEV_UNCONNECTED;
ep0_readable (dev);
exit:
spin_unlock_irqrestore (&dev->lock, flags);
@@ -2161,6 +2231,7 @@ gadget_dev_open (struct inode *inode, struct file *fd)
spin_lock_irq(&dev->lock);
if (dev->state == STATE_DEV_DISABLED) {
dev->ev_next = 0;
+ dev->setup_abort = 0;
dev->state = STATE_DEV_OPENED;
fd->private_data = dev;
get_dev (dev);
--
Git-155)
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v8 2/4] usb: gadget: gadgetfs: fix ep0 setup abort and unbind state races
2026-11-10 4:13 ` [PATCH v8 2/4] usb: gadget: gadgetfs: fix ep0 setup abort and unbind state races Jing Lin via B4 Relay
@ 2026-11-10 4:36 ` sashiko-bot
0 siblings, 0 replies; 10+ messages in thread
From: sashiko-bot @ 2026-11-10 4:36 UTC (permalink / raw)
To: Jing Lin; +Cc: linux-usb
> From: Jing Lin <kayano050408@gmail.com>
>
> Several ep0 state transitions lose setup abort notifications or leave
> the file descriptor in an inconsistent state when control requests,
> disconnects, or unbinds race with userspace I/O:
>
> 1. ep0_write() drops dev->lock while copying an IN payload from
> userspace. If a new SETUP packet arrives in that window, the stale
> response must not be queued for the new transfer.
>
> 2. ep0_read() waiting for a delegated control OUT data stage only checks
> dev->setup_out_ready, so it does not wake when a new SETUP packet,
> disconnect, or unbind aborts the transfer, and can consume a newer
> transfer's payload if dev->state re-enters STATE_DEV_SETUP. In
> addition, gadgetfs_disconnect() overwrites STATE_DEV_UNCONNECTED
> [ ... ]
> re-enter STATE_DEV_SETUP for a request the host has already abandoned.
>
> Fixes: 1da177e4c3f41 ("Linux-2.6.12-rc2")
> Cc: stable@vger.kernel.org
> Signed-off-by: Jing Lin <kayano050408@gmail.com>
Sashiko has reviewed this patch and found no issues.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261110-gadgetfs-ep0-v8-v8-0-8f90434574fe@gmail.com?part=2
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v8 3/4] usb: gadget: gadgetfs: prevent ep0 event counter underflow
2026-11-10 4:13 [PATCH v8 0/4] usb: gadget: gadgetfs: fix ep0 request lifetime and event counter Jing Lin via B4 Relay
2026-11-10 4:13 ` [PATCH v8 1/4] usb: gadget: gadgetfs: fix ep0 request lifetime and buffer races Jing Lin via B4 Relay
2026-11-10 4:13 ` [PATCH v8 2/4] usb: gadget: gadgetfs: fix ep0 setup abort and unbind state races Jing Lin via B4 Relay
@ 2026-11-10 4:13 ` Jing Lin via B4 Relay
2026-11-10 4:34 ` sashiko-bot
2026-11-10 4:13 ` [PATCH v8 4/4] usb: gadget: gadgetfs: avoid writing through the const setup pointer Jing Lin via B4 Relay
2026-11-10 22:49 ` [PATCH v8 0/4] usb: gadget: gadgetfs: fix ep0 request lifetime and event counter Alan Stern
4 siblings, 1 reply; 10+ messages in thread
From: Jing Lin via B4 Relay @ 2026-11-10 4:13 UTC (permalink / raw)
To: Neill Kapron, Greg Kroah-Hartman
Cc: Jing Lin, linux-usb, linux-kernel, stable
From: Jing Lin <kayano050408@gmail.com>
ep0_read() snapshots dev->ev_next, drops dev->lock while copy_to_user()
runs directly from dev->event[], and subtracts the snapshot afterwards.
A concurrent disconnect, connect, setup, or another reader can reset,
deduplicate, or consume the queue in that window. This races on
dev->event[], can drop newly arrived GADGETFS_SETUP events, and wraps
dev->ev_next if the queue shrinks below the snapshot.
Dequeue the events into the local buffer and update dev->ev_next before
dropping dev->lock for copy_to_user().
Fixes: 1da177e4c3f41 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Signed-off-by: Jing Lin <kayano050408@gmail.com>
---
drivers/usb/gadget/legacy/inode.c | 26 ++++++++++----------------
1 file changed, 10 insertions(+), 16 deletions(-)
diff --git a/drivers/usb/gadget/legacy/inode.c b/drivers/usb/gadget/legacy/inode.c
index d2e148054..d15739df2 100644
--- a/drivers/usb/gadget/legacy/inode.c
+++ b/drivers/usb/gadget/legacy/inode.c
@@ -1266,25 +1266,19 @@ ep0_read (struct file *fd, char __user *buf, size_t len, loff_t *ptr)
break;
}
}
- spin_unlock_irq (&dev->lock);
- len = n * sizeof (struct usb_gadgetfs_event);
- if (copy_to_user (buf, &dev->event, len))
+ len = n * sizeof(struct usb_gadgetfs_event);
+ memcpy(tmp, &dev->event, len);
+ if (dev->ev_next > n) {
+ memmove(&dev->event[0], &dev->event[n],
+ sizeof(struct usb_gadgetfs_event)
+ * (dev->ev_next - n));
+ }
+ dev->ev_next -= n;
+ spin_unlock_irq(&dev->lock);
+ if (copy_to_user(buf, tmp, len))
retval = -EFAULT;
else
retval = len;
- if (len > 0) {
- /* NOTE this doesn't guard against broken drivers;
- * concurrent ep0 readers may lose events.
- */
- spin_lock_irq (&dev->lock);
- if (dev->ev_next > n) {
- memmove(&dev->event[0], &dev->event[n],
- sizeof (struct usb_gadgetfs_event)
- * (dev->ev_next - n));
- }
- dev->ev_next -= n;
- spin_unlock_irq (&dev->lock);
- }
return retval;
}
if (fd->f_flags & O_NONBLOCK) {
--
Git-155)
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v8 3/4] usb: gadget: gadgetfs: prevent ep0 event counter underflow
2026-11-10 4:13 ` [PATCH v8 3/4] usb: gadget: gadgetfs: prevent ep0 event counter underflow Jing Lin via B4 Relay
@ 2026-11-10 4:34 ` sashiko-bot
0 siblings, 0 replies; 10+ messages in thread
From: sashiko-bot @ 2026-11-10 4:34 UTC (permalink / raw)
To: Jing Lin; +Cc: linux-usb
> From: Jing Lin <kayano050408@gmail.com>
>
> ep0_read() snapshots dev->ev_next, drops dev->lock while copy_to_user()
> runs directly from dev->event[], and subtracts the snapshot afterwards.
> A concurrent disconnect, connect, setup, or another reader can reset,
> deduplicate, or consume the queue in that window. This races on
> dev->event[], can drop newly arrived GADGETFS_SETUP events, and wraps
> dev->ev_next if the queue shrinks below the snapshot.
>
> Dequeue the events into the local buffer and update dev->ev_next before
> dropping dev->lock for copy_to_user().
>
> Fixes: 1da177e4c3f41 ("Linux-2.6.12-rc2")
> Cc: stable@vger.kernel.org
> Signed-off-by: Jing Lin <kayano050408@gmail.com>
Sashiko has reviewed this patch and found no issues.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261110-gadgetfs-ep0-v8-v8-0-8f90434574fe@gmail.com?part=3
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v8 4/4] usb: gadget: gadgetfs: avoid writing through the const setup pointer
2026-11-10 4:13 [PATCH v8 0/4] usb: gadget: gadgetfs: fix ep0 request lifetime and event counter Jing Lin via B4 Relay
` (2 preceding siblings ...)
2026-11-10 4:13 ` [PATCH v8 3/4] usb: gadget: gadgetfs: prevent ep0 event counter underflow Jing Lin via B4 Relay
@ 2026-11-10 4:13 ` Jing Lin via B4 Relay
2026-11-10 4:18 ` sashiko-bot
2026-11-10 22:49 ` [PATCH v8 0/4] usb: gadget: gadgetfs: fix ep0 request lifetime and event counter Alan Stern
4 siblings, 1 reply; 10+ messages in thread
From: Jing Lin via B4 Relay @ 2026-11-10 4:13 UTC (permalink / raw)
To: Neill Kapron, Greg Kroah-Hartman
Cc: Jing Lin, linux-usb, linux-kernel, Alan Stern
From: Jing Lin <kayano050408@gmail.com>
gadgetfs_setup() receives the setup packet as const, but clamps an
oversized control IN request by casting away the constness and writing
the truncated length back into the caller's wLength field.
The setup() callback has no business modifying the setup packet, and
ctrl->wLength is only used afterwards when initializing event->u.setup.
Store the clamped w_length into event->u.setup.wLength directly instead.
Suggested-by: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Jing Lin <kayano050408@gmail.com>
---
drivers/usb/gadget/legacy/inode.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/gadget/legacy/inode.c b/drivers/usb/gadget/legacy/inode.c
index d15739df2..12f04329b 100644
--- a/drivers/usb/gadget/legacy/inode.c
+++ b/drivers/usb/gadget/legacy/inode.c
@@ -1616,10 +1616,6 @@ gadgetfs_setup (struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
if (w_length > RBUF_SIZE) {
if (ctrl->bRequestType & USB_DIR_IN) {
- /* Cast away the const, we are going to overwrite on purpose. */
- __le16 *temp = (__le16 *)&ctrl->wLength;
-
- *temp = cpu_to_le16(RBUF_SIZE);
w_length = RBUF_SIZE;
} else {
spin_unlock(&dev->lock);
@@ -1777,6 +1773,8 @@ gadgetfs_setup (struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
/* state changes when reader collects event */
event = next_event (dev, GADGETFS_SETUP);
event->u.setup = *ctrl;
+ /* Report the length we can actually handle */
+ event->u.setup.wLength = cpu_to_le16(w_length);
ep0_readable (dev);
spin_unlock (&dev->lock);
/*
--
Git-155)
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v8 0/4] usb: gadget: gadgetfs: fix ep0 request lifetime and event counter
2026-11-10 4:13 [PATCH v8 0/4] usb: gadget: gadgetfs: fix ep0 request lifetime and event counter Jing Lin via B4 Relay
` (3 preceding siblings ...)
2026-11-10 4:13 ` [PATCH v8 4/4] usb: gadget: gadgetfs: avoid writing through the const setup pointer Jing Lin via B4 Relay
@ 2026-11-10 22:49 ` Alan Stern
4 siblings, 0 replies; 10+ messages in thread
From: Alan Stern @ 2026-11-10 22:49 UTC (permalink / raw)
To: kayano050408
Cc: Neill Kapron, Greg Kroah-Hartman, linux-usb, linux-kernel,
syzbot+c73e1942197af9f54ca3, stable
On Tue, Nov 10, 2026 at 12:13:21PM +0800, Jing Lin via B4 Relay wrote:
> This series fixes the ep0 request lifetime and shared request buffer
> races in gadgetfs, and the pre-existing event counter underflow.
>
> The first patch covers request lifetime and buffer ownership, the second
> the ep0 setup abort and unbind state races, the third the event counter
> underflow, and the fourth removes the const cast in gadgetfs_setup().
>
> The setup() callback runs in interrupt context and may not sleep, so a
> mutex cannot be used to serialize it against ep0_read() and ep0_write().
> The shared request state remains protected by dev->lock and the control
> payloads are handed off through private buffers.
>
> The series was tested with the syzbot reproducer and with targeted
> lifecycle, request-reuse, abort, unbind, stale-setup, and event-counter
> tests on arm64 QEMU with dummy_hcd, KASAN, and lockdep. The syzbot
> reproducer still exits 137 on the fixed kernel without a BUG, WARNING,
> KASAN report, or lockdep splat.
>
> Signed-off-by: Jing Lin <kayano050408@gmail.com>
> ---
I'm going to need some time to refresh my memory on how all this stuff
works in the current driver before I can review these patches properly.
And since I'm going to be very busy during the next few days with other
matters, it will be a while before I can reply.
But first, one thought about coordinating setup packets and their
replies. It seems that there's an unavoidable race between a gadget
driver queuing a request in response to an old setup packet and the UDC
driver invoking the gadget driver's ->setup() callback for a newly
arrived setup packet:
UDC driver Gadget driver
---------- -------------
Setup packet arrives
Call ->setup()
->setup() returns DELAYED_STATUS
Prepare proper reply to IN request
or carry out actions needed for
OUT request
New setup packet arrives
Call ->setup Call usb_ep_queue() with reply to
or status for first setup
At this point there's no way for the UDC driver to know whether the
request it was just asked to queue is a response to the old setup packet
or the new one. This is a real flaw in the gadget API.
I've often thought that UDC drivers should assign a sequential ID number
to each setup packet and pass this number to the ->setup() callback, and
gadget drivers should include the number in their response so that the
UDC driver can tell whether the response is obsolete. Too late to
change the API now, unfortunately.
Alan Stern
^ permalink raw reply [flat|nested] 10+ messages in thread