[PATCH v4] usb: gadget: u_serial: fix stale req->dep warn on disconnect/reconnect

[PATCH v4] usb: gadget: u_serial: fix stale req->dep warn on disconnect/reconnect

[Daehwan Jung]

When DWC3_EP_DELAY_STOP defers ENDXFER, giveback of -ESHUTDOWN requests
arrives after gserial_disconnect(). These stale requests then accumulate
in read_pool via gs_rx_push(). After gadget exit frees the old endpoint,
req->dep becomes a dangling pointer. On the next gserial_connect(),
gs_start_rx() queues these stale requests onto a new endpoint, triggering:

  WARNING: CPU: 0 at drivers/usb/dwc3/gadget.c:1990
  request 00000000e6e350a5 belongs to ''

  Call trace:
   dwc3_gadget_ep_queue+0x158/0x1e8
   usb_ep_queue+0x60/0xe8
   gs_start_rx+0xa4/0x128
   gs_start_io+0x128/0x254
   gserial_connect+0xb4/0x14c
   acm_set_alt+0xa0/0x114
   set_config+0x22c/0x384
   composite_setup+0x37c/0xc7c
   configfs_composite_setup+0x5c/0x88
   dwc3_ep0_interrupt+0x6c8/0xbcc
   dwc3_thread_interrupt+0xa0/0x1284
   irq_thread_fn+0x48/0xa8
   irq_thread+0x150/0x31c
   kthread+0x150/0x27c
   ret_from_fork+0x10/0x20

Fix by freeing the request in gs_read_complete() when the port has
already been disconnected (port->port_usb is NULL), so that stale
requests never enter read_pool.

Fixes: 937ef73d5075 ("USB: serial gadget: rx path data loss fixes")
Signed-off-by: Daehwan Jung <dh10.jung@samsung.com>
---
Changes in v4:
- Check port->port_usb instead of req->status == -ESHUTDOWN to detect
  completions after gserial_disconnect()
- Drop the read_started decrement, gserial_disconnect() already resets it
- Rephrase the code comment in terms of u_serial
Link to v3: https://lore.kernel.org/all/20260727051335.1745955-1-dh10.jung@samsung.com/

Changes in v3:
- Add missing version tag in subject
Link to v2: https://lore.kernel.org/linux-usb/20260727042632.1726391-1-dh10.jung@samsung.com/

Changes in v2:
- Correct the name of author
- Modify commit subject (panic -> warn)
Link to v1: https://lore.kernel.org/all/20260721013509.2327242-1-dh10.jung@samsung.com/
---
 drivers/usb/gadget/function/u_serial.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/gadget/function/u_serial.c b/drivers/usb/gadget/function/u_serial.c
index cdd1dfc666c4f..917fe0a714972 100644
--- a/drivers/usb/gadget/function/u_serial.c
+++ b/drivers/usb/gadget/function/u_serial.c
@@ -459,10 +459,21 @@ static void gs_read_complete(struct usb_ep *ep, struct usb_request *req)
 {
 	struct gs_port	*port = ep->driver_data;
 
-	/* Queue all received data until the tty layer is ready for it. */
 	spin_lock(&port->port_lock);
-	list_add_tail(&req->list, &port->read_queue);
-	schedule_delayed_work(&port->push, 0);
+	if (!port->port_usb) {
+		/*
+		 * The port was disconnected: gserial_disconnect() has already
+		 * freed read_pool/read_queue and reset the request counters.
+		 * Free this request now instead of queueing it, otherwise
+		 * gs_rx_push() would return it to read_pool and the next
+		 * gserial_connect() would submit it on the new OUT endpoint.
+		 */
+		gs_free_req(ep, req);
+	} else {
+		/* Queue all received data until the tty layer is ready for it. */
+		list_add_tail(&req->list, &port->read_queue);
+		schedule_delayed_work(&port->push, 0);
+	}
 	spin_unlock(&port->port_lock);
 }
 
-- 
2.34.1

Re: [PATCH v4] usb: gadget: u_serial: fix stale req->dep warn on disconnect/reconnect

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] usb: cdns3: use-after-free on request->buf in gadget giveback
--

commit 8f8778218c9b936c4a5d0fa16a09a9091afc687a
Author: Daehwan Jung <dh10.jung@samsung.com>

usb: gadget: u_serial: fix stale req->dep warn on disconnect/reconnect

When DWC3_EP_DELAY_STOP defers ENDXFER, giveback of -ESHUTDOWN requests
arrives after gserial_disconnect(). This commit frees the stale request
in gs_read_complete() when the port is already disconnected to prevent
subsequent warnings.

Link to v3: https://lore.kernel.org/all/20260727051335.1745955-1-dh10.jung@samsung.com/

> diff --git a/drivers/usb/gadget/function/u_serial.c b/drivers/usb/gadget/function/u_serial.c
> index cdd1dfc666c4f..ba6af7127fc47 100644
> --- a/drivers/usb/gadget/function/u_serial.c
> +++ b/drivers/usb/gadget/function/u_serial.c
> @@ -459,10 +459,21 @@ static void gs_read_complete(struct usb_ep *ep, struct usb_request *req)
>  {
>  	struct gs_port	*port = ep->driver_data;
>  
> -	/* Queue all received data until the tty layer is ready for it. */
>  	spin_lock(&port->port_lock);
> -	list_add_tail(&req->list, &port->read_queue);
> -	schedule_delayed_work(&port->push, 0);
> +	if (!port->port_usb) {
> +		/*
> +		 * The port was disconnected: gserial_disconnect() has already
> +		 * freed read_pool/read_queue and reset the request counters.
> +		 * Free this request now instead of queueing it, otherwise
> +		 * gs_rx_push() would return it to read_pool and the next
> +		 * gserial_connect() would submit it on the new OUT endpoint.
> +		 */
> +		gs_free_req(ep, req);

[Severity: High]
Does this cause a use-after-free when used with the cdns3 USB controller?

The cdns3 driver evaluates request->buf after calling the giveback handler:

drivers/usb/cdns3/cdns3-gadget.c:cdns3_gadget_giveback() {
    ...
		usb_gadget_giveback_request(&priv_ep->endpoint,
					    request);
		spin_lock(&priv_dev->lock);
	}

	if (request->buf == priv_dev->zlp_buf)
		cdns3_gadget_ep_free_request(&priv_ep->endpoint, request);
    ...
}

If gs_read_complete() synchronously frees the request via gs_free_req() when
the port is disconnected, will the subsequent check in cdns3_gadget_giveback()
dereference the freed request pointer?

> +	} else {
> +		/* Queue all received data until the tty layer is ready for it. */
> +		list_add_tail(&req->list, &port->read_queue);
> +		schedule_delayed_work(&port->push, 0);
> +	}
>  	spin_unlock(&port->port_lock);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/1860250540.01791791282424.JavaMail.epsvc@epcpadp2new?part=1