Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933141AbbFIPAB (ORCPT ); Tue, 9 Jun 2015 11:00:01 -0400 Received: from iolanthe.rowland.org ([192.131.102.54]:38447 "HELO iolanthe.rowland.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S933019AbbFIO7v (ORCPT ); Tue, 9 Jun 2015 10:59:51 -0400 Date: Tue, 9 Jun 2015 10:59:50 -0400 (EDT) From: Alan Stern X-X-Sender: stern@iolanthe.rowland.org To: Kishon Vijay Abraham I cc: Michael Trimarchi , Felipe Balbi , , , , , Subject: Re: [RFC PATCH] usb: dwc3: ep0: Fix mem corruption on OUT transfers of more than 512 bytes In-Reply-To: <5576FBBA.4000905@ti.com> Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3202 Lines: 74 On Tue, 9 Jun 2015, Kishon Vijay Abraham I wrote: > Hi, > > On Tuesday 09 June 2015 08:09 PM, Michael Trimarchi wrote: > > Hi > > > > On Jun 9, 2015 4:36 PM, "Kishon Vijay Abraham I" > > wrote: > > > > > > DWC3 uses bounce buffer to handle non max packet aligned OUT transfers and > > > the size of bounce buffer is 512 bytes. However if the host initiates OUT > > > transfers of size more than 512 bytes (and non max packet aligned), the > > > driver throws a WARN dump but still programs the TRB to receive more than > > > 512 bytes. This will cause bounce buffer to overflow and corrupt the > > > adjacent memory locations which can be fatal. > > > > > > Fix it by programming the TRB to receive a maximum of DWC3_EP0_BOUNCE_SIZE > > > (512) bytes. > > > > > > Signed-off-by: Kishon Vijay Abraham I > > > > --- > > > Steps to see the issue (before this patch) > > > 1) Insert g_zero in DUT > > > 2) run './testusb -t 14 -c 1 -s 520 -v 1' in host (size should be > 512) > > > > > > The test should FAIL since bounce buffer can handle only 512 bytes, but the > > > test PASS. There is a WARN dump in DUT but still there will be memory > > > corruption since the bounce buffer overflows. > > > > > > Tested this patch using USB3 Gen X CV (ch9 tests: usb2 and usb3, link layer > > > testing and MSC tests) and using USB2 X CV (ch9 tests, MSC tests). > > > > > > After the patch, the tests timeout! > > > ./testusb -t 14 -c 1 -s 514 -v 1 > > > unknown speed /dev/bus/usb/001/018 0 > > > /dev/bus/usb/001/018 test 14 --> 110 (Connection timed out) > > > > > > IMO a patch to fix this is required for stable releases too. So If this > > > patch is alright, I can post the patch cc'ing stable. While the actual fix > > > would be to have chained TRB, I'm not sure if it can go to stable > > > releases. > > > drivers/usb/dwc3/ep0.c | 12 ++++++++++-- > > > 1 file changed, 10 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c > > > index 2ef3c8d..8858c60 100644 > > > --- a/drivers/usb/dwc3/ep0.c > > > +++ b/drivers/usb/dwc3/ep0.c > > > @@ -816,6 +816,11 @@ static void dwc3_ep0_complete_data(struct dwc3 *dwc, > > > unsigned maxp = ep0->endpoint.maxpacket; > > > > > > transfer_size += (maxp - (transfer_size % maxp)); > > > + > > > + /* Maximum of DWC3_EP0_BOUNCE_SIZE can only be received */ > > > + if (transfer_size > DWC3_EP0_BOUNCE_SIZE) > > > + transfer_size = DWC3_EP0_BOUNCE_SIZE; > > > + > > > > Can you just use maxp in the correct way? > > what do you mean by correct way? Using roundup() to calculate transfer_size? Why not just make the bounce buffer size the same as the maxpacket size? In other words, 1024 bytes instead of 512, for ep0 on a USB-3 device. Alan stern -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/