Re: [PATCH 01/19] usb: core: sysfs: take the port lock to read the BOS

[Alan Stern]

On Tue, Oct 27, 2026 at 03:57:41PM +0100, Greg Kroah-Hartman wrote:
> From: Josef Bacik <josef@xxxxxxxxxxxxxx>
> 
> usb_reset_and_verify_device() sets udev->bos aside, lets hub_port_init()
> read a new BOS into udev->bos, compares the two, frees the new one and
> puts the old one back. bos_descriptors_read() takes the device lock
> against that since commit 4e0197fbb0ee ("usb: core: sysfs: add lock to
> bos_descriptors_read()").
> 
> This does not cover one caller. finish_port_resume() does a reset-resume
> if the device has the quirk, if its port lost power, or if the device
> does not answer GET_STATUS after a normal resume. In an autoresume this
> runs without the device lock, for instance from
> usb_autopm_get_interface() of an interface driver. The comment above
> usb_reset_and_verify_device() says that tasks which hold the device lock
> are expected to resume the device first.
> 
> bos_descriptors_read() does not. The file is readable for everybody, so
> a reader can meet the temporary BOS of a device which is being resumed:
> while usb_get_bos_descriptor() has not set ->desc yet (NULL
> dereference), before it has corrected a wTotalLength which is larger
> than the buffer (read beyond the buffer), or when it is freed (use after
> free).
> 
> The file was made to give the descriptors to everybody without USB I/O,
> so the reader should not wake up the device. There is a lock which both
> callers of usb_reset_and_verify_device() hold, the one of the port:
> usb_reset_device() takes it around the call and usb_port_resume() for
> the whole resume. Take it in the reader too, inside the device lock,
> which is the order of usb_reset_device(). A root hub has no port and is
> never reset this way.
> 
> The port belongs to the parent hub, which removes its ports after it has
> disconnected the children. usb_disconnect() cannot finish while the file
> is being read, device_del() waits for the reader.

This is not the right way to fix the problem.  It should fixed in the 
same as commit ff33299ec8bb ("USB: core: Fix race by not overwriting 
udev->descriptor in hub_port_init()") fixed the analogous problem with 
the device descriptor.

In this case, usb_reset_and_verify_device() should allocate temporary 
storage for the new BOS descriptor, and usb_get_bos_descriptor() should 
read the updated descriptor into that new location, leaving the original 
one alone.  The address of the temporary storage should be passed as a 
new argument to usb_get_bos_descriptor() (which means also passing it as 
a new argument to hub_port_init()).  The kerneldoc for 
usb_get_bos_descriptor() should be updated much like ff33299ec8bb 
updated the kerneldoc for hub_port_init().

Alan Stern