Message ID | 1491222031-18120-2-git-send-email-rogerq@ti.com |
---|---|
State | New |
Headers | show |
Series | usb: dwc3: dual-role support | expand |
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@ti.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() call on the previous line invokes the gadget->dev.release callback, which might deallocate gadget. If that happens, your new memset will oops. In general, if an object relies on reference counting for its lifetime, you cannot register and unregister it more than once. A typical issue is that some code retains a reference to the old instance and tries to use it after the new instance has been registered, thereby messing up the new instance. I don't know if that is possible in this case, but it is something to watch out for. Alan Stern
Hi, Alan Stern <stern@rowland.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@ti.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
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);
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@ti.com> --- drivers/usb/gadget/udc/core.c | 1 + 1 file changed, 1 insertion(+) -- 2.7.4