[PATCH v3 0/2] usb: gadget: gadgetfs: fix ep0 request lifetime and event counter

[PATCH v3 0/2] usb: gadget: gadgetfs: fix ep0 request lifetime and event counter

[Jing Lin via B4 Relay]

gadgetfs_unbind() frees the preallocated ep0 request after disconnecting
the gadget, assuming that no I/O remains active. With dummy_hcd, a
delayed userspace response can leave the request queued when it is freed,
which triggers the warning in dummy_free_request() and a use-after-free
when dummy_udc_stop() drains the endpoint queue.

gadgetfs_setup() also reuses dev->req for a new control request while a
completed delegated OUT transfer still has setup_out_ready set waiting
for ep0_read() to copy the payload to userspace. This overwrites req->buf
with a descriptor pointer, so the later clean_req() frees the descriptor
buffer instead of the request buffer.

Keep the pending OUT payload in a separate buffer, track whether the
request is queued, dequeue it before freeing, and do not leave ep0 busy
after a zero-length status stage. The second patch prevents the ep0 event
counter from underflowing when a concurrent disconnect or connect purges
the queue while ep0_read() copies events to userspace.

Changes in v3:
- Replace the v2 -EBUSY rejection with a separate buffer for pending OUT
  data, so a new SETUP can reuse dev->req without overwriting unread data
  or leaving ep0 permanently busy after a zero-length status stage.
- Dequeue a pending ep0 request before freeing it in gadgetfs_unbind().
- Add a separate patch preventing the ep0 event counter underflow.
- Link to v2: https://patch.msgid.link/20261107-b4-gadgetfs-ep0-v2-1-d8896e1ad517@gmail.com

Signed-off-by: Jing Lin <kayano050408@gmail.com>
---
Jing Lin (2):
      usb: gadget: gadgetfs: fix ep0 request lifetime on disconnect
      usb: gadget: gadgetfs: prevent ep0 event counter underflow

 drivers/usb/gadget/legacy/inode.c | 102 ++++++++++++++++++++++++++++----------
 1 file changed, 77 insertions(+), 25 deletions(-)
---
base-commit: f66469d9905b39d2c260a15d7d6db450f4db67c1
change-id: 20261106-b4-gadgetfs-ep0-f814c14717a9

Best regards,
--  
Jing Lin <kayano050408@gmail.com>

[PATCH v3 1/2] usb: gadget: gadgetfs: fix ep0 request lifetime on disconnect

[Jing Lin via B4 Relay]

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 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() also reuses dev->req for a new control request while the
completed delegated OUT transfer still has setup_out_ready set waiting
for ep0_read() to copy the payload to userspace. Reusing the request
overwrites req->buf with a descriptor pointer, so the later clean_req()
frees the descriptor buffer instead of the request buffer and the gadget
keeps using freed memory.

Keep the pending OUT payload in a separate buffer so the request can be
reused immediately, and clear the pending state from ep0_read(). Track
whether dev->req is queued and dequeue it without dev->lock before
freeing it in gadgetfs_unbind(). Do not reject new control requests while
data is pending, since that leaves ep0 permanently busy after a
zero-length status stage.

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 | 83 ++++++++++++++++++++++++++++++---------
 1 file changed, 64 insertions(+), 19 deletions(-)

diff --git a/drivers/usb/gadget/legacy/inode.c b/drivers/usb/gadget/legacy/inode.c
index 7f6d84601..87aff568a 100644
--- a/drivers/usb/gadget/legacy/inode.c
+++ b/drivers/usb/gadget/legacy/inode.c
@@ -140,6 +140,8 @@ struct dev_data {
 					setup_abort : 1,
 					gadget_registered : 1;
 	unsigned			setup_wLength;
+	unsigned int			setup_out_len;
+	bool				ep0_req_queued;
 
 	/* the rest is basically write-once */
 	struct usb_config_descriptor	*config, *hs_config;
@@ -153,6 +155,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,12 +987,23 @@ static inline void ep0_readable (struct dev_data *dev)
 	kill_fasync (&dev->fasync, SIGIO, POLL_IN);
 }
 
