linux-kernel - Re: Re: [PATCH v2] usb: fix reference leak in usb_new

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-Id: <20241218032242.2969330-1-make_ruc2021@163.com>
Date: Wed, 18 Dec 2024 11:22:42 +0800
From: Ma Ke <make_ruc2021@....com>
To: stern@...land.harvard.edu
Cc: christophe.jaillet@...adoo.fr,
	gregkh@...uxfoundation.org,
	javier.carrasco@...fvision.net,
	kay.sievers@...y.org,
	linux-kernel@...r.kernel.org,
	linux-usb@...r.kernel.org,
	make_ruc2021@....com,
	mka@...omium.org,
	oneukum@...e.com,
	quic_ugoswami@...cinc.com,
	stable@...r.kernel.org,
	stanley_chang@...ltek.com
Subject: Re: Re: [PATCH v2] usb: fix reference leak in usb_new_device()

Alan Stern<stern@...land.harvard.edu> wrote:
> Ma Ke <make_ruc2021@....com> 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