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: <AE90C24D6B3A694183C094C60CF0A2F6026B743B@saturn3.aculab.com>
Date:	Wed, 20 Nov 2013 17:02:54 -0000
From:	"David Laight" <David.Laight@...LAB.COM>
To:	"Sarah Sharp" <sarah.a.sharp@...ux.intel.com>
Cc:	"Alan Stern" <stern@...land.harvard.edu>,
	"Ben Hutchings" <bhutchings@...arflare.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.

> From: Sarah Sharp [mailto:sarah.a.sharp@...ux.intel.com]
> On Wed, Nov 20, 2013 at 09:36:11AM -0000, David Laight wrote:
> > > From: Alan Stern [mailto:stern@...land.harvard.edu]
> > > On Tue, 19 Nov 2013, Sarah Sharp wrote:
> > >
> > > > The xHCI driver can limit the number of sg-list entries through
> > > > hcd->self.sg_tablesize.  It's currently set to ~0, which is "however
> > > > many entries you want.  You could set that to the number of TRBs in a
> > > > segment (minus one for the link TRB).
> > > >
...
> 
> Ok, so the networking layer should be fine.  However, with the current
> patch, if the mass storage driver sends down a scatter-gather list
> that's bigger than a ring segment, or needs to be split up so it doesn't
> cross 64K boundaries, then the URB submission will fail.  We don't want
> that to happen.
> 
> At the very least, we should limit hcd->self.sg_tablesize in
> drivers/usb/host/xhci.c to 63 (TRBS_PER_SEGMENT - 1).  But we could
> still potentially run into the 64K boundary issue in one or maybe all of
> those entries.  Would it be crazy to limit the number of entries to half
> that (31)?  It may impact performance, but it ensures that SCSI reads
> and writes don't randomly fail.  We can always increase the ring segment
> size in a later patch.

Unless there is a limit on the overall length of the transfer you'll always lose.
An aligned 4MB transfer in a contiguous memory buffer requires 64 TRBs.
So won't fit in a ring segment.

Although I'd guess that such transfers are unusual.
If you bet that crossing a 64k boundaries is unusual then 32 is probably
a sane limit.

Fragments over 16k can easily be split between ring segments - so a more
complex check could allow for that - but it won't be quick to write.

	David



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