Re: [PATCH v2 11/11] xhci: Prevent invalid vdev dereference during sideband unregister

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] xhci: TOCTOU race and Use-After-Free in sideband registration
--

commit 7085c7c59881c747b14aeec07262203621aaa8b5
Author: Mathias Nyman <mathias.nyman@linux.intel.com>
xhci: Prevent invalid vdev dereference during sideband unregister

This commit prevents the reallocation of the xhci virtual device in the setup
device error path if a sideband is registered. It aims to fix a kernel oops
caused by dereferencing a stale vdev pointer during USB audio disconnect.

Closes: https://lore.kernel.org/linux-usb/TYUPR06MB6217000B59003EDF233D7246D2B22@TYUPR06MB6217.apcprd06.prod.outlook.com/

> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -4435,10 +4435,13 @@ static int xhci_setup_device(struct usb_hcd *hcd, struct usb_device *udev,
>  		dev_warn(&udev->dev, "Device not responding to setup %s.\n", act);
>  
>  		mutex_unlock(&xhci->mutex);
> -		ret = xhci_disable_and_free_slot(xhci, udev->slot_id);
> -		if (!ret) {
> -			if (xhci_alloc_dev(hcd, udev) == 1)
> -				xhci_setup_addressable_virt_dev(xhci, udev);
> +
> +		if (!virt_dev->sideband) {
> +			ret = xhci_disable_and_free_slot(xhci, udev->slot_id);

[Severity: High]
Can this lockless check introduce a Time-Of-Check to Time-Of-Use (TOCTOU)
race condition leading to a use-after-free?

If an asynchronous caller like snd_usb_rediscover_devices() calls
xhci_sideband_register() concurrently, it acquires xhci->lock, reads
xhci->devs[slot_id], and attaches the sideband.

If this lockless check evaluates to true just before the sideband is
attached, we proceed to xhci_disable_and_free_slot() without the lock:

xhci_setup_device()
    if (!virt_dev->sideband) {
        xhci_disable_and_free_slot()
            xhci_free_virt_device()
                xhci->devs[slot_id] = NULL;
                kfree(dev);

Because xhci_free_virt_device() does not hold xhci->lock, it can free the
device immediately after xhci_sideband_register() attaches the sideband,
leaving the sideband with a dangling pointer to a freed vdev.

> +			if (!ret) {
> +				if (xhci_alloc_dev(hcd, udev) == 1)
> +					xhci_setup_addressable_virt_dev(xhci, udev);
> +			}
>  		}
>  		kfree(command->completion);
>  		kfree(command);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261009151624.618967-1-mathias.nyman@linux.intel.com?part=11