[PATCH 1/2] usb: dwc3: st: fix probed platform device ref count on probe error path

[PATCH 1/2] usb: dwc3: st: fix probed platform device ref count on probe error path

[Krzysztof Kozlowski]

The probe function never performs any paltform device allocation, thus
error path "undo_platform_dev_alloc" is entirely bogus.  It drops the
reference count from the platform device being probed.  If error path is
triggered, this will lead to unbalanced device reference counts and
premature release of device resources, thus possible use-after-free when
releasing remaining devm-managed resources.

Fixes: f83fca0707c6 ("usb: dwc3: add ST dwc3 glue layer to manage dwc3 HC")
Cc: <stable@vger.kernel.org>
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 drivers/usb/dwc3/dwc3-st.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-st.c b/drivers/usb/dwc3/dwc3-st.c
index 211360eee95a..a9cb04043f08 100644
--- a/drivers/usb/dwc3/dwc3-st.c
+++ b/drivers/usb/dwc3/dwc3-st.c
@@ -219,10 +219,8 @@ static int st_dwc3_probe(struct platform_device *pdev)
 	dwc3_data->regmap = regmap;
 
 	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "syscfg-reg");
-	if (!res) {
-		ret = -ENXIO;
-		goto undo_platform_dev_alloc;
-	}
+	if (!res)
+		return -ENXIO;
 
 	dwc3_data->syscfg_reg_off = res->start;
 
@@ -233,8 +231,7 @@ static int st_dwc3_probe(struct platform_device *pdev)
 		devm_reset_control_get_exclusive(dev, "powerdown");
 	if (IS_ERR(dwc3_data->rstc_pwrdn)) {
 		dev_err(&pdev->dev, "could not get power controller\n");
-		ret = PTR_ERR(dwc3_data->rstc_pwrdn);
-		goto undo_platform_dev_alloc;
+		return PTR_ERR(dwc3_data->rstc_pwrdn);
 	}
 
 	/* Manage PowerDown */
@@ -300,8 +297,6 @@ static int st_dwc3_probe(struct platform_device *pdev)
 	reset_control_assert(dwc3_data->rstc_rst);
 undo_powerdown:
 	reset_control_assert(dwc3_data->rstc_pwrdn);
-undo_platform_dev_alloc:
-	platform_device_put(pdev);
 	return ret;
 }
 
-- 
2.43.0

[PATCH 2/2] usb: dwc3: st: add missing depopulate in probe error path

[Krzysztof Kozlowski]

Depopulate device in probe error paths to fix leak of children
resources.

Fixes: f83fca0707c6 ("usb: dwc3: add ST dwc3 glue layer to manage dwc3 HC")
Cc: <stable@vger.kernel.org>
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

---

Context of my other cleanup patches (separate series to be sent soon)
will depend on this.
---
 drivers/usb/dwc3/dwc3-st.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/dwc3/dwc3-st.c b/drivers/usb/dwc3/dwc3-st.c
index a9cb04043f08..c8c7cd0c1796 100644
--- a/drivers/usb/dwc3/dwc3-st.c
+++ b/drivers/usb/dwc3/dwc3-st.c
@@ -266,7 +266,7 @@ static int st_dwc3_probe(struct platform_device *pdev)
 	if (!child_pdev) {
 		dev_err(dev, "failed to find dwc3 core device\n");
 		ret = -ENODEV;
-		goto err_node_put;
+		goto depopulate;
 	}
 
 	dwc3_data->dr_mode = usb_get_dr_mode(&child_pdev->dev);
@@ -282,6 +282,7 @@ static int st_dwc3_probe(struct platform_device *pdev)
 	ret = st_dwc3_drd_init(dwc3_data);
 	if (ret) {
 		dev_err(dev, "drd initialisation failed\n");
+		of_platform_depopulate(dev);
 		goto undo_softreset;
 	}
 
@@ -291,6 +292,8 @@ static int st_dwc3_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, dwc3_data);
 	return 0;
 
+depopulate:
+	of_platform_depopulate(dev);
 err_node_put:
 	of_node_put(child);
 undo_softreset:
-- 
2.43.0

Re: [PATCH 2/2] usb: dwc3: st: add missing depopulate in probe error path

[Patrice CHOTARD]

