Re: [PATCH v3 1/2] usb: xhci-pci: add generic auxiliary device interface

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Lockless modification of devres payload in `xhci_pci_try_remove_aux_device` bypasses the subsystem's devres locking mechanism.
--

commit bb8a85242e2a88f6613cf1810360076073e5ac66
Author: Jihong Min <hurryman2212@gmail.com>

usb: xhci-pci: add generic auxiliary device interface

This commit adds a small auxiliary device registration path for selected
xHCI PCI controllers like the AMD Promontory 21. It creates an auxiliary
device and stores it in devres so subsystem-specific child drivers can bind
to controller-specific functionality outside xhci-pci.

> diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
> index 585b2f3117b08..618d6840e1083 100644
> --- a/drivers/usb/host/xhci-pci.c
> +++ b/drivers/usb/host/xhci-pci.c
[ ... ]
> @@ -103,6 +106,80 @@ static int xhci_pci_run(struct usb_hcd *hcd);
[ ... ]
> +static void xhci_pci_try_remove_aux_device(struct pci_dev *pdev)
> +{
> +	struct xhci_pci_aux_devres *devres;
> +
> +	devres = devres_find(&pdev->dev, xhci_pci_aux_devres_release, NULL,
> +			     NULL);
> +	if (!devres || !devres->auxdev)
> +		return;
> +
> +	auxiliary_device_destroy(devres->auxdev);
> +	devres->auxdev = NULL;

Is it safe to locklessly modify the devres payload here?

The code uses devres_find() to obtain the devres object and manually clears
devres->auxdev after destroying the auxiliary device. This bypasses the
devres subsystem's internal spinlocks and leaves a dead devres node in the
device's list until standard devres_release_all() runs.

Should this use devres_release() instead? Calling devres_release() would
safely acquire the required locks, invoke the callback, remove the node
from the list, and free the memory.

> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1778123510.git.hurryman2212@gmail.com?part=1