linux-kernel - Re: [RFC PATCH 2/2] usb: dwc3: Add chained TRB support for ep0

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] [day] [month] [year] [list]

Date:	Mon, 1 Jun 2015 13:42:36 -0500
From:	Felipe Balbi <balbi@...com>
To:	Kishon Vijay Abraham I <kishon@...com>
CC:	<balbi@...com>, <linux-usb@...r.kernel.org>,
	<linux-omap@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
	<gregkh@...uxfoundation.org>, Sekhar Nori <nsekhar@...com>
Subject: Re: [RFC PATCH 2/2] usb: dwc3: Add chained TRB support for ep0

Hi,

On Mon, Jun 01, 2015 at 03:23:48PM +0530, Kishon Vijay Abraham I wrote:
> >>+		transferred = transfer_size - length;
> >>+		buf = (u8 *)buf + transferred;
> >>+		ur->actual += transferred;
> >
> >this is dangerous. The extra size is because you *must* align OUT to
> >wMaxPacketSize, so you cannot allow more than the original req->length
> >to be copied into buf. That bounce buffer, is really supposed to be a
> 
> Here we are not handling bounce buffer. The bounce buffer is used only for
> the 2nd TRB which actually programs to receive data that is less than bounce
> buffer size. The 1st TRB will always be max packet aligned and the data is
> directly copied to the request buffer. (However note that if the request
> length is less than bounce buffer size, we'll use 1 TRB only)
> 
> To summarize..
> We are splitting req->length into 2 TRB's if the req->length is not aligned
> to wMaxPacketSize _and_ req->length is greater than bounce buffer size. By
> this way we can make the 2nd TRB to receive data lesser than or equal to
> bounce buffer size and the rest of it can be received using the 1st TRB.
> 
> Consider the following case.
> ur->length = 612;
> maxp = 512;
> 
> This case can't be handled by the existing bounce buffer mechanism since the
> size of bounce buffer is only 512. So we program 2 TRB's.
> First TRB
> trb->size = 512; /* We don't need bounce buffer for this TRB since it is max
> packet aligned. The data is directly loaded to the request buffer. */
> 
> Second TRB
> trb->size = 512 /* For the remaining 100 bytes we use bounce buffer and it
> uses the same existing bounce buffer mechanism. But instead of copying the
> data to the start of the request buffer, it has to be appended to the data
> that is received due to first TRB. */

this is all fine, but you need to make sure that you only copy the
remaining 100 bytes. Never, ever, ever copy anything past that.

> >throw-away buffer and should never have data in it. You should really
> >add a big fat WARN() if transferred > req->length.
> >
> >The thing is that if host sends more data than we were expecting, this
> >could be someone trying to use our driver as an exploit vector, trying
> >to send more data than it should. We must be robust against that.
> 
> This is handled in the existing bounce buffer mechanism and I use the same
> bounce buffer mechanism for the 2nd TRB.

ok.

-- 
balbi

Download attachment "signature.asc" of type "application/pgp-signature" (820 bytes)