On 8/14/24 11:39, Krzysztof Kozlowski wrote:
> Depopulate device in probe error paths to fix leak of children
> resources.
> 
> Fixes: f83fca0707c6 ("usb: dwc3: add ST dwc3 glue layer to manage dwc3 HC")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> 
> ---
> 
> Context of my other cleanup patches (separate series to be sent soon)
> will depend on this.
> ---
>  drivers/usb/dwc3/dwc3-st.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/dwc3/dwc3-st.c b/drivers/usb/dwc3/dwc3-st.c
> index a9cb04043f08..c8c7cd0c1796 100644
> --- a/drivers/usb/dwc3/dwc3-st.c
> +++ b/drivers/usb/dwc3/dwc3-st.c
> @@ -266,7 +266,7 @@ static int st_dwc3_probe(struct platform_device *pdev)
>  	if (!child_pdev) {
>  		dev_err(dev, "failed to find dwc3 core device\n");
>  		ret = -ENODEV;
> -		goto err_node_put;
> +		goto depopulate;
>  	}
>  
>  	dwc3_data->dr_mode = usb_get_dr_mode(&child_pdev->dev);
> @@ -282,6 +282,7 @@ static int st_dwc3_probe(struct platform_device *pdev)
>  	ret = st_dwc3_drd_init(dwc3_data);
>  	if (ret) {
>  		dev_err(dev, "drd initialisation failed\n");
> +		of_platform_depopulate(dev);
>  		goto undo_softreset;
>  	}
>  
> @@ -291,6 +292,8 @@ static int st_dwc3_probe(struct platform_device *pdev)
>  	platform_set_drvdata(pdev, dwc3_data);
>  	return 0;
>  
> +depopulate:
> +	of_platform_depopulate(dev);
>  err_node_put:
>  	of_node_put(child);
>  undo_softreset:

Reviewed-by: Patrice Chotard <patrice.chotard@foss.st.com>

Thanks
Patrice

Re: [PATCH 2/2] usb: dwc3: st: add missing depopulate in probe error path

[Thinh Nguyen]

On Wed, Aug 14, 2024, Krzysztof Kozlowski wrote:
> Depopulate device in probe error paths to fix leak of children
> resources.
> 
> Fixes: f83fca0707c6 ("usb: dwc3: add ST dwc3 glue layer to manage dwc3 HC")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> 
> ---
> 
> Context of my other cleanup patches (separate series to be sent soon)
> will depend on this.
> ---
>  drivers/usb/dwc3/dwc3-st.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/dwc3/dwc3-st.c b/drivers/usb/dwc3/dwc3-st.c
> index a9cb04043f08..c8c7cd0c1796 100644
> --- a/drivers/usb/dwc3/dwc3-st.c
> +++ b/drivers/usb/dwc3/dwc3-st.c
> @@ -266,7 +266,7 @@ static int st_dwc3_probe(struct platform_device *pdev)
>  	if (!child_pdev) {
>  		dev_err(dev, "failed to find dwc3 core device\n");
>  		ret = -ENODEV;
> -		goto err_node_put;
> +		goto depopulate;
>  	}
>  
>  	dwc3_data->dr_mode = usb_get_dr_mode(&child_pdev->dev);
> @@ -282,6 +282,7 @@ static int st_dwc3_probe(struct platform_device *pdev)
>  	ret = st_dwc3_drd_init(dwc3_data);
>  	if (ret) {
>  		dev_err(dev, "drd initialisation failed\n");
> +		of_platform_depopulate(dev);
>  		goto undo_softreset;
>  	}
>  
> @@ -291,6 +292,8 @@ static int st_dwc3_probe(struct platform_device *pdev)
>  	platform_set_drvdata(pdev, dwc3_data);
>  	return 0;
>  
> +depopulate:
> +	of_platform_depopulate(dev);
>  err_node_put:
>  	of_node_put(child);
>  undo_softreset:
> -- 
> 2.43.0
> 

Acked-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>

Thanks,
Thinh

Re: [PATCH 1/2] usb: dwc3: st: fix probed platform device ref count on probe error path

[Patrice CHOTARD]

On 8/14/24 11:39, Krzysztof Kozlowski wrote:
> The probe function never performs any paltform device allocation, thus

Hi Krzysztof

s/paltform/platform

