mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v3] usb: gadget: u_serial: fix stale req->dep warn on disconnect/reconnect
       [not found] <CGME20260727051638epcas2p11f7b91e0be7604331bfe0f46beef9ce1@epcas2p1.samsung.com>
@ 2026-07-27  5:13 ` Daehwan Jung
  2026-09-21  5:01   ` Daehwan Jung
  2026-09-22  6:07   ` Prashanth K
  0 siblings, 2 replies; 4+ messages in thread
From: Daehwan Jung @ 2026-07-27  5:13 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: open list:USB SUBSYSTEM, open list, 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 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) {
+		/*
+		 * 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.
+		 */
+		gs_free_req(ep, req);
+		port->read_started--;
+	} 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


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v3] usb: gadget: u_serial: fix stale req->dep warn on disconnect/reconnect
  2026-07-27  5:13 ` [PATCH v3] usb: gadget: u_serial: fix stale req->dep warn on disconnect/reconnect Daehwan Jung
@ 2026-09-21  5:01   ` Daehwan Jung
  2026-09-22  6:07   ` Prashanth K
  1 sibling, 0 replies; 4+ messages in thread
From: Daehwan Jung @ 2026-09-21  5:01 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: open list:USB SUBSYSTEM, open list

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

On Mon, Jul 27, 2026 at 02:13:35PM +0900, 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) {
> +		/*
> +		 * 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.
> +		 */
> +		gs_free_req(ep, req);
> +		port->read_started--;
> +	} 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
> 
> 

Hi,
This is gentle ping. Any comments would be appreciated.

Best Regards,
Daehwan Jung

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



^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v3] usb: gadget: u_serial: fix stale req->dep warn on disconnect/reconnect
  2026-07-27  5:13 ` [PATCH v3] usb: gadget: u_serial: fix stale req->dep warn on disconnect/reconnect Daehwan Jung
  2026-09-21  5:01   ` Daehwan Jung
@ 2026-09-22  6:07   ` Prashanth K
  2026-09-23  7:34     ` Daehwan Jung
  1 sibling, 1 reply; 4+ messages in thread
From: Prashanth K @ 2026-09-22  6:07 UTC (permalink / raw)
  To: Daehwan Jung, Greg Kroah-Hartman; +Cc: open list:USB SUBSYSTEM, open list



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.> +		/*
> +		 * 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.

> +		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
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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v3] usb: gadget: u_serial: fix stale req->dep warn on disconnect/reconnect
  2026-09-22  6:07   ` Prashanth K
@ 2026-09-23  7:34     ` Daehwan Jung
  0 siblings, 0 replies; 4+ messages in thread
From: Daehwan Jung @ 2026-09-23  7:34 UTC (permalink / raw)
  To: Prashanth K
  Cc: Greg Kroah-Hartman, open list:USB SUBSYSTEM, open list, cpgs, cpgsproxy1

[-- 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 --]



^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-09-23  7:45 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <CGME20260727051638epcas2p11f7b91e0be7604331bfe0f46beef9ce1@epcas2p1.samsung.com>
2026-07-27  5:13 ` [PATCH v3] usb: gadget: u_serial: fix stale req->dep warn on disconnect/reconnect Daehwan Jung
2026-09-21  5:01   ` Daehwan Jung
2026-09-22  6:07   ` Prashanth K
2026-09-23  7:34     ` Daehwan Jung

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®