linux-kernel - Re: [BUG REPORT] USB UDC/gadget regression

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: <Y9rT7irH/d9vd5MQ@rowland.harvard.edu>
Date:   Wed, 1 Feb 2023 16:04:46 -0500
From:   Alan Stern <stern@...land.harvard.edu>
To:     Aaro Koskinen <aaro.koskinen@....fi>
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [BUG REPORT] USB UDC/gadget regression

On Wed, Feb 01, 2023 at 09:20:22PM +0200, Aaro Koskinen wrote:
> Hi,
> 
> After commit fc274c1e9973 ("USB: gadget: Add a new bus for gadgets"),
> g_ether started to crash on modprobe when used with OMAP UDC.
> 
> It seems that because of drv->bus getting set to NULL in the middle of
> bus_add_driver() things go wrong.
> 
> The below simple change seems to fix it, but I'm not sure if this is
> the correct way? Many drivers seem to be doing the same in UDC start -
> are they really all broken?

Gosh, yes... it's pretty surprising.  None of the UDC drivers should 
change driver->driver.bus.  I don't know why they ever did this; you 
would have expected that field to be NULL anyway before the fc274c1e9973 
commit because it would never get set to anything.

> diff --git a/drivers/usb/gadget/udc/omap_udc.c b/drivers/usb/gadget/udc/omap_udc.c
> index 6e9314e95be3..47d83bbb09e3 100644
> --- a/drivers/usb/gadget/udc/omap_udc.c
> +++ b/drivers/usb/gadget/udc/omap_udc.c
> @@ -2062,7 +2062,6 @@ static int omap_udc_start(struct usb_gadget *g,
>  	udc->softconnect = 1;
>  
>  	/* hook up the driver */
> -	driver->driver.bus = NULL;
>  	udc->driver = driver;
>  	spin_unlock_irqrestore(&udc->lock, flags);

If you would like to submit a patch fixing all the UDC drivers that do 
this, it would be great.

Alan Stern