> error path "undo_platform_dev_alloc" is entirely bogus.  It drops the
> reference count from the platform device being probed.  If error path is
> triggered, this will lead to unbalanced device reference counts and
> premature release of device resources, thus possible use-after-free when
> releasing remaining devm-managed resources.
> 
> Fixes: f83fca0707c6 ("usb: dwc3: add ST dwc3 glue layer to manage dwc3 HC")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
>  drivers/usb/dwc3/dwc3-st.c | 11 +++--------
>  1 file changed, 3 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/dwc3-st.c b/drivers/usb/dwc3/dwc3-st.c
> index 211360eee95a..a9cb04043f08 100644
> --- a/drivers/usb/dwc3/dwc3-st.c
> +++ b/drivers/usb/dwc3/dwc3-st.c
> @@ -219,10 +219,8 @@ static int st_dwc3_probe(struct platform_device *pdev)
>  	dwc3_data->regmap = regmap;
>  
>  	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "syscfg-reg");
> -	if (!res) {
> -		ret = -ENXIO;
> -		goto undo_platform_dev_alloc;
> -	}
> +	if (!res)
> +		return -ENXIO;
>  
>  	dwc3_data->syscfg_reg_off = res->start;
>  
> @@ -233,8 +231,7 @@ static int st_dwc3_probe(struct platform_device *pdev)
>  		devm_reset_control_get_exclusive(dev, "powerdown");
>  	if (IS_ERR(dwc3_data->rstc_pwrdn)) {
>  		dev_err(&pdev->dev, "could not get power controller\n");
> -		ret = PTR_ERR(dwc3_data->rstc_pwrdn);
> -		goto undo_platform_dev_alloc;
> +		return PTR_ERR(dwc3_data->rstc_pwrdn);
>  	}
>  
>  	/* Manage PowerDown */
> @@ -300,8 +297,6 @@ static int st_dwc3_probe(struct platform_device *pdev)
>  	reset_control_assert(dwc3_data->rstc_rst);
>  undo_powerdown:
>  	reset_control_assert(dwc3_data->rstc_pwrdn);
> -undo_platform_dev_alloc:
> -	platform_device_put(pdev);
>  	return ret;
>  }
>  

Reviewed-by: Patrice Chotard <patrice.chotard@foss.st.com>

Thanks
Patrice

Re: [PATCH 1/2] usb: dwc3: st: fix probed platform device ref count on probe error path

[Thinh Nguyen]

On Wed, Aug 14, 2024, Krzysztof Kozlowski wrote:
> The probe function never performs any paltform device allocation, thus
> error path "undo_platform_dev_alloc" is entirely bogus.  It drops the
> reference count from the platform device being probed.  If error path is
> triggered, this will lead to unbalanced device reference counts and
> premature release of device resources, thus possible use-after-free when
> releasing remaining devm-managed resources.
> 
> Fixes: f83fca0707c6 ("usb: dwc3: add ST dwc3 glue layer to manage dwc3 HC")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
>  drivers/usb/dwc3/dwc3-st.c | 11 +++--------
>  1 file changed, 3 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/dwc3-st.c b/drivers/usb/dwc3/dwc3-st.c
> index 211360eee95a..a9cb04043f08 100644
> --- a/drivers/usb/dwc3/dwc3-st.c
> +++ b/drivers/usb/dwc3/dwc3-st.c
> @@ -219,10 +219,8 @@ static int st_dwc3_probe(struct platform_device *pdev)
>  	dwc3_data->regmap = regmap;
>  
>  	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "syscfg-reg");
> -	if (!res) {
> -		ret = -ENXIO;
> -		goto undo_platform_dev_alloc;
> -	}
> +	if (!res)
> +		return -ENXIO;
>  
>  	dwc3_data->syscfg_reg_off = res->start;
>  
> @@ -233,8 +231,7 @@ static int st_dwc3_probe(struct platform_device *pdev)
>  		devm_reset_control_get_exclusive(dev, "powerdown");
>  	if (IS_ERR(dwc3_data->rstc_pwrdn)) {
>  		dev_err(&pdev->dev, "could not get power controller\n");
> -		ret = PTR_ERR(dwc3_data->rstc_pwrdn);
> -		goto undo_platform_dev_alloc;
> +		return PTR_ERR(dwc3_data->rstc_pwrdn);
>  	}
>  
>  	/* Manage PowerDown */
> @@ -300,8 +297,6 @@ static int st_dwc3_probe(struct platform_device *pdev)
>  	reset_control_assert(dwc3_data->rstc_rst);
>  undo_powerdown:
>  	reset_control_assert(dwc3_data->rstc_pwrdn);
> -undo_platform_dev_alloc:
> -	platform_device_put(pdev);
>  	return ret;
>  }
>  
> -- 
> 2.43.0
> 

Acked-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>

Thanks,
Thinh