+static void dev_req_complete(struct usb_ep *ep, struct usb_request *req)
+{
+	struct dev_data		*dev = ep->driver_data;
+	unsigned long		flags;
+
+	spin_lock_irqsave(&dev->lock, flags);
+	dev->ep0_req_queued = false;
+	spin_unlock_irqrestore(&dev->lock, flags);
+}
+
 static void clean_req (struct usb_ep *ep, struct usb_request *req)
 {
 	struct dev_data		*dev = ep->driver_data;
 
 	if (req->buf != dev->rbuf) {
-		kfree(req->buf);
+		if (req->buf != dev->setup_buf)
+			kfree(req->buf);
 		req->buf = dev->rbuf;
 	}
 	req->complete = epio_complete;
@@ -1004,12 +1018,18 @@ static void ep0_complete (struct usb_ep *ep, struct usb_request *req)
 
 	/* for control OUT, data must still get to userspace */
 	spin_lock_irqsave(&dev->lock, flags);
+	dev->ep0_req_queued = false;
 	if (!dev->setup_in) {
-		dev->setup_out_error = (req->status != 0);
-		if (!dev->setup_out_error)
-			free = 0;
-		dev->setup_out_ready = 1;
-		ep0_readable (dev);
+		/* a zero-length status stage carries no data */
+		if (dev->setup_wLength) {
+			dev->setup_out_error = (req->status != 0);
+			dev->setup_out_len = dev->setup_out_error
+						? 0 : req->actual;
+			dev->setup_out_ready = 1;
+			ep0_readable(dev);
+		}
+		req->buf = dev->rbuf;
+		free = 0;
 	}
 
 	/* clean up as appropriate */
@@ -1023,12 +1043,10 @@ 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) {
-		DBG (dev, "ep0 request busy!\n");
-		return -EBUSY;
-	}
 	if (len > sizeof (dev->rbuf))
 		req->buf = kmalloc(len, GFP_ATOMIC);
+	else
+		req->buf = dev->rbuf;
 	if (req->buf == NULL) {
 		req->buf = dev->rbuf;
 		return -ENOMEM;
@@ -1045,6 +1063,7 @@ 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) {
@@ -1074,10 +1093,15 @@ ep0_read (struct file *fd, char __user *buf, size_t len, loff_t *ptr)
 
 			if ((retval = setup_req (ep, req, 0)) == 0) {
 				++dev->udc_usage;
+				dev->ep0_req_queued = true;
 				spin_unlock_irq (&dev->lock);
 				retval = usb_ep_queue (ep, req, GFP_KERNEL);
 				spin_lock_irq (&dev->lock);
 				--dev->udc_usage;
+				if (retval < 0) {
+					dev->ep0_req_queued = false;
+					clean_req(ep, req);
+				}
 			}
 			dev->state = STATE_DEV_CONNECTED;
 
@@ -1115,19 +1139,24 @@ ep0_read (struct file *fd, char __user *buf, size_t len, loff_t *ptr)
 			}
 			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 */
 			}
 		}
