[PATCH] usb: dwc2: remove useless check of !chan

Jisheng Zhang posted 1 patch 5 days, 20 hours ago
drivers/usb/dwc2/hcd_intr.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] usb: dwc2: remove useless check of !chan
Posted by Jisheng Zhang 5 days, 20 hours ago
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
Re: [PATCH] usb: dwc2: remove useless check of !chan
Posted by Greg Kroah-Hartman 5 days, 17 hours ago
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
Re: [PATCH] usb: dwc2: remove useless check of !chan
Posted by Jisheng Zhang 5 days, 3 hours ago
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