Re: [PATCH v2 6/6] Revert "usb: udc: allow adding and removing the same gadget device"

["stern@rowland.harvard.edu" <stern@rowland.harvard.edu>]

On Wed, Aug 19, 2020 at 01:31:14AM +0000, Peter Chen wrote:
> On 20-08-18 10:46:55, stern@rowland.harvard.edu wrote:
> > On Tue, Aug 18, 2020 at 10:05:51AM +0000, Peter Chen wrote:
> > >  
> > > > >
> > > > > diff --git a/drivers/usb/gadget/udc/core.c
> > > > > b/drivers/usb/gadget/udc/core.c index 473e74088b1f..43351b0af569
> > > > > 100644
> > > > > --- a/drivers/usb/gadget/udc/core.c
> > > > > +++ b/drivers/usb/gadget/udc/core.c
> > > > > @@ -1386,7 +1386,6 @@ void usb_del_gadget_udc(struct usb_gadget
> > > > > *gadget)  {
> > > > >  	usb_del_gadget(gadget);
> > > > >  	usb_put_gadget(gadget);
> > > > > -	memset(&gadget->dev, 0x00, sizeof(gadget->dev));
> > > > 
> > > > Shouldn't you do this patch earlier in the series, as the
> > > > usb_put_gadget() call could have freed the memory that is being cleared here?
> > > > 
> > > 
> > > If I did it earlier, it would cause dwc3 break if people do 'git bisect', dwc3 issue is
> > > fixed at patch 5.
> > 
> > If you use the patch I posted earlier:
> > 
> > 	https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmarc.info%2F%3Fl%3Dlinux-usb%26m%3D159605415210351%26w%3D2&data=02%7C01%7Cpeter.chen%40nxp.com%7C84c12532be684ba94c1708d843858e86%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637333588196922016&sdata=gOe5kecj38gR9qIbkfjVkNO%2FICp0bHis30Yi2tomrc8%3D&reserved=0
> > 
> > instead of this one then dwc3 would continue to work correctly during 
> > the intermediate stages of the series.
> > 
> 
> But at last, we don't want below at .release function
> 
> 	memset(dev, 0, sizeof(*dev));
> 
> It still needs another patch to delete it after dwc3 changes,
> and it changes .release function name to usb_udc_zero_release,
> this change may also not be needed.
> 
> Or I only do move memory clear operation at the first patch, and
> delete it at the last patch, it could let the reader not see
> the memory clear operation at the usb_del_gadget during the patch
> series.

One way or another, the existing code is wrong.  I guess the best we can 
do for now is to let it remain wrong during the patch series, rather 
than changing it to be wrong in a different way.

To put it another way, we already run the risk of clearing memory that 
has been freed.  The series does not make that risk any worse, and it 
eventually fixes the problem.

This means your patch should remain in its position at the end of the 
series.

Alan Stern