@@ -1271,6 +1300,7 @@ ep0_write (struct file *fd, const char __user *buf, size_t len, loff_t *ptr)
 			if (retval == 0) {
 				dev->state = STATE_DEV_CONNECTED;
 				++dev->udc_usage;
+				dev->ep0_req_queued = true;
 				spin_unlock_irq (&dev->lock);
 				if (copy_from_user (dev->req->buf, buf, len))
 					retval = -EFAULT;
@@ -1284,6 +1314,7 @@ ep0_write (struct file *fd, const char __user *buf, size_t len, loff_t *ptr)
 				spin_lock_irq(&dev->lock);
 				--dev->udc_usage;
 				if (retval < 0) {
+					dev->ep0_req_queued = false;
 					clean_req (dev->gadget->ep0, dev->req);
 				} else
 					retval = len;
@@ -1618,6 +1649,7 @@ 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)) {
@@ -1626,13 +1658,16 @@ gadgetfs_setup (struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
 				if (value < 0)
 					break;
 
+				dev->req->buf = dev->setup_buf;
 				++dev->udc_usage;
+				dev->ep0_req_queued = true;
 				spin_unlock (&dev->lock);
 				value = usb_ep_queue (gadget->ep0, dev->req,
 							GFP_KERNEL);
 				spin_lock (&dev->lock);
 				--dev->udc_usage;
 				if (value < 0) {
+					dev->ep0_req_queued = false;
 					clean_req (gadget->ep0, dev->req);
 					break;
 				}
@@ -1663,17 +1698,21 @@ 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;
+		dev->ep0_req_queued = true;
 		spin_unlock (&dev->lock);
 		value = usb_ep_queue (gadget->ep0, req, GFP_KERNEL);
 		spin_lock(&dev->lock);
 		--dev->udc_usage;
-		spin_unlock(&dev->lock);
 		if (value < 0) {
+			dev->ep0_req_queued = false;
+			req->complete = epio_complete;
 			DBG (dev, "ep_queue --> %d\n", value);
 			req->status = 0;
 		}
+		spin_unlock(&dev->lock);
 		return value;
 	}
 
@@ -1771,6 +1810,7 @@ static void
 gadgetfs_unbind (struct usb_gadget *gadget)
 {
 	struct dev_data		*dev = get_gadget_data (gadget);
+	bool			queued;
 
 	DBG (dev, "%s\n", __func__);
 
@@ -1781,15 +1821,20 @@ gadgetfs_unbind (struct usb_gadget *gadget)
 		usleep_range(1000, 2000);
 		spin_lock_irq(&dev->lock);
 	}
+	queued = dev->ep0_req_queued;
 	spin_unlock_irq (&dev->lock);
 
+	if (dev->req) {
+		if (queued)
+			usb_ep_dequeue(gadget->ep0, dev->req);
+		usb_ep_free_request(gadget->ep0, dev->req);
+		dev->req = NULL;
+	}
+
 	destroy_ep_files (dev);
 	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);
 }

-- 
Git-155)

Re: [PATCH v3 1/2] usb: gadget: gadgetfs: fix ep0 request lifetime on disconnect

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [High] usb: gadget: gadgetfs: unchecked reuse of dev->req clears
  ep0_req_queued
- [High] usb: gadget: gadgetfs: race on dev->req->buf frees descriptor
  in clean_req()
--

commit 6cf737aca44046a58d9c7ba5151c4864f9f42306
Author: Jing Lin <kayano050408@gmail.com>

usb: gadget: gadgetfs: fix ep0 request lifetime on disconnect

This commit buffers delegated control OUT payloads in a separate
setup_buf array so dev->req can be reused before ep0_read() consumes the
data, and tracks whether dev->req is queued so gadgetfs_unbind() can
dequeue it before freeing.

