Re: [PATCH v8 2/2] usb: cdnsp: Add support for device-only configuration

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] In `cdnsp_pci_probe()`, the error path unconditionally frees the shared `wrap` structure if `platform_device_register_full()` fails, causing a use-after-free and double-free vulnerability.
--

commit 4dafbda57ec59e424fb1db39575dac6fe3174dd5
Author: Pawel Laszczak <pawell@cadence.com>

usb: cdnsp: Add support for device-only configuration

This patch introduces support for the Cadence USBSSP (cdnsp) controller in
hardware configurations where the Dual-Role Device (DRD) register block is
not implemented or is inaccessible.

Closes: https://lore.kernel.org/oe-kbuild-all/202605141023.18vWXyw3-lkp@intel.com/

> diff --git a/drivers/usb/cdns3/cdnsp-pci.c b/drivers/usb/cdns3/cdnsp-pci.c
> index 432007cfe6954..1a71dee5044cc 100644
> --- a/drivers/usb/cdns3/cdnsp-pci.c
> +++ b/drivers/usb/cdns3/cdnsp-pci.c
> @@ -93,7 +104,7 @@ static int cdnsp_pci_probe(struct pci_dev *pdev,
>  
>  	pci_set_master(pdev);
>  
> -	if (pci_is_enabled(func)) {
> +	if (func && pci_is_enabled(func)) {
>  		wrap = pci_get_drvdata(func);
>  	} else {
>  		wrap = kzalloc_obj(*wrap);

[ ... ]

> @@ -158,6 +183,7 @@ static int cdnsp_pci_probe(struct pci_dev *pdev,
>  		plat_info.dma_mask = pdev->dma_mask;
>  		plat_info.data = &pdata;
>  		plat_info.size_data = sizeof(pdata);
> +		plat_info.properties = wrap->prop;
>  		wrap->devfn = pdev->devfn;
>  		/* register platform device */
>  		wrap->plat_dev = platform_device_register_full(&plat_info);

This might be a pre-existing issue rather than a new regression, but is there
a use-after-free risk in the error handling immediately following this
platform device registration?

If probing a dual-function device, the first PCI function allocates the
shared wrap structure, stores it in its drvdata, and completes its probe.
When the second PCI function probes, it retrieves the shared wrap from the
first function and calls platform_device_register_full().

If that registration fails:

    wrap->plat_dev = platform_device_register_full(&plat_info);
    if (IS_ERR(wrap->plat_dev)) {
        ret = PTR_ERR(wrap->plat_dev);
        kfree(wrap);
        goto put_pci;
    }

Does the unconditional kfree(wrap) here free the memory that the first PCI
function's drvdata still points to?

If so, when the first function is later unbound, cdnsp_pci_remove() might
retrieve the dangling wrap pointer and eventually call kfree(wrap) again,
causing a double-free. Should the error path only free wrap if it was
allocated exclusively during the current probe execution?

> @@ -185,13 +211,17 @@ static void cdnsp_pci_remove(struct pci_dev *pdev)
>  	if (wrap->devfn == pdev->devfn)
>  		platform_device_unregister(wrap->plat_dev);
>  
> -	if (!pci_is_enabled(func))
> +	if (!func || !pci_is_enabled(func))
>  		kfree(wrap);
>  
>  	pci_dev_put(func);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260515-b4-no_drd_config-v8-0-75cb05317679@cadence.com?part=2