cdns3_get_second_fun() looks up the second PCI function with
pci_get_device(), which returns the device with a reference held, and
hands it back to its callers. Neither cdns3_pci_probe() nor
cdns3_pci_remove() ever drops that reference, so the second function's
pci_dev can never be released.

The cdnsp-pci.c driver handles the same situation correctly:
cdnsp_pci_probe() jumps to a 'put_pci:' label that calls
pci_dev_put(), and cdnsp_pci_remove() calls pci_dev_put() as well.

While at it, also add the missing NULL check in cdns3_pci_remove().
pci_is_enabled() dereferences its argument, so if
cdns3_get_second_fun() returns NULL, the current code may crashes.
cdnsp_pci_remove() guards with "!func ||".

Fixes: 7733f6c32e36 ("usb: cdns3: Add Cadence USB3 DRD Driver")
Cc: stable@vger.kernel.org
Signed-off-by: Ma Ke <make_ruc2021@163.com>
---
 drivers/usb/cdns3/cdns3-pci-wrap.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/cdns3/cdns3-pci-wrap.c b/drivers/usb/cdns3/cdns3-pci-wrap.c
index eb5760f75b9d..d18c15fb9365 100644
--- a/drivers/usb/cdns3/cdns3-pci-wrap.c
+++ b/drivers/usb/cdns3/cdns3-pci-wrap.c
@@ -89,7 +89,7 @@ static int cdns3_pci_probe(struct pci_dev *pdev,
 	err = pcim_enable_device(pdev);
 	if (err) {
 		dev_err(&pdev->dev, "Enabling PCI device has failed %d\n", err);
-		return err;
+		goto put_pci;
 	}
 
 	pci_set_master(pdev);
@@ -98,8 +98,10 @@ static int cdns3_pci_probe(struct pci_dev *pdev,
 		wrap = pci_get_drvdata(func);
 	} else {
 		wrap = kzalloc_obj(*wrap);
-		if (!wrap)
-			return -ENOMEM;
+		if (!wrap) {
+			err = -ENOMEM;
+			goto put_pci;
+		}
 	}
 
 	res = wrap->dev_res;
@@ -160,11 +162,13 @@ static int cdns3_pci_probe(struct pci_dev *pdev,
 		if (IS_ERR(wrap->plat_dev)) {
 			err = PTR_ERR(wrap->plat_dev);
 			kfree(wrap);
-			return err;
+			goto put_pci;
 		}
 	}
 
 	pci_set_drvdata(pdev, wrap);
+put_pci:
+	pci_dev_put(func);
 	return err;
 }
 
@@ -179,8 +183,10 @@ static void cdns3_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);
 }
 
 static const struct pci_device_id cdns3_pci_ids[] = {
-- 
2.43.0

On 26-09-14 19:23:20, Ma Ke wrote:
> cdns3_get_second_fun() looks up the second PCI function with
> pci_get_device(), which returns the device with a reference held, and
> hands it back to its callers. Neither cdns3_pci_probe() nor
> cdns3_pci_remove() ever drops that reference, so the second function's
> pci_dev can never be released.
> 
> The cdnsp-pci.c driver handles the same situation correctly:
> cdnsp_pci_probe() jumps to a 'put_pci:' label that calls
> pci_dev_put(), and cdnsp_pci_remove() calls pci_dev_put() as well.
> 
> While at it, also add the missing NULL check in cdns3_pci_remove().
> pci_is_enabled() dereferences its argument, so if
> cdns3_get_second_fun() returns NULL, the current code may crashes.
> cdnsp_pci_remove() guards with "!func ||".
> 
> Fixes: 7733f6c32e36 ("usb: cdns3: Add Cadence USB3 DRD Driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Ma Ke <make_ruc2021@163.com>

If you have used LLM tools, please add Assisted-by tag for it, otherwise:

Acked-by: Peter Chen <peter.chen@kernel.org>

Peter

> ---
>  drivers/usb/cdns3/cdns3-pci-wrap.c | 16 +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/usb/cdns3/cdns3-pci-wrap.c b/drivers/usb/cdns3/cdns3-pci-wrap.c
> index eb5760f75b9d..d18c15fb9365 100644
> --- a/drivers/usb/cdns3/cdns3-pci-wrap.c
> +++ b/drivers/usb/cdns3/cdns3-pci-wrap.c
> @@ -89,7 +89,7 @@ static int cdns3_pci_probe(struct pci_dev *pdev,
>  	err = pcim_enable_device(pdev);
>  	if (err) {
>  		dev_err(&pdev->dev, "Enabling PCI device has failed %d\n", err);
> -		return err;
> +		goto put_pci;
>  	}
>  
>  	pci_set_master(pdev);
> @@ -98,8 +98,10 @@ static int cdns3_pci_probe(struct pci_dev *pdev,
>  		wrap = pci_get_drvdata(func);
>  	} else {
>  		wrap = kzalloc_obj(*wrap);
> -		if (!wrap)
> -			return -ENOMEM;
> +		if (!wrap) {
> +			err = -ENOMEM;
> +			goto put_pci;
> +		}
>  	}
>  
>  	res = wrap->dev_res;
> @@ -160,11 +162,13 @@ static int cdns3_pci_probe(struct pci_dev *pdev,
>  		if (IS_ERR(wrap->plat_dev)) {
>  			err = PTR_ERR(wrap->plat_dev);
>  			kfree(wrap);
> -			return err;
> +			goto put_pci;
>  		}
>  	}
>  
>  	pci_set_drvdata(pdev, wrap);
> +put_pci:
> +	pci_dev_put(func);
>  	return err;
>  }
>  
> @@ -179,8 +183,10 @@ static void cdns3_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);
>  }
>  
>  static const struct pci_device_id cdns3_pci_ids[] = {
> -- 
> 2.43.0
> 

-- 

Thanks,
Peter Chen