> diff --git a/drivers/usb/gadget/legacy/inode.c b/drivers/usb/gadget/legacy/inode.c
> index 7f6d8460152f9..87aff568aa596 100644
> --- a/drivers/usb/gadget/legacy/inode.c
> +++ b/drivers/usb/gadget/legacy/inode.c
[ ... ]
> @@ -984,12 +987,23 @@ static inline void ep0_readable (struct dev_data *dev)
>  	kill_fasync (&dev->fasync, SIGIO, POLL_IN);
>  }
>  
> +static void dev_req_complete(struct usb_ep *ep, struct usb_request *req)
> +{
> +	struct dev_data		*dev = ep->driver_data;
> +	unsigned long		flags;
> +
> +	spin_lock_irqsave(&dev->lock, flags);
> +	dev->ep0_req_queued = false;
> +	spin_unlock_irqrestore(&dev->lock, flags);
> +}
> +
>  static void clean_req (struct usb_ep *ep, struct usb_request *req)
>  {
>  	struct dev_data		*dev = ep->driver_data;
>  
>  	if (req->buf != dev->rbuf) {
> -		kfree(req->buf);
> +		if (req->buf != dev->setup_buf)
> +			kfree(req->buf);
>  		req->buf = dev->rbuf;
>  	}
>  	req->complete = epio_complete;
[ ... ]
> @@ -1074,10 +1093,15 @@ ep0_read (struct file *fd, char __user *buf, size_t len, loff_t *ptr)
>  
>  			if ((retval = setup_req (ep, req, 0)) == 0) {
>  				++dev->udc_usage;
> +				dev->ep0_req_queued = true;
>  				spin_unlock_irq (&dev->lock);
>  				retval = usb_ep_queue (ep, req, GFP_KERNEL);
>  				spin_lock_irq (&dev->lock);
>  				--dev->udc_usage;
> +				if (retval < 0) {
> +					dev->ep0_req_queued = false;
> +					clean_req(ep, req);
> +				}
>  			}
>  			dev->state = STATE_DEV_CONNECTED;
>  

[Severity: High]
Can reusing dev->req without checking dev->ep0_req_queued clear
dev->ep0_req_queued while dev->req is still queued on gadget->ep0?

When gadgetfs_setup() delegates a control OUT transfer with w_length > 0,
it queues dev->req on gadget->ep0, sets dev->ep0_req_queued = true, and
queues a GADGETFS_SETUP event.  Once userspace reads that event (entering
STATE_DEV_SETUP with dev->setup_in == 0), a call to ep0_read() with
len == 0 enters this branch while the OUT data stage request is still in
flight, overwrites dev->req in setup_req(), and calls usb_ep_queue().
The UDC rejects the already-queued request with -EINVAL, and the error
path sets dev->ep0_req_queued = false even though dev->req remains on the
endpoint queue.  The same false clearing happens in ep0_write() and
gadgetfs_setup() if a previous ep0 request is still queued when a new
transfer starts, or if a second ep0_read() / ep0_write() races while
dev->lock is dropped before dev->state is updated.

When gadgetfs_unbind() later runs, it sees dev->ep0_req_queued == false,
skips usb_ep_dequeue(), and frees the active request, triggering the
WARN_ON(!list_empty(&req->queue)) in dummy_free_request() and a
use-after-free when dummy_udc_stop() drains the queue.

[ ... ]

> @@ -1271,6 +1300,7 @@ ep0_write (struct file *fd, const char __user *buf, size_t len, loff_t *ptr)
>  			if (retval == 0) {
>  				dev->state = STATE_DEV_CONNECTED;
>  				++dev->udc_usage;
> +				dev->ep0_req_queued = true;
>  				spin_unlock_irq (&dev->lock);
>  				if (copy_from_user (dev->req->buf, buf, len))
>  					retval = -EFAULT;
> @@ -1284,6 +1314,7 @@ ep0_write (struct file *fd, const char __user *buf, size_t len, loff_t *ptr)
>  				spin_lock_irq(&dev->lock);
>  				--dev->udc_usage;
>  				if (retval < 0) {
> +					dev->ep0_req_queued = false;
>  					clean_req (dev->gadget->ep0, dev->req);
>  				} else
>  					retval = len;

[Severity: High]
Can a concurrent USB_REQ_GET_DESCRIPTOR setup request while dev->lock is
dropped here cause copy_from_user() to overwrite descriptor memory or
clean_req() to kfree() it?

ep0_write() sets dev->state = STATE_DEV_CONNECTED and drops dev->lock
before copy_from_user() and usb_ep_queue().  If a USB_REQ_GET_DESCRIPTOR
request arrives in gadgetfs_setup() during that window, it sees
dev->state != STATE_DEV_SETUP and points dev->req->buf at dev->dev,
dev->config, or dev->hs_config (all inside dev->buf), and
dev_req_complete() leaves dev->req->buf pointing there:

CPU0                                  CPU1
ep0_write()
  setup_req()
    dev->req->buf = dev->rbuf
  dev->state = STATE_DEV_CONNECTED
  spin_unlock_irq(&dev->lock)
                                      gadgetfs_setup()
                                        case USB_DT_CONFIG:
                                          config_buf()
                                            dev->req->buf = dev->config
                                        usb_ep_queue(..., dev->req, ...)
  copy_from_user(dev->req->buf, ...)
    --> overwrites dev->config or
        returns -EFAULT
  spin_lock_irq(&dev->lock)
  clean_req()
    kfree(req->buf)                   <-- frees dev->config (dev->buf)

If copy_from_user() succeeds, up to RBUF_SIZE (256) bytes are written into
the descriptor buffer, overflowing the 18-byte dev->dev at the end of
dev->buf or corrupting dev->config.  If copy_from_user() or usb_ep_queue()
fails (here or in the new clean_req() error path in ep0_read()),
clean_req() sees req->buf is neither dev->rbuf nor dev->setup_buf and
calls kfree(req->buf), either freeing dev->buf while the gadget is still
active (leading to use-after-free and double free in dev_release()) or
passing an interior slab pointer (dev->hs_config or dev->dev) to kfree().

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261107-b4-gadgetfs-ep0-v3-1-62089dc0ec07@gmail.com?part=1

[PATCH v3 2/2] usb: gadget: gadgetfs: prevent ep0 event counter underflow

[Jing Lin via B4 Relay]

From: Jing Lin <kayano050408@gmail.com>

ep0_read() snapshots dev->ev_next, drops dev->lock while copy_to_user()
runs, and subtracts the snapshot afterwards. A concurrent disconnect,
connect, or another reader can reset or consume the queue in that
window. If the new value is smaller than the snapshot, the unsigned
subtraction wraps and the next read walks past dev->event[N_EVENT] or
the event purge memmove uses a huge length.

Count queue purges with a generation counter and skip the consumption
when the queue was purged while userspace was copying. Clamp the
subtraction for any other concurrent decrease.

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 | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/gadget/legacy/inode.c b/drivers/usb/gadget/legacy/inode.c
index 87aff568a..6db3b7879 100644
--- a/drivers/usb/gadget/legacy/inode.c
+++ b/drivers/usb/gadget/legacy/inode.c
@@ -126,6 +126,7 @@ struct dev_data {
 	enum ep0_state			state;		/* P: lock */
 	struct usb_gadgetfs_event	event [N_EVENT];
 	unsigned			ev_next;
+	unsigned int			ev_generation;
 	struct fasync_struct		*fasync;
 	u8				current_config;
 
@@ -1174,7 +1175,7 @@ ep0_read (struct file *fd, char __user *buf, size_t len, loff_t *ptr)
 scan:
 	/* return queued events right away */
 	if (dev->ev_next != 0) {
-		unsigned		i, n;
+		unsigned int		i, n, generation;
 
 		n = len / sizeof (struct usb_gadgetfs_event);
 		if (dev->ev_next < n)
@@ -1188,6 +1189,7 @@ ep0_read (struct file *fd, char __user *buf, size_t len, loff_t *ptr)
 				break;
 			}
 		}
+		generation = dev->ev_generation;
 		spin_unlock_irq (&dev->lock);
 		len = n * sizeof (struct usb_gadgetfs_event);
 		if (copy_to_user (buf, &dev->event, len))
@@ -1199,12 +1201,16 @@ ep0_read (struct file *fd, char __user *buf, size_t len, loff_t *ptr)
 			 * 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));
