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]
Message-ID: <20150601184236.GH26081@saruman.tx.rr.com>
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)

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