dmaengine: ti: edma: Check return value of of_dma_controller_register

[PATCH] dmaengine: ti: edma: Check return value of of_dma_controller_register

Posted by Mikhail Arkhipov 1 year, 4 months ago

If of_dma_controller_register() fails within the edma_probe() function,
the driver does not check the return value or log the failure. This
oversight can cause other drivers that rely on this DMA controller to fail
during their probe phase. Specifically, when other drivers call
of_dma_request_slave_channel(), they may receive an error code
-EPROBE_DEFER, causing their initialization to be delayed or fail without
clear logging of the root cause.

Add a check for the return value of of_dma_controller_register() in the
edma_probe() function. If the function returns an error, log an appropriate
error message and handle the failure by cleaning up resources and returning
the error code. This ensures that the failure is properly reported, which
aids in debugging and maintains system stability.

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Fixes: dc9b60552f6a ("ARM/dmaengine: edma: Move of_dma_controller_register to the dmaengine driver")
Signed-off-by: Mikhail Arkhipov <m.arhipov@rosa.ru>
---
 drivers/dma/ti/edma.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/dma/ti/edma.c b/drivers/dma/ti/edma.c
index 5f8d2e93ff3f..9fcbd97d346f 100644
--- a/drivers/dma/ti/edma.c
+++ b/drivers/dma/ti/edma.c
@@ -2529,18 +2529,27 @@ static int edma_probe(struct platform_device *pdev)
 		if (ret) {
 			dev_err(dev, "memcpy ddev registration failed (%d)\n",
 				ret);
-			dma_async_device_unregister(&ecc->dma_slave);
-			goto err_reg1;
+			goto err_unregister_dma_slave;
 		}
 	}
 
-	if (node)
-		of_dma_controller_register(node, of_edma_xlate, ecc);
+	if (node) {
+		ret = of_dma_controller_register(node, of_edma_xlate, ecc);
+		if (ret) {
+			dev_err(dev, "Failed to register DMA controller (%d)\n", ret);
+			goto err_unregister_dma_memcpy;
+		}
+	}

 	dev_info(dev, "TI EDMA DMA engine driver\n");
 
 	return 0;
 
+err_unregister_dma_memcpy:
+	if (ecc->dma_memcpy)
+		dma_async_device_unregister(ecc->dma_memcpy);
+err_unregister_dma_slave:
+	dma_async_device_unregister(&ecc->dma_slave);
 err_reg1:
 	edma_free_slot(ecc, ecc->dummy_slot);
 err_disable_pm:
-- 
2.39.3 (Apple Git-146)

Re: [PATCH] dmaengine: ti: edma: Check return value of of_dma_controller_register

Posted by Vinod Koul 1 year, 3 months ago

On Mon, 23 Sep 2024 22:37:03 +0300, Mikhail Arkhipov wrote:
> If of_dma_controller_register() fails within the edma_probe() function,
> the driver does not check the return value or log the failure. This
> oversight can cause other drivers that rely on this DMA controller to fail
> during their probe phase. Specifically, when other drivers call
> of_dma_request_slave_channel(), they may receive an error code
> -EPROBE_DEFER, causing their initialization to be delayed or fail without
> clear logging of the root cause.
> 
> [...]

Applied, thanks!

[1/1] dmaengine: ti: edma: Check return value of of_dma_controller_register
      commit: 83158a3a712ad6ebdebd470d25af04c66c4e2223

Best regards,
-- 
~Vinod

Re: [PATCH] dmaengine: ti: edma: Check return value of of_dma_controller_register

Posted by Péter Ujfalusi 1 year, 4 months ago


On 23/09/2024 22:37, Mikhail Arkhipov wrote:
> If of_dma_controller_register() fails within the edma_probe() function,
> the driver does not check the return value or log the failure. This
> oversight can cause other drivers that rely on this DMA controller to fail
> during their probe phase. Specifically, when other drivers call
> of_dma_request_slave_channel(), they may receive an error code
> -EPROBE_DEFER, causing their initialization to be delayed or fail without
> clear logging of the root cause.
> 
> Add a check for the return value of of_dma_controller_register() in the
> edma_probe() function. If the function returns an error, log an appropriate
> error message and handle the failure by cleaning up resources and returning
> the error code. This ensures that the failure is properly reported, which
> aids in debugging and maintains system stability.
> 
> Found by Linux Verification Center (linuxtesting.org) with SVACE.

Acked-by: Peter Ujfalusi <peter.ujfalusi@gmail.com>

> 
> Fixes: dc9b60552f6a ("ARM/dmaengine: edma: Move of_dma_controller_register to the dmaengine driver")
> Signed-off-by: Mikhail Arkhipov <m.arhipov@rosa.ru>
> ---
>  drivers/dma/ti/edma.c | 17 +++++++++++++----
>  1 file changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/dma/ti/edma.c b/drivers/dma/ti/edma.c
> index 5f8d2e93ff3f..9fcbd97d346f 100644
> --- a/drivers/dma/ti/edma.c
> +++ b/drivers/dma/ti/edma.c
> @@ -2529,18 +2529,27 @@ static int edma_probe(struct platform_device *pdev)
>  		if (ret) {
>  			dev_err(dev, "memcpy ddev registration failed (%d)\n",
>  				ret);
> -			dma_async_device_unregister(&ecc->dma_slave);
> -			goto err_reg1;
> +			goto err_unregister_dma_slave;
>  		}
>  	}
>  
> -	if (node)
> -		of_dma_controller_register(node, of_edma_xlate, ecc);
> +	if (node) {
> +		ret = of_dma_controller_register(node, of_edma_xlate, ecc);
> +		if (ret) {
> +			dev_err(dev, "Failed to register DMA controller (%d)\n", ret);
> +			goto err_unregister_dma_memcpy;
> +		}
> +	}
> 
>  	dev_info(dev, "TI EDMA DMA engine driver\n");
>  
>  	return 0;
>  
> +err_unregister_dma_memcpy:
> +	if (ecc->dma_memcpy)
> +		dma_async_device_unregister(ecc->dma_memcpy);
> +err_unregister_dma_slave:
> +	dma_async_device_unregister(&ecc->dma_slave);
>  err_reg1:
>  	edma_free_slot(ecc, ecc->dummy_slot);
>  err_disable_pm:

-- 
Péter