+			if (dev->ev_generation == generation) {
+				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;
+				} else {
+					dev->ev_next = 0;
+				}
 			}
-			dev->ev_next -= n;
 			spin_unlock_irq (&dev->lock);
 		}
 		return retval;
@@ -1252,6 +1258,7 @@ next_event (struct dev_data *dev, enum usb_gadgetfs_event_type type)
 		fallthrough;
 	case GADGETFS_CONNECT:
 		dev->ev_next = 0;
+		dev->ev_generation++;
 		break;
 	case GADGETFS_SETUP:		/* previous request timed out */
 	case GADGETFS_SUSPEND:		/* same effect */

-- 
Git-155)

Re: [PATCH v3 2/2] usb: gadget: gadgetfs: prevent ep0 event counter underflow

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] usb: gadget: gadgetfs: unlocked dev->event copy and event
  loss in ep0_read()
--

commit 7de7074f8592544c9b5457f297c92470cf9e3ac3
Author: Jing Lin <kayano050408@gmail.com>

usb: gadget: gadgetfs: prevent ep0 event counter underflow

This commit adds an event generation counter and clamps the subtraction on
dev->ev_next in ep0_read() to prevent an unsigned counter underflow when
the event queue changes during copy_to_user().

> diff --git a/drivers/usb/gadget/legacy/inode.c b/drivers/usb/gadget/legacy/inode.c
> index 87aff568aa596..6db3b7879c94d 100644
> --- a/drivers/usb/gadget/legacy/inode.c
> +++ b/drivers/usb/gadget/legacy/inode.c
[ ... ]
> @@ -1174,7 +1175,7 @@ ep0_read (struct file *fd, char __user *buf, size_t len, loff_t *ptr)
>  scan:
>  	/* return queued events right away */
>  	if (dev->ev_next != 0) {
> -		unsigned		i, n;
> +		unsigned int		i, n, generation;
>  
>  		n = len / sizeof (struct usb_gadgetfs_event);
>  		if (dev->ev_next < n)
> @@ -1188,6 +1189,7 @@ ep0_read (struct file *fd, char __user *buf, size_t len, loff_t *ptr)
>  				break;
>  			}
>  		}
> +		generation = dev->ev_generation;
>  		spin_unlock_irq (&dev->lock);
>  		len = n * sizeof (struct usb_gadgetfs_event);
>  		if (copy_to_user (buf, &dev->event, len))
> @@ -1199,12 +1201,16 @@ ep0_read (struct file *fd, char __user *buf, size_t len, loff_t *ptr)
>  			 * 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));
> +			if (dev->ev_generation == generation) {
> +				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;
> +				} else {
> +					dev->ev_next = 0;
> +				}
>  			}
> -			dev->ev_next -= n;
>  			spin_unlock_irq (&dev->lock);
>  		}
>  		return retval;
> @@ -1252,6 +1258,7 @@ next_event (struct dev_data *dev, enum usb_gadgetfs_event_type type)
>  		fallthrough;
>  	case GADGETFS_CONNECT:
>  		dev->ev_next = 0;
> +		dev->ev_generation++;
>  		break;
>  	case GADGETFS_SETUP:		/* previous request timed out */
>  	case GADGETFS_SUSPEND:		/* same effect */

