linux-kernel - Re: Allocating buffers for USB transfers (again)

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]

Message-ID: <CACTFLAOi5u-YL+FKGotPSBv5BgEhKO5FLwZeJKdgyis3nZLbcw@mail.gmail.com>
Date:	Thu, 11 Aug 2011 19:27:15 +0200
From:	Daniel Mack <zonque@...il.com>
To:	Sarah Sharp <sarah.a.sharp@...ux.intel.com>
Cc:	Alan Stern <stern@...land.harvard.edu>,
	Florian Mickler <florian@...kler.org>,
	Oliver Neukum <oliver@...kum.org>, linux-usb@...r.kernel.org,
	alsa-devel@...a-project.org, Takashi Iwai <tiwai@...e.de>,
	Clemens Ladisch <clemens@...isch.de>, pedrib@...il.com,
	William Light <wrl@...est.net>, Greg KH <greg@...ah.com>,
	linux-kernel@...r.kernel.org, Robert Hancock <hancockrwd@...il.com>
Subject: Re: Allocating buffers for USB transfers (again)

On Thu, Aug 11, 2011 at 6:45 PM, Sarah Sharp
<sarah.a.sharp@...ux.intel.com> wrote:
> On Thu, Aug 11, 2011 at 02:57:41AM +0200, Daniel Mack wrote:
>> On Thu, Aug 11, 2011 at 1:15 AM, Sarah Sharp
>> <sarah.a.sharp@...ux.intel.com> wrote:
>> > On Wed, Aug 10, 2011 at 05:33:02PM +0200, Daniel Mack wrote:
>> >> On 08/10/2011 04:32 PM, Alan Stern wrote:
>> >> >Looking at the driver's current code, it appears that your patch
>> >> >does not fix the bug properly.  Using discontiguous regions in the
>> >> >transfer buffer is perfectly okay.  The real problem is later on,
>> >> >where you do:
>> >> >
>> >> >if (send_it) { out->number_of_packets = FRAMES_PER_URB;
>> >> >
>> >> >This should be
>> >> >
>> >> >out->number_of_packets = outframe;
>> >> >
>> >> >The way it is now, the USB stack will try to use data from all the
>> >> >frame descriptors, and the last few will be stale because the loop
>> >> >doesn't set them.
>> >>
>> >> That's actually true, even though it doesn't seem to cause any trouble.
>> >> I tested everything here of course, and the output URBs return back from
>> >> the USB stack with their length fields zeroed out, which then
>> >> causes the stack to send packets with zero-length fields at the end.
>> >
>> > Actually, it causes system hangs when the driver is loaded on a device
>> > attached to a USB 3.0 port, as Alan Stern pointed out:
>> >
>> > https://bugzilla.kernel.org/show_bug.cgi?id=40702
>>
>> Yes, I've noticed this.
>>
>> > Please don't submit zero-length transfers.  The xHCI driver just isn't
>> > able to handle it.  Arguably, it probably should have just rejected your
>> > URB when it found a zero length buffer, so I'll probably be submitting a
>> > patch to fix that.
>>
>> According to the spec, sending zero-length frames should be fine, no?
>> Is there any particular reason why XCHI can't handle this while EHCI
>> can? And does my patch fix the driver for XHCI?
>
> Ok, yes, you're correct that the xHCI spec allows the transfer length to
> be set to zero.  In the case where the frame buffer is zero-length, is
> the buffer pointer still valid?  It's not clear from the spec whether it
> needs to be.

Well, the buffer pointer is set in the URB, not in its individual iso
subframes which just denotes them via the offset field. So yes, it is
valid in my case. But it doesn't matter anymore, as the code which
does that is now gone :)


Daniel
--
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/