Linux USB
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Alan Stern <stern@rowland.harvard.edu>
Cc: Griffin Kroah-Hartman <griffin@kroah.com>,
	linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] usb: core: Strengthen error handling in hub_hub_status()
Date: Fri, 17 Jul 2026 11:48:46 +0200	[thread overview]
Message-ID: <2026071717-richness-opossum-b855@gregkh> (raw)
In-Reply-To: <f66a5507-5e83-424f-9c11-15b1882be499@rowland.harvard.edu>

On Tue, Jul 14, 2026 at 12:32:06PM -0400, Alan Stern wrote:
> On Tue, Jul 14, 2026 at 11:04:33AM +0200, Griffin Kroah-Hartman wrote:
> > Add additional error handling after the call to get_hub_status() in
> > hub_hub_status().
> > 
> > get_hub_status() uses usb_control_msg() which does not verify that the
> > message is the correct length, substituting it for
> > usb_control_msg_recv() would also solve this issue but increase memory
> > allocations.
> > 
> > Instead, error handling is copied from the method used in
> > hub_ext_port_status(), which shares the same flow of logic as
> > hub_hub_status().
> > 
> > Assisted-by: gkh_clanker_t1000
> > Signed-off-by: Griffin Kroah-Hartman <griffin@kroah.com>
> > ---
> >  drivers/usb/core/hub.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> > index 5262e11c12cd..0d2166b8923a 100644
> > --- a/drivers/usb/core/hub.c
> > +++ b/drivers/usb/core/hub.c
> > @@ -991,10 +991,12 @@ static int hub_hub_status(struct usb_hub *hub,
> >  
> >  	mutex_lock(&hub->status_mutex);
> >  	ret = get_hub_status(hub->hdev, &hub->status->hub);
> > -	if (ret < 0) {
> > +	if (ret < sizeof(hub->status->hub)) {
> >  		if (ret != -ENODEV)
> >  			dev_err(hub->intfdev,
> >  				"%s failed (err = %d)\n", __func__, ret);
> > +		if (ret >= 0)
> > +			ret = -EIO;
> 
> It would be better to put these two lines above the preceding test.  
> That way it won't print confusing things like "hub_hub_status failed 
> (err = 2)".

It is confusing, but it matches the other check for this same error in
hub_ext_port_status() as it's the same error condition.  How about this
patch happens first int he series, which "unifies" them, and then fix up
both of the patterns at the same time to be more clear in a later patch
in the series?

thanks,

greg k-h

      parent reply	other threads:[~2026-07-17  9:48 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-14  9:04 [PATCH] usb: core: Strengthen error handling in hub_hub_status() Griffin Kroah-Hartman
2026-07-14 16:32 ` Alan Stern
2026-07-14 17:48   ` Oliver Neukum
2026-07-14 18:59     ` Alan Stern
2026-07-17  9:48   ` Greg Kroah-Hartman [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=2026071717-richness-opossum-b855@gregkh \
    --to=gregkh@linuxfoundation.org \
    --cc=griffin@kroah.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=stern@rowland.harvard.edu \
    /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