linux-kernel - Re: [PATCH v3 1/3] usb: udc: allow adding and removing the same gadget device

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 04 Apr 2017 10:47:38 +0300
From:   Felipe Balbi <balbi@...nel.org>
To:     Alan Stern <stern@...land.harvard.edu>,
        Roger Quadros <rogerq@...com>
Cc:     vivek.gautam@...eaurora.org, linux-usb@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 1/3] usb: udc: allow adding and removing the same gadget device


Hi,

Alan Stern <stern@...land.harvard.edu> writes:
> On Mon, 3 Apr 2017, Roger Quadros wrote:
>
>> allow usb_del_gadget_udc() and usb add_gadget_udc() to be called
>> repeatedly on the same gadget->dev structure.
>> 
>> We need to clear the gadget->dev structure so that kobject_init()
>> doesn't complain about already initialized object.
>> 
>> Signed-off-by: Roger Quadros <rogerq@...com>
>> ---
>>  drivers/usb/gadget/udc/core.c | 1 +
>>  1 file changed, 1 insertion(+)
>> 
>> diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c
>> index d685d82..efce68e 100644
>> --- a/drivers/usb/gadget/udc/core.c
>> +++ b/drivers/usb/gadget/udc/core.c
>> @@ -1273,6 +1273,7 @@ void usb_del_gadget_udc(struct usb_gadget *gadget)
>>  	flush_work(&gadget->work);
>>  	device_unregister(&udc->dev);
>>  	device_unregister(&gadget->dev);
>> +	memset(&gadget->dev, 0x00, sizeof(gadget->dev));
>>  }
>>  EXPORT_SYMBOL_GPL(usb_del_gadget_udc);
>
> Isn't this dangerous?  It's quite possible that the device_unregister() 

not on the gadget API, no.

> call on the previous line invokes the gadget->dev.release callback, 
> which might deallocate gadget.  If that happens, your new memset will 
> oops.

that won't happen. struct usb_gadget is a member of the UDC's private
structure, like this:

struct dwc3 {
	[...]
	struct usb_gadget	gadget;
	struct usb_gadget_driver *gadget_driver;
	[...]
};

I'm actually thinking that struct usb_gadget shouldn't have a struct
device at all. Just a pointer to a device, that would solve all these
issues.

-- 
balbi

Download attachment "signature.asc" of type "application/pgp-signature" (833 bytes)