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

[Daehwan Jung <dh10.jung@samsung.com>]

[-- Attachment #1: Type: text/plain, Size: 3986 bytes --]

On Tue, Sep 22, 2026 at 11:37:55AM +0530, Prashanth K wrote:
> 
> 
> On 7/27/2026 10:43 AM, Daehwan Jung wrote:
> > 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 discarding -ESHUTDOWN requests immediately in gs_read_complete()
> > while ep is still valid, preventing stale reqs from entering read_pool.
> > 
> > Fixes: 937ef73d5075 ("USB: serial gadget: rx path data loss fixes")
> > Signed-off-by: Daehwan Jung <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 | 18 +++++++++++++++---
> >  1 file changed, 15 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/usb/gadget/function/u_serial.c b/drivers/usb/gadget/function/u_serial.c
> > index cdd1dfc666c4..97ee1b9cb065 100644
> > --- a/drivers/usb/gadget/function/u_serial.c
> > +++ b/drivers/usb/gadget/function/u_serial.c
> > @@ -459,10 +459,22 @@ 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 (req->status == -ESHUTDOWN) {
> 
> Can we make sure this happens only if gserial_disconnect() is already
> executed, maybe checking for port_usb would help.> +		/*

Hi Prashanth K, Thanks for the comments.

I saw read_started = -16 when this issue occured. That's why I suspect
gserial_disconnect() resets it before. We can check port_usb instead as you
said.

> > +		 * Discard shutdown completions here while ep is still valid.
> > +		 * Returning them to read_pool after gserial_disconnect() has
> > +		 * reset the counters causes stale reqs (with req->dep pointing
> > +		 * to the old ep) to be queued onto a new ep at reconnect time,
> > +		 * triggering a req->dep != dep WARN.
> > +		 */
> 
> Nitpick, please rephrase this with content related to u_serial driver,
> we dont need dwc3 driver related comments here.
> 
Okay, I will modify it.
> > +		gs_free_req(ep, req);
> > +		port->read_started--;
> 
> read_started would be set to 0 by gserial_disconnect, so reducing it
> here doesn't make sense. Moreover it might cause underflow in next

You are right. It's better to remove that decrements read_started.
I will remove it on the next version.

Best Regards,
Daehwan Jung

> reads> +	} 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);
> >  }
> >  
> 
> Regards,
> Prashanth K
> 


[-- Attachment #2: Type: text/plain, Size: 0 bytes --]