[PATCH] media: stm32: dcmi: fix some error handling bugs in probe()

Dan Carpenter posted 1 patch 1 week ago
drivers/media/platform/st/stm32/stm32-dcmi.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
[PATCH] media: stm32: dcmi: fix some error handling bugs in probe()
Posted by Dan Carpenter 1 week ago
There are a few issues here:

1) After we assign:
        chan = dma_request_chan(&pdev->dev, "tx");
   Then the error paths need to clean up before returning.  The first
   error path does a direct return.
2) The error paths check "dcmi->mdma_chan" but that is not assigned
   until later so it results in memory leaks.  Test "mdma_chan"
   instead.
3) The error handling calls dma_release_channel(dcmi->dma_chan) before
   "dcmi->dma_chan" has been assigned which leads to a NULL pointer
   dereference.  Use the "chan" variable instead.

I also moved the call to dma_release_channel() after the call to
dma_release_channel() so it mirrors the allocation code better.

Fixes: bc901885fae0 ("media: stm32: dcmi: perform dmaengine_slave_config at probe")
Signed-off-by: Dan Carpenter <error27@gmail.com>
---
From static analysis.  Untested.

 drivers/media/platform/st/stm32/stm32-dcmi.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/media/platform/st/stm32/stm32-dcmi.c b/drivers/media/platform/st/stm32/stm32-dcmi.c
index eeb0199864dd..c9f08b2465be 100644
--- a/drivers/media/platform/st/stm32/stm32-dcmi.c
+++ b/drivers/media/platform/st/stm32/stm32-dcmi.c
@@ -2024,8 +2024,10 @@ static int dcmi_probe(struct platform_device *pdev)
 	mdma_chan = dma_request_chan(&pdev->dev, "mdma_tx");
 	if (IS_ERR(mdma_chan)) {
 		ret = PTR_ERR(mdma_chan);
-		if (ret != -ENODEV)
-			return dev_err_probe(&pdev->dev, ret, "Failed to request MDMA channel\n");
+		if (ret != -ENODEV) {
+			dev_err_probe(&pdev->dev, ret, "Failed to request MDMA channel\n");
+			goto err_release_chan;
+		}
 		mdma_chan = NULL;
 	}
 
@@ -2206,12 +2208,13 @@ static int dcmi_probe(struct platform_device *pdev)
 err_media_device_cleanup:
 	media_device_cleanup(&dcmi->mdev);
 err_mdma_slave_config:
-	if (dcmi->mdma_chan)
+	if (mdma_chan)
 		gen_pool_free(dcmi->sram_pool, (unsigned long)dcmi->sram_buf, dcmi->sram_buf_size);
 err_dma_slave_config:
-	dma_release_channel(dcmi->dma_chan);
-	if (dcmi->mdma_chan)
+	if (mdma_chan)
 		dma_release_channel(mdma_chan);
+err_release_chan:
+	dma_release_channel(chan);
 
 	return ret;
 }
-- 
2.53.0
Re: [PATCH] media: stm32: dcmi: fix some error handling bugs in probe()
Posted by Alain Volmat 3 days, 19 hours ago
Hi Dan,

thanks for the patch.

On Fri, Jul 17, 2026 at 12:12:29PM +0300, Dan Carpenter wrote:
> There are a few issues here:
> 
> 1) After we assign:
>         chan = dma_request_chan(&pdev->dev, "tx");
>    Then the error paths need to clean up before returning.  The first
>    error path does a direct return.
> 2) The error paths check "dcmi->mdma_chan" but that is not assigned
>    until later so it results in memory leaks.  Test "mdma_chan"
>    instead.
> 3) The error handling calls dma_release_channel(dcmi->dma_chan) before
>    "dcmi->dma_chan" has been assigned which leads to a NULL pointer
>    dereference.  Use the "chan" variable instead.
> 
> I also moved the call to dma_release_channel() after the call to
> dma_release_channel() so it mirrors the allocation code better.
> 
> Fixes: bc901885fae0 ("media: stm32: dcmi: perform dmaengine_slave_config at probe")
> Signed-off-by: Dan Carpenter <error27@gmail.com>

Acked-by: Alain Volmat <alain.volmat@foss.st.com>

> ---
> From static analysis.  Untested.
> 
>  drivers/media/platform/st/stm32/stm32-dcmi.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/media/platform/st/stm32/stm32-dcmi.c b/drivers/media/platform/st/stm32/stm32-dcmi.c
> index eeb0199864dd..c9f08b2465be 100644
> --- a/drivers/media/platform/st/stm32/stm32-dcmi.c
> +++ b/drivers/media/platform/st/stm32/stm32-dcmi.c
> @@ -2024,8 +2024,10 @@ static int dcmi_probe(struct platform_device *pdev)
>  	mdma_chan = dma_request_chan(&pdev->dev, "mdma_tx");
>  	if (IS_ERR(mdma_chan)) {
>  		ret = PTR_ERR(mdma_chan);
> -		if (ret != -ENODEV)
> -			return dev_err_probe(&pdev->dev, ret, "Failed to request MDMA channel\n");
> +		if (ret != -ENODEV) {
> +			dev_err_probe(&pdev->dev, ret, "Failed to request MDMA channel\n");
> +			goto err_release_chan;
> +		}
>  		mdma_chan = NULL;
>  	}
>  
> @@ -2206,12 +2208,13 @@ static int dcmi_probe(struct platform_device *pdev)
>  err_media_device_cleanup:
>  	media_device_cleanup(&dcmi->mdev);
>  err_mdma_slave_config:
> -	if (dcmi->mdma_chan)
> +	if (mdma_chan)
>  		gen_pool_free(dcmi->sram_pool, (unsigned long)dcmi->sram_buf, dcmi->sram_buf_size);
>  err_dma_slave_config:
> -	dma_release_channel(dcmi->dma_chan);
> -	if (dcmi->mdma_chan)
> +	if (mdma_chan)
>  		dma_release_channel(mdma_chan);
> +err_release_chan:
> +	dma_release_channel(chan);
>  
>  	return ret;
>  }
> -- 
> 2.53.0
> 

Regards,
Alain