> On Thu, Jul 30, 2020 at 03:28:09AM +0000, Peter Chen wrote: > > On 20-07-29 16:22:31, Alan Stern wrote: > > > Abusing the kernel's device model, some UDC drivers (including > > > dwc3 and cdns3) register and unregister their gadget structures > > > multiple times. This is strictly forbidden; device structures may > > > not be reused. > > > > Register and unregister gadget structures multiple times should be > > allowed if we pass a clean (zeroed) gadget device structure. I checked > > the cdns3 code (cdns3_gadget_start), it always zeroed struct > > usb_gadget before calling usb_add_gadget_udc when start device mode. > > How do you "know" that the structure really was properly freed/released by the > driver core at that point in time? > > That's the issue, even if you do unregister it, the driver core, or any other part of > the kernel, can hold on to the memory for an unbounded amount of time, due to the > fact that this is a reference counted pointer. > Yes, I find many UDC drivers have issues for that. The UDC driver's .remove has no sync with device reference counter for gadget device, they free device private structure which contains gadget device unconditional without considering device reference counter for gadget device. The UDC driver may need to call get_device/put_device for &gadget->dev to sync with device core, and at gadget->dev .release callback, free the whole device's private structure. So, a common .release (usb_udc_nop_release) callback at core.c may not suitable for most of UDC driver. > So please, never "recycle" memory structures like this. The documentation for the > kernel explicitly says "do not do this!" Yes, "recycle" memory structures have issue, but at cdns3 driver(cdns3/gadget.c), it does not use the same memory, it always allocates a NEW memory region for device structure before adding to device core. Peter