[Severity: Medium]
Can ep0_read() still drop a newly arrived GADGETFS_SETUP or
GADGETFS_SUSPEND event or race on dev->event[] during copy_to_user()?

next_event() only increments dev->ev_generation on GADGETFS_DISCONNECT and
GADGETFS_CONNECT.  For GADGETFS_SETUP and GADGETFS_SUSPEND, it deduplicates
existing events of the same type in place without bumping
dev->ev_generation:

drivers/usb/gadget/legacy/inode.c:next_event() {
	...
	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--;
	...
}

Because ep0_read() leaves the n events in dev->event[] until after
copy_to_user() returns, a new GADGETFS_SETUP arriving during that window
deduplicates against the old entry still at dev->event[0]:

CPU0                                  CPU1
ep0_read()
  spin_lock_irq(&dev->lock)
  /* event[0] is GADGETFS_SETUP, n = 1 */
  generation = dev->ev_generation
  spin_unlock_irq(&dev->lock)
  copy_to_user(buf, &dev->event, len)
                                      gadgetfs_setup()
                                        spin_lock(&dev->lock)
                                        next_event(dev, GADGETFS_SETUP)
                                          /* discards old event[0] */
                                          /* writes new SETUP at event[0] */
                                          /* dev->ev_next is still 1 */
                                        spin_unlock(&dev->lock)
  spin_lock_irq(&dev->lock)
  if (dev->ev_generation == generation)
    dev->ev_next = 0;  <-- drops the new GADGETFS_SETUP

Dropping the new GADGETFS_SETUP event leaves ep0 control transfers wedged
when gadgetfs_setup() returns USB_GADGET_DELAYED_STATUS and leaves
dev->state out of sync with the queue.  Also, because copy_to_user() reads
directly from dev->event[] without holding dev->lock, concurrent
next_event() calls can overwrite or memmove() entries while they are being
copied to userspace, or a GADGETFS_CONNECT/DISCONNECT purge can write a new
event into dev->event[0] that gets copied by the current read and then left
queued for a duplicate delivery because dev->ev_generation changed.

Dequeuing the n events into a local buffer (N_EVENT * sizeof(struct
usb_gadgetfs_event) is 60 bytes, which fits in tmp[RBUF_SIZE]) and updating
dev->ev_next before dropping dev->lock for copy_to_user() would avoid the
underflow, the unlocked read of dev->event[], and the lost events.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261107-b4-gadgetfs-ep0-v3-1-62089dc0ec07@gmail.com?part=2