The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [PATCH] usb: dwc2: remove useless check of !chan
@ 2026-05-19  6:00 Jisheng Zhang
  2026-05-19  9:27 ` Greg Kroah-Hartman
  0 siblings, 1 reply; 3+ messages in thread
From: Jisheng Zhang @ 2026-05-19  6:00 UTC (permalink / raw)
  To: Minas Harutyunyan, Greg Kroah-Hartman; +Cc: linux-usb, linux-kernel

It looks a bit strange we check !chan after dereference of this pointer
with "if (chan->ep_type != USB_ENDPOINT_XFER_CONTROL)".

In fact, when entering the dwc2_hcd_save_data_toggle(), the chan won't
be NULL, because its caller or indirect caller has ensured this,
specifically, it's checked with below line in dwc2_hc_n_intr()

if (!chan) {
    dev_err(hsotg->dev, "## hc_ptr_array for channel is NULL ##\n");
    return;
}

This addresses the following issue reported by klocwork tool:
  - Suspicious dereference of pointer 'chan' before NULL check at
    line 518

Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
---
 drivers/usb/dwc2/hcd_intr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/dwc2/hcd_intr.c b/drivers/usb/dwc2/hcd_intr.c
index 5c7538d498dd..397c63393c7f 100644
--- a/drivers/usb/dwc2/hcd_intr.c
+++ b/drivers/usb/dwc2/hcd_intr.c
@@ -515,7 +515,7 @@ void dwc2_hcd_save_data_toggle(struct dwc2_hsotg *hsotg,
 	u32 pid = (hctsiz & TSIZ_SC_MC_PID_MASK) >> TSIZ_SC_MC_PID_SHIFT;
 
 	if (chan->ep_type != USB_ENDPOINT_XFER_CONTROL) {
-		if (WARN(!chan || !chan->qh,
+		if (WARN(!chan->qh,
 			 "chan->qh must be specified for non-control eps\n"))
 			return;
 
-- 
2.53.0


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

* Re: [PATCH] usb: dwc2: remove useless check of !chan
  2026-05-19  6:00 [PATCH] usb: dwc2: remove useless check of !chan Jisheng Zhang
@ 2026-05-19  9:27 ` Greg Kroah-Hartman
  2026-05-19 23:27   ` Jisheng Zhang
  0 siblings, 1 reply; 3+ messages in thread
From: Greg Kroah-Hartman @ 2026-05-19  9:27 UTC (permalink / raw)
  To: Jisheng Zhang; +Cc: Minas Harutyunyan, linux-usb, linux-kernel

On Tue, May 19, 2026 at 02:00:01PM +0800, Jisheng Zhang wrote:
> It looks a bit strange we check !chan after dereference of this pointer
> with "if (chan->ep_type != USB_ENDPOINT_XFER_CONTROL)".
> 
> In fact, when entering the dwc2_hcd_save_data_toggle(), the chan won't
> be NULL, because its caller or indirect caller has ensured this,
> specifically, it's checked with below line in dwc2_hc_n_intr()
> 
> if (!chan) {
>     dev_err(hsotg->dev, "## hc_ptr_array for channel is NULL ##\n");
>     return;
> }
> 
> This addresses the following issue reported by klocwork tool:
>   - Suspicious dereference of pointer 'chan' before NULL check at
>     line 518
> 
> Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
> ---
>  drivers/usb/dwc2/hcd_intr.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/dwc2/hcd_intr.c b/drivers/usb/dwc2/hcd_intr.c
> index 5c7538d498dd..397c63393c7f 100644
> --- a/drivers/usb/dwc2/hcd_intr.c
> +++ b/drivers/usb/dwc2/hcd_intr.c
> @@ -515,7 +515,7 @@ void dwc2_hcd_save_data_toggle(struct dwc2_hsotg *hsotg,
>  	u32 pid = (hctsiz & TSIZ_SC_MC_PID_MASK) >> TSIZ_SC_MC_PID_SHIFT;
>  
>  	if (chan->ep_type != USB_ENDPOINT_XFER_CONTROL) {
> -		if (WARN(!chan || !chan->qh,
> +		if (WARN(!chan->qh,
>  			 "chan->qh must be specified for non-control eps\n"))
>  			return;

Can this ever actually happen?  If so, the machine just rebooted as
almost all devices with this hardware in it run with panic-on-warn
enabled.  If not, can you fix this up to properly handle the error and
return correctly and not crash?

thanks,

greg k-h

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

* Re: [PATCH] usb: dwc2: remove useless check of !chan
  2026-05-19  9:27 ` Greg Kroah-Hartman
@ 2026-05-19 23:27   ` Jisheng Zhang
  0 siblings, 0 replies; 3+ messages in thread
From: Jisheng Zhang @ 2026-05-19 23:27 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Minas Harutyunyan, linux-usb, linux-kernel

On Tue, May 19, 2026 at 11:27:55AM +0200, Greg Kroah-Hartman wrote:
> On Tue, May 19, 2026 at 02:00:01PM +0800, Jisheng Zhang wrote:
> > It looks a bit strange we check !chan after dereference of this pointer
> > with "if (chan->ep_type != USB_ENDPOINT_XFER_CONTROL)".
> > 
> > In fact, when entering the dwc2_hcd_save_data_toggle(), the chan won't
> > be NULL, because its caller or indirect caller has ensured this,
> > specifically, it's checked with below line in dwc2_hc_n_intr()
> > 
> > if (!chan) {
> >     dev_err(hsotg->dev, "## hc_ptr_array for channel is NULL ##\n");
> >     return;
> > }
> > 
> > This addresses the following issue reported by klocwork tool:
> >   - Suspicious dereference of pointer 'chan' before NULL check at
> >     line 518
> > 
> > Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
> > ---
> >  drivers/usb/dwc2/hcd_intr.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/usb/dwc2/hcd_intr.c b/drivers/usb/dwc2/hcd_intr.c
> > index 5c7538d498dd..397c63393c7f 100644
> > --- a/drivers/usb/dwc2/hcd_intr.c
> > +++ b/drivers/usb/dwc2/hcd_intr.c
> > @@ -515,7 +515,7 @@ void dwc2_hcd_save_data_toggle(struct dwc2_hsotg *hsotg,
> >  	u32 pid = (hctsiz & TSIZ_SC_MC_PID_MASK) >> TSIZ_SC_MC_PID_SHIFT;
> >  
> >  	if (chan->ep_type != USB_ENDPOINT_XFER_CONTROL) {
> > -		if (WARN(!chan || !chan->qh,
> > +		if (WARN(!chan->qh,
> >  			 "chan->qh must be specified for non-control eps\n"))
> >  			return;
> 
> Can this ever actually happen?  If so, the machine just rebooted as
> almost all devices with this hardware in it run with panic-on-warn
> enabled.  If not, can you fix this up to properly handle the error and
> return correctly and not crash?

Good question! The WARN was introduced by below commit
62943b7dfa35 ("usb: dwc2: host: fix the data toggle error in full speed
descriptor dma"
IMHO, it can't actually happen. And indeed, WARN() usage isn't
suggested in drivers, let me remove the WARN usage in v2

Thanks for the kind review

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

end of thread, other threads:[~2026-05-19 23:46 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-19  6:00 [PATCH] usb: dwc2: remove useless check of !chan Jisheng Zhang
2026-05-19  9:27 ` Greg Kroah-Hartman
2026-05-19 23:27   ` Jisheng Zhang

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