linux-kernel - Re: [PATCH 07/10] ehci-dbgp,ehci: Allow early or late use of the dbgp device

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 31 Jul 2009 14:53:03 -0400 (EDT)
From:	Alan Stern <stern@...land.harvard.edu>
To:	Jason Wessel <jason.wessel@...driver.com>
cc:	gregkh@...e.de, <linux-usb@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>, <dbrownell@...rs.sourceforge.net>,
	Ingo Molnar <mingo@...e.hu>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Yinghai Lu <yinghai@...nel.org>,
	"Eric W. Biederman" <ebiederm@...ssion.com>
Subject: Re: [PATCH 07/10] ehci-dbgp,ehci: Allow early or late use of the
 dbgp device

On Fri, 31 Jul 2009, Jason Wessel wrote:

> >> --- a/drivers/usb/host/ehci-hub.c
> >> +++ b/drivers/usb/host/ehci-hub.c
> >> @@ -816,6 +816,15 @@ static int ehci_hub_control (
> >>      case SetPortFeature:
> >>          selector = wIndex >> 8;
> >>          wIndex &= 0xff;
> >> +        if (unlikely(ehci->debug)) {
> >
> > Don't you need a similar test for ClearPortFeature?  Or does that not
> > matter?
> 
> I guess it depends if the user space tries to poke it, and then
> perhaps yes.  Do you know if that would be the case?

It's extremely unlikely.

> The rejection of the SetPortFeature at this level prevents the generic
> hub code from activating the port, in so far as all my tests cases
> show.
> 
> Given that it won't hurt anything to have the same code in the
> ClearPortFeature I can put it in.

It probably doesn't matter.

> >> +                retval = -ENODEV;
> >> +                goto error_exit;
> >
> > This is the wrong return value.  You should return -EPIPE, i.e., goto
> > error instead of error_exit.
> 
> 
> Do you have some insight as to how to account for the not using the
> port another way?  If I return -EPIPE, vs -ENODEV.  The kernel code
> does very different things.  I chose -ENODEV based on the results
> below.
> 
> With -ENODEV the kernel emits the following when trying to probe the
> usb port with the dbgp device.
> 
> 
> hub 2-0:1.0: cannot reset port 1 (err = -19)
> hub 2-0:1.0: cannot reset port 1 (err = -19)
> hub 2-0:1.0: cannot reset port 1 (err = -19)
> hub 2-0:1.0: cannot reset port 1 (err = -19)
> hub 2-0:1.0: unable to enumerate USB device on port 1
> 
> 
> With returning -EPIPE you get this result.
> 
> hub 2-0:1.0: cannot reset port 1 (err = -32)
> hub 2-0:1.0: cannot reset port 1 (err = -32)
> USB Serial support registered for generic
> hub 2-0:1.0: cannot reset port 1 (err = -32)
> hub 2-0:1.0: cannot reset port 1 (err = -32)
> hub 2-0:1.0: cannot reset port 1 (err = -32)
> hub 2-0:1.0: Cannot enable port 1.  Maybe the USB cable is bad?
> hub 2-0:1.0: cannot reset port 1 (err = -32)
> hub 2-0:1.0: cannot reset port 1 (err = -32)
> hub 2-0:1.0: cannot reset port 1 (err = -32)
> hub 2-0:1.0: cannot reset port 1 (err = -32)
> hub 2-0:1.0: cannot reset port 1 (err = -32)
> hub 2-0:1.0: Cannot enable port 1.  Maybe the USB cable is bad?
> hub 2-0:1.0: cannot reset port 1 (err = -32)
> hub 2-0:1.0: cannot reset port 1 (err = -32)
> hub 2-0:1.0: cannot reset port 1 (err = -32)
> hub 2-0:1.0: cannot reset port 1 (err = -32)
> hub 2-0:1.0: cannot reset port 1 (err = -32)
> hub 2-0:1.0: Cannot enable port 1.  Maybe the USB cable is bad?
> hub 2-0:1.0: cannot reset port 1 (err = -32)
> hub 2-0:1.0: cannot reset port 1 (err = -32)
> hub 2-0:1.0: cannot reset port 1 (err = -32)
> hub 2-0:1.0: cannot reset port 1 (err = -32)
> hub 2-0:1.0: cannot reset port 1 (err = -32)
> hub 2-0:1.0: Cannot enable port 1.  Maybe the USB cable is bad?
> hub 2-0:1.0: unable to enumerate USB device on port 1
> 
> 
> I was looking for the easiest maintainable approach to disabling the
> physical port from getting reset by the HCD hub driver, which does
> different things depending on the error code that is passed back.

Evidently I was wrong; -ENODEV is better.

> Knowing what happens with -EPIPE, would you still prefer it to be
> changed that way?  Another possibility here is to implement the
> restriction on the port reset another way, if you have any
> suggestions.
> 
> In general folks may not need to use earlyprintk=dbpg,keep too often,
> but it sure is nice when it works because you can use it as a full
> console monitoring device after the /sbin/init handoff, including
> debugging USB printk's because the usb debug controller driver is
> completely separate.

What happens across a system suspend?

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/