netdev - Re: [PATCH] usb: xhci: Link TRB must not occur with a USB payload burst.

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: <Pine.LNX.4.44L0.1311081119510.1162-100000@iolanthe.rowland.org>
Date:	Fri, 8 Nov 2013 11:45:59 -0500 (EST)
From:	Alan Stern <stern@...land.harvard.edu>
To:	Sarah Sharp <sarah.a.sharp@...ux.intel.com>
cc:	David Laight <David.Laight@...LAB.COM>, <netdev@...r.kernel.org>,
	<linux-usb@...r.kernel.org>
Subject: Re: [PATCH] usb: xhci: Link TRB must not occur with a USB payload
 burst.

On Thu, 7 Nov 2013, Sarah Sharp wrote:

> On Thu, Nov 07, 2013 at 05:20:49PM -0000, David Laight wrote:
> > 
> > Section 4.11.7.1 of rev 1.0 of the xhci specification states that a link TRB
> > can only occur at a boundary between underlying USB frames (512 bytes for 480M).
> 
> Which version of the spec are you looking at?  I'm looking at the
> updated version from 08/2012 and I don't see any such requirement.
> 
> I do see that section says "A TD Fragment shall not span Transfer Ring
> Segments" where a TD fragment is defined as exact multiples of Max Burst
> Size * Max Packet Size bytes for the endpoint.  Is that what you mean
> about USB frames?

...

> The driver would also have to make sure that TD fragments didn't have
> link TRBs in them.  That's an issue, since the spec decidedly unclear
> about what exactly comprises a TD fragment.  Is the xHC host greedy, and
> will lump all multiples into one TD fragment?  Or will it assume the TD
> fragment has ended once it consumes one multiple of Max Burst Size * Max
> Packet Size bytes?
> 
> This ambiguity means it's basically impossible to figure out whether a
> TD with a link TRB in the middle is constructed properly.  The xHCI spec
> architect didn't have a good solution to this problem, so I punted and
> never implemented TD fragments.  If this is really an issue, it's going
> to be pretty complex to solve.

This is something I wanted to ask you about also.

The whole idea of TD fragments is exceedingly cloudy.  The definition
doesn't make sense, and the spec doesn't say what the actual hardware
restrictions are, i.e., what is the underlying reality that the TD
fragment concept wants to express.

Even if you do your best to ignore the whole idea, there still appear
to be certain restrictions you need to obey.  In addition to the
question of where Link TRBs are allowed, you also have to worry about
TDs whose size isn't a multiple of the Max Burst Payload (MBP) size =
MaxBurstSize * MaxPacketSize.

According to my version of the spec (Rev 1.0, dated 5/21/2010), if a TD
is larger than the MBP and its length isn't a multiple of the MBP, then
the last MBP boundary in the TD must also be a TRB boundary.  This 
follows from two statements in section 4.11.7.1:

	If the TD Transfer Size is an even multiple of the MBP then all 
	TD Fragments shall define exact multiples of MBP data bytes.  
	If not, then only the last TD Fragment shall define less than 
	MBP data (or the Residue) bytes.

So if a TD is longer than MBP then it must contain at least two TD
fragments, and the last fragment must consist of the last Residue
bytes (i.e., the bytes beyond the last MBP boundary).

	Each TD Fragment is comprised of one or more TRBs.

Hence the last MBP boundary in the TD, which marks the beginning of the 
last TD fragment, must also be a TRB boundary.

I have no idea whether violating this restriction will cause trouble
for real hardware.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html