The Linux Kernel Mailing List
 help / color / mirror / Atom feed
From: Jisheng Zhang <jszhang@kernel.org>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Minas Harutyunyan <hminas@synopsys.com>,
	linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] usb: dwc2: remove useless check of !chan
Date: Wed, 20 May 2026 07:27:45 +0800	[thread overview]
Message-ID: <agzx8caQJEduwvS4@xhacker> (raw)
In-Reply-To: <2026051917-deceiver-monthly-cb22@gregkh>

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

      reply	other threads:[~2026-05-19 23:46 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=agzx8caQJEduwvS4@xhacker \
    --to=jszhang@kernel.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=hminas@synopsys.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox