Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753836Ab3I0Pct (ORCPT ); Fri, 27 Sep 2013 11:32:49 -0400 Received: from netrider.rowland.org ([192.131.102.5]:55260 "HELO netrider.rowland.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1753497Ab3I0Pcq (ORCPT ); Fri, 27 Sep 2013 11:32:46 -0400 Date: Fri, 27 Sep 2013 11:32:45 -0400 (EDT) From: Alan Stern X-X-Sender: stern@netrider.rowland.org To: Robert Baldyga cc: balbi@ti.com, , , , , , Subject: Re: [PATCH] USB: gadget: epautoconf: fix ep maxpacket check In-Reply-To: <1380277808-17736-1-git-send-email-r.baldyga@samsung.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: 2202 Lines: 72 On Fri, 27 Sep 2013, Robert Baldyga wrote: > This patch fixes validation of maxpacket value given in endpoint descriptor. > Added check of maxpacket for bulk endpoints. > Correct maxpacket value is: > > FULL-SPEED HIGH-SPEED > BULK 64 512 Don't forget SuperSpeed. It requires Bulk endpoints to have maxpacket = 1024. > INTERRUPT 64 1024 > ISOCHRONOUS 1023 1024 > > Signed-off-by: Robert Baldyga > --- > drivers/usb/gadget/epautoconf.c | 16 +++++++++++++--- > 1 file changed, 13 insertions(+), 3 deletions(-) > > diff --git a/drivers/usb/gadget/epautoconf.c b/drivers/usb/gadget/epautoconf.c > index a777f7b..35bde34 100644 > --- a/drivers/usb/gadget/epautoconf.c > +++ b/drivers/usb/gadget/epautoconf.c > @@ -136,16 +136,26 @@ ep_matches ( > * the usb spec fixes high speed bulk maxpacket at 512 bytes. > */ > max = 0x7ff & usb_endpoint_maxp(desc); > + > + if (ep->maxpacket < max) > + return 0; > + > switch (type) { > + case USB_ENDPOINT_XFER_BULK: > + /* BULK: limit 512 high/super speed */ 512 high, 1024 super. > + if (max > 512) > + return 0; > + /* FALLTHROUGH */ > + > case USB_ENDPOINT_XFER_INT: > - /* INT: limit 64 bytes full speed, 1024 high/super speed */ > + /* BULK/INT: limit 64 bytes full speed */ > if (!gadget_is_dualspeed(gadget) && max > 64) > return 0; > /* FALLTHROUGH */ > > case USB_ENDPOINT_XFER_ISOC: > - /* ISO: limit 1023 bytes full speed, 1024 high/super speed */ > - if (ep->maxpacket < max) > + /* INT/ISO: limit 1023 bytes full speed, 1024 high/super speed */ The comment mentions INT/ISO. But this code can also run in the BULK case. > + if (max > 1024) > return 0; > if (!gadget_is_dualspeed(gadget) && max > 1023) > return 0; I suspect this code would become a lot clearer if all the FALLTHROUGH logic were eliminated and each case was handled on its own. 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/