linux-kernel - RE: [EXT] Re: [PATCH 1/1] usb: gadget: call usb_gadget_check

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: <AM6PR04MB483828D189E93F8215C2F5398825A@AM6PR04MB4838.eurprd04.prod.outlook.com>
Date:   Thu, 29 Jun 2023 14:48:00 +0000
From:   Frank Li <frank.li@....com>
To:     Ravi Gunasekaran <r-gunasekaran@...com>,
        "rogerq@...nel.org" <rogerq@...nel.org>,
        "imx@...ts.linux.dev" <imx@...ts.linux.dev>,
        Jun Li <jun.li@....com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Elson Roy Serrao <quic_eserrao@...cinc.com>,
        Thinh Nguyen <Thinh.Nguyen@...opsys.com>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Jó Ágila Bitsch <jgilab@...il.com>,
        Prashanth K <quic_prashk@...cinc.com>,
        Peter Chen <peter.chen@...nel.org>,
        "open list:USB SUBSYSTEM" <linux-usb@...r.kernel.org>,
        open list <linux-kernel@...r.kernel.org>
Subject: RE: [EXT] Re: [PATCH 1/1] usb: gadget: call usb_gadget_check_config()
 to verify UDC capability



> -----Original Message-----
> From: Ravi Gunasekaran <r-gunasekaran@...com>
> Sent: Thursday, June 29, 2023 12:11 AM
> To: Frank Li <frank.li@....com>; rogerq@...nel.org; imx@...ts.linux.dev;
> Jun Li <jun.li@....com>; Greg Kroah-Hartman
> <gregkh@...uxfoundation.org>; Elson Roy Serrao
> <quic_eserrao@...cinc.com>; Thinh Nguyen
> <Thinh.Nguyen@...opsys.com>; Andy Shevchenko
> <andriy.shevchenko@...ux.intel.com>; Jó Ágila Bitsch <jgilab@...il.com>;
> Prashanth K <quic_prashk@...cinc.com>; Peter Chen
> <peter.chen@...nel.org>; open list:USB SUBSYSTEM <linux-
> usb@...r.kernel.org>; open list <linux-kernel@...r.kernel.org>
> Subject: [EXT] Re: [PATCH 1/1] usb: gadget: call usb_gadget_check_config()
> to verify UDC capability
> 
> Caution: This is an external email. Please take care when clicking links or
> opening attachments. When in doubt, report the message using the 'Report
> this email' button
> 
> 
> On 6/29/23 3:54 AM, Frank Li wrote:
> > The legacy gadget driver omitted calling usb_gadget_check_config()
> > to ensure that the USB device controller (UDC) has adequate resources,
> > including sufficient endpoint numbers and types, to support the given
> > configuration.
> >
> > Previously, usb_add_config() was solely invoked by the legacy gadget
> > driver. Adds the necessary usb_gadget_check_config() after the bind()
> > operation to fix the issue.
> >
> > Fixes: dce49449e04f ("usb: cdns3: allocate TX FIFO size according to
> composite EP number")
> > Reported-by: Ravi Gunasekaran <r-gunasekaran@...com>
> > Signed-off-by: Frank Li <Frank.Li@....com>
> > ---
> >  drivers/usb/gadget/composite.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/drivers/usb/gadget/composite.c
> b/drivers/usb/gadget/composite.c
> > index 1b3489149e5e..dd9b90481b4c 100644
> > --- a/drivers/usb/gadget/composite.c
> > +++ b/drivers/usb/gadget/composite.c
> > @@ -1125,6 +1125,10 @@ int usb_add_config(struct usb_composite_dev
> *cdev,
> >               goto done;
> >
> >       status = bind(config);
> > +
> > +     if (status == 0)
> > +             status = usb_gadget_check_config(cdev->gadget);
> 
> This will work for legacy gadgets that support only one configurations.
> Take for example g_ether. It has two configurations when RNDIS is enabled.
> And usb_add_config() is invoked for each configuration.
> 
> cdns3_gadget_check_config() calculates the total IN end-points based on the
> ep->claimed flag.
> 
>         list_for_each_entry(ep, &gadget->ep_list, ep_list) {
>                 if (ep->claimed && (ep->address & USB_DIR_IN))
>                         n_in++;
>         }
> 
> This ep->claimed flag is later cleared by usb_ep_autoconfig_reset() which is
> invoked in usb_add_config(). So for multi-configurations, actual total in end
> points
> is not taken into consideration thus resulting in incorrect fifo size
> allocation/check.
> 
> g_ffs is another gadget which has multiple configurations.

Actually it is another problem that I am trying to figure out. Configfs should have
 similar Problem. We never test multi config case in configfs also.  Generally,
composite device only have multi interfaces. 

cdns3_gadget_match_ep wrong cache all endpoints for all configs,  but actually
only one config can be active.

Frank 

> 
> 
> > +
> >       if (status < 0) {
> >               while (!list_empty(&config->functions)) {
> >                       struct usb_function             *f;
> 
> --
> Regards,
> Ravi