Messages in this thread | | | Date | Tue, 24 Nov 2015 10:53:33 -0500 (EST) | From | Alan Stern <> | Subject | Re: [PATCH v7 4/4] usb: gadget: udc-core: independent registration of gadgets and gadget drivers |
| |
On Tue, 24 Nov 2015, Peter Chen wrote:
> > > @@ -403,6 +408,18 @@ int usb_add_gadget_udc_release(struct device *parent, struct usb_gadget *gadget, > > > usb_gadget_set_state(gadget, USB_STATE_NOTATTACHED); > > > udc->vbus = true; > > > > > > + /* pick up one of pending gadget drivers */ > > > + list_for_each_entry(driver, &gadget_driver_pending_list, pending) { > > > + if (!driver->udc_name || strcmp(driver->udc_name, > > > + dev_name(&udc->dev)) == 0) { > > > + ret = udc_bind_to_driver(udc, driver); > > > + if (ret) > > > + goto err4; > > > + list_del(&driver->pending); > > > > This has to be list_del_init(). And somewhere in > > usb_gadget_probe_driver() you need to do > > INIT_LIST_HEAD(&driver->pending). > > > > > @@ -577,6 +596,10 @@ int usb_gadget_unregister_driver(struct usb_gadget_driver *driver) > > > break; > > > } > > > > > > + if (ret) { > > > + list_del(&driver->pending); > > > > Otherwise this will cause a crash or corrupt some random area of > > memory. > > > > Alan, would you please explain more what use case will cause memory > corruption for above code?
On Tue, 24 Nov 2015, Marek Szyprowski wrote:
> 'pending' handle is added to gadget_driver_pending_list in > usb_gadget_probe_driver(), > then when udc is available it is removed from this list and bound to the > given driver. > In usb_gadget_unregister_driver() the gadget is either unbound from the > udc or if is > was not bound to any udc, removed from pending list. I see no need for > adding > INIT_LIST_HEAD.
I'm sorry, you guys are right. I didn't notice the "if (ret) {" line added to usb_gadget_unregister_driver(). That line insures that we don't try to remove the driver from the pending list unless it really is on the list.
So never mind...
Alan Stern
| |