Re: Re: [PATCH v2] usb: fix reference leak in usb_new_device()

[Ma Ke]

Alan Stern<stern@xxxxxxxxxxxxxxxxxxx> wrote:
> Ma Ke <make_ruc2021@xxxxxxx> writes:
> > When device_add(&udev->dev) failed, calling put_device() to explicitly
> > release udev->dev. And the routine which calls usb_new_device() does
> > not call put_device() when an error occurs.
> 
> That is wrong.
> 
> usb_new_device() is called by hub_port_connect().  The code does:
> 
> 			status = usb_new_device(udev);
> 			...
> 
> 		if (status)
> 			goto loop_disable;
> 		...
> 
> loop_disable:
> 		hub_port_disable(hub, port1, 1);
> loop:
> 		usb_ep0_reinit(udev);
> 		release_devnum(udev);
> 		hub_free_dev(udev);
> 		if (retry_locked) {
> 			mutex_unlock(hcd->address0_mutex);
> 			usb_unlock_port(port_dev);
> 		}
> 		usb_put_dev(udev);
> 
> And usb_put_dev() is defined in usb.c as:
> 
> void usb_put_dev(struct usb_device *dev)
> {
> 	if (dev)
> 		put_device(&dev->dev);
> }
> 
> So you see, if usb_new_device() returns a nonzero value then 
> put_device() _is_ called.
> 
> >  As comment of device_add()
> > says, 'if device_add() succeeds, you should call device_del() when you
> > want to get rid of it. If device_add() has not succeeded, use only
> > put_device() to drop the reference count'.
> 
> You are correct that if device_add() succeeds and a later call fails, 
> then usb_new_device() does not properly call device_del().  Please 
> rewrite your patch to fix only that problem.
> 
> Alan Stern
Thank you for guiding me on the vulnerability I submitted. I will 
resubmit the patch based on your guidance and suggestions.
--
Regards,

Ma Ke