[PATCH] usb: cdns3: drop the second function reference after use

[PATCH] usb: cdns3: drop the second function reference after use

[Wentao Liang]

cdns3_get_second_fun() returns a struct pci_dev obtained with
pci_get_device(), which takes a reference on the device, and both
cdns3_pci_probe() and cdns3_pci_remove() keep that reference until they
return without ever dropping it.  Probing or removing either function of
the USBSS controller therefore pins the sibling PCI device and its
driver's module forever.  cdns3_get_second_fun() also leaks the
reference itself when the device it finds is not one of the two expected
functions.

Release the reference on every path that gives up ownership: in the
error return of cdns3_get_second_fun(), and at the end of the probe and
remove callbacks.

Fixes: 7733f6c32e36 ("usb: cdns3: Add Cadence USB3 DRD Driver")
Cc: stable@vger.kernel.org
Assisted-by: DeepSeek-V4.1-Flash and OpenAI Codex
Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
---
 drivers/usb/cdns3/cdns3-pci-wrap.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/cdns3/cdns3-pci-wrap.c b/drivers/usb/cdns3/cdns3-pci-wrap.c
index fd06a3d8e638..4891802a2567 100644
--- a/drivers/usb/cdns3/cdns3-pci-wrap.c
+++ b/drivers/usb/cdns3/cdns3-pci-wrap.c
@@ -59,6 +59,7 @@ static struct pci_dev *cdns3_get_second_fun(struct pci_dev *pdev)
 
 	if (func->devfn != PCI_DEV_FN_HOST_DEVICE &&
 	    func->devfn != PCI_DEV_FN_OTG) {
+		pci_dev_put(func);
 		return NULL;
 	}
 
@@ -89,6 +90,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);
+		pci_dev_put(func);
 		return err;
 	}
 
@@ -99,12 +101,15 @@ static int cdns3_pci_probe(struct pci_dev *pdev,
 		if (!wrap) {
 			dev_err(&pdev->dev,
 				"second function not initialized, retrying\n");
+			pci_dev_put(func);
 			return -EPROBE_DEFER;
 		}
 	} else {
 		wrap = kzalloc_obj(*wrap);
-		if (!wrap)
+		if (!wrap) {
+			pci_dev_put(func);
 			return -ENOMEM;
+		}
 	}
 
 	res = wrap->dev_res;
@@ -165,11 +170,13 @@ static int cdns3_pci_probe(struct pci_dev *pdev,
 		if (IS_ERR(wrap->plat_dev)) {
 			err = PTR_ERR(wrap->plat_dev);
 			kfree(wrap);
+			pci_dev_put(func);
 			return err;
 		}
 	}
 
 	pci_set_drvdata(pdev, wrap);
+	pci_dev_put(func);
 	return err;
 }
 
@@ -186,6 +193,8 @@ static void cdns3_pci_remove(struct pci_dev *pdev)
 
 	if (!pci_is_enabled(func))
 		kfree(wrap);
+
+	pci_dev_put(func);
 }
 
 static const struct pci_device_id cdns3_pci_ids[] = {
-- 
2.34.1

Re: [PATCH] usb: cdns3: drop the second function reference after use

[sashiko-bot]

> cdns3_get_second_fun() returns a struct pci_dev obtained with
> pci_get_device(), which takes a reference on the device, and both
> cdns3_pci_probe() and cdns3_pci_remove() keep that reference until they
> return without ever dropping it.  Probing or removing either function of
> the USBSS controller therefore pins the sibling PCI device and its
> driver's module forever.  cdns3_get_second_fun() also leaks the
> reference itself when the device it finds is not one of the two expected
> functions.
> 
> Release the reference on every path that gives up ownership: in the
> error return of cdns3_get_second_fun(), and at the end of the probe and
> remove callbacks.
> 
> Fixes: 7733f6c32e36 ("usb: cdns3: Add Cadence USB3 DRD Driver")
> Cc: stable@vger.kernel.org
> Assisted-by: DeepSeek-V4.1-Flash and OpenAI Codex
> Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261104021453.1468443-1-vulab@iscas.ac.cn?part=1

Re: [PATCH] usb: cdns3: drop the second function reference after use

[Peter Chen]

On 26-11-04 02:14:53, Wentao Liang wrote:
> cdns3_get_second_fun() returns a struct pci_dev obtained with
> pci_get_device(), which takes a reference on the device, and both
> cdns3_pci_probe() and cdns3_pci_remove() keep that reference until they
> return without ever dropping it.  Probing or removing either function of
> the USBSS controller therefore pins the sibling PCI device and its
> driver's module forever.  cdns3_get_second_fun() also leaks the
> reference itself when the device it finds is not one of the two expected
> functions.
> 
> Release the reference on every path that gives up ownership: in the
> error return of cdns3_get_second_fun(), and at the end of the probe and
> remove callbacks.
> 
> Fixes: 7733f6c32e36 ("usb: cdns3: Add Cadence USB3 DRD Driver")
> Cc: stable@vger.kernel.org
> Assisted-by: DeepSeek-V4.1-Flash and OpenAI Codex
> Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>

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

Peter
> ---
>  drivers/usb/cdns3/cdns3-pci-wrap.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/cdns3/cdns3-pci-wrap.c b/drivers/usb/cdns3/cdns3-pci-wrap.c
> index fd06a3d8e638..4891802a2567 100644
> --- a/drivers/usb/cdns3/cdns3-pci-wrap.c
> +++ b/drivers/usb/cdns3/cdns3-pci-wrap.c
> @@ -59,6 +59,7 @@ static struct pci_dev *cdns3_get_second_fun(struct pci_dev *pdev)
>  
>  	if (func->devfn != PCI_DEV_FN_HOST_DEVICE &&
>  	    func->devfn != PCI_DEV_FN_OTG) {
> +		pci_dev_put(func);
>  		return NULL;
>  	}
>  
> @@ -89,6 +90,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);
> +		pci_dev_put(func);
>  		return err;
>  	}
>  
> @@ -99,12 +101,15 @@ static int cdns3_pci_probe(struct pci_dev *pdev,
>  		if (!wrap) {
>  			dev_err(&pdev->dev,
>  				"second function not initialized, retrying\n");
> +			pci_dev_put(func);
>  			return -EPROBE_DEFER;
>  		}
>  	} else {
>  		wrap = kzalloc_obj(*wrap);
> -		if (!wrap)
> +		if (!wrap) {
> +			pci_dev_put(func);
>  			return -ENOMEM;
> +		}
>  	}
>  
>  	res = wrap->dev_res;
> @@ -165,11 +170,13 @@ static int cdns3_pci_probe(struct pci_dev *pdev,
>  		if (IS_ERR(wrap->plat_dev)) {
>  			err = PTR_ERR(wrap->plat_dev);
>  			kfree(wrap);
> +			pci_dev_put(func);
>  			return err;
>  		}
>  	}
>  
>  	pci_set_drvdata(pdev, wrap);
> +	pci_dev_put(func);
>  	return err;
>  }
>  
> @@ -186,6 +193,8 @@ static void cdns3_pci_remove(struct pci_dev *pdev)
>  
>  	if (!pci_is_enabled(func))
>  		kfree(wrap);
> +
> +	pci_dev_put(func);
>  }
>  
>  static const struct pci_device_id cdns3_pci_ids[] = {
> -- 
> 2.34.1
> 
> 

-- 

Thanks,
Peter Chen