linux-kernel - Re: [PATCH] USB: output an error message when the pipe type doesn't match the endpoint type

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 for Android: free password hash cracker in your pocket

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

Message-ID: <Pine.LNX.4.44L0.1009021019090.1513-100000@iolanthe.rowland.org>
Date:	Thu, 2 Sep 2010 10:22:25 -0400 (EDT)
From:	Alan Stern <stern@...land.harvard.edu>
To:	Simon Arlott <simon@...e.lp0.eu>
cc:	Greg KH <greg@...ah.com>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	USB list <linux-usb@...r.kernel.org>
Subject: Re: [PATCH] USB: output an error message when the pipe type  doesn't
 match the endpoint type

On Thu, 2 Sep 2010, Simon Arlott wrote:

> On Wed, September 1, 2010 18:49, Alan Stern wrote:
> > On Wed, 1 Sep 2010, Simon Arlott wrote:
> >> > This is okay with me.  If you're serious about not changing the
> >> > behavior merely because debugging is enabled, you could move this test
> >> > out of the debug-only region and possibly change the dev_err to
> >> > dev_dbg.  However doing so might break some devices that are currently
> >> > working.
> >>
> >> I'd expect that to break potentially many devices, although only cxacru
> >> stopped working for me. The USB API isn't really suitable for adding
> >> this type of check because it allows the drivers to get away with too
> >> much already.
> >
> > Unlike device hardware, drivers can always be changed.  If adding a
> > check will help spot errors, it's probably worthwhile.
> 
> Yes, however the information about the device endpoint types hasn't been
> required in the past for the driver to work. The only way to check is to
> have the hardware available so the error may only show up after a full
> release of the kernel and break drivers that used to work. It could be
> enabled for all -rc kernels...

Which suggests that the best approach is to print the error message 
always, but allow the submission unless CONFIG_USB_DEBUG is set.

> >> usb_clear_halt() takes a pipe when it really wants the endpoint, the
> >> pipe type is ignored.
> >
> > What's wrong with that?  Besides, in the end we shouldn't be using
> > pipes at all; we should always use pointers to struct
> > usb_host_endpoint.
> 
> If a driver was trying to conform to the "don't use the wrong pipe type
> with an endpoint" rule then it may have to check it was using the
> correct pipe when calling usb_clear_halt(), although this is only a
> problem with drivers where the devices sometimes have interrupt instead
> of bulk endpoints.
> 
> Looking at usb_clear_halt(), it doesn't use the direction either... but
> drivers can call it in both directions. Several drivers already do this.

What do you mean?  Look at the first lines of code in usb_clear_halt():

	if (usb_pipein(pipe))
		endp |= USB_DIR_IN;

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/