mmc: pxamci: Fix passing NULL to PTR_ERR() in pxamci_probe()

[PATCH] mmc: pxamci: Fix passing NULL to PTR_ERR() in pxamci_probe()

Posted by Rakuram Eswaran 4 months ago

Smatch reported:
drivers/mmc/host/pxamci.c:709 pxamci_probe() warn: passing zero to 'PTR_ERR'

Case 1:
When dma_request_chan() fails, host->dma_chan_rx is an ERR_PTR(),
but it is reset to NULL before using PTR_ERR(), resulting in PTR_ERR(0).
This mistakenly returns 0 instead of the real error code.

Case 2:
When devm_clk_get() fails, host->clk is an ERR_PTR() resulting in the similar
issue like case 1. 

Store the error code before nullifying the pointers in both the cases.

Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Closes: https://lore.kernel.org/r/202510041841.pRlunIfl-lkp@intel.com/
Fixes: 58c40f3faf742c ("mmc: pxamci: Use devm_mmc_alloc_host() helper")
Signed-off-by: Rakuram Eswaran <rakuram.e96@gmail.com>
---

Build and Analysis:
This patch was compiled against the configuration file reported by
0day CI in the above link (config: s390-randconfig-r071-20251004) using
`s390x-linux-gnu-gcc (Ubuntu 14.2.0-19ubuntu2) 14.2.0`. 

Static analysis was performed with Smatch to ensure the reported warning 
no longer reproduces after applying this fix.

Command used for verification:
  ARCH=s390 CROSS_COMPILE=s390x-linux-gnu- \
  ~/project/smatch/smatch_scripts/kchecker ./drivers/mmc/host/pxamci.c

 drivers/mmc/host/pxamci.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/pxamci.c b/drivers/mmc/host/pxamci.c
index 26d03352af63..4fab693d3b32 100644
--- a/drivers/mmc/host/pxamci.c
+++ b/drivers/mmc/host/pxamci.c
@@ -653,8 +653,9 @@ static int pxamci_probe(struct platform_device *pdev)
 
 	host->clk = devm_clk_get(dev, NULL);
 	if (IS_ERR(host->clk)) {
+		ret = PTR_ERR(host->clk);
 		host->clk = NULL;
-		return PTR_ERR(host->clk);
+		return ret;
 	}
 
 	host->clkrate = clk_get_rate(host->clk);
@@ -705,8 +706,9 @@ static int pxamci_probe(struct platform_device *pdev)
 
 	host->dma_chan_rx = dma_request_chan(dev, "rx");
 	if (IS_ERR(host->dma_chan_rx)) {
+		ret = PTR_ERR(host->dma_chan_rx);
 		host->dma_chan_rx = NULL;
-		return dev_err_probe(dev, PTR_ERR(host->dma_chan_rx),
+		return dev_err_probe(dev, ret,
 				     "unable to request rx dma channel\n");
 	}
 
-- 
2.48.1

Re: [PATCH] mmc: pxamci: Fix passing NULL to PTR_ERR() in pxamci_probe()

Posted by Uwe Kleine-König 4 months ago

On Tue, Oct 07, 2025 at 09:47:44PM +0530, Rakuram Eswaran wrote:
> Smatch reported:
> drivers/mmc/host/pxamci.c:709 pxamci_probe() warn: passing zero to 'PTR_ERR'
> 
> Case 1:
> When dma_request_chan() fails, host->dma_chan_rx is an ERR_PTR(),
> but it is reset to NULL before using PTR_ERR(), resulting in PTR_ERR(0).
> This mistakenly returns 0 instead of the real error code.
> 
> Case 2:
> When devm_clk_get() fails, host->clk is an ERR_PTR() resulting in the similar
> issue like case 1. 
> 
> Store the error code before nullifying the pointers in both the cases.

Why is the pointer set to NULL at all? This is in both cases memory that
is freed directly afterwards (as `host` is devm managed). So I'd claim

diff --git a/drivers/mmc/host/pxamci.c b/drivers/mmc/host/pxamci.c
index 26d03352af63..404f78198252 100644
--- a/drivers/mmc/host/pxamci.c
+++ b/drivers/mmc/host/pxamci.c
@@ -652,10 +652,8 @@ static int pxamci_probe(struct platform_device *pdev)
 	host->clkrt = CLKRT_OFF;
 
 	host->clk = devm_clk_get(dev, NULL);
-	if (IS_ERR(host->clk)) {
-		host->clk = NULL;
+	if (IS_ERR(host->clk))
 		return PTR_ERR(host->clk);
-	}
 
 	host->clkrate = clk_get_rate(host->clk);
 
@@ -704,11 +702,9 @@ static int pxamci_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, mmc);
 
 	host->dma_chan_rx = dma_request_chan(dev, "rx");
-	if (IS_ERR(host->dma_chan_rx)) {
-		host->dma_chan_rx = NULL;
+	if (IS_ERR(host->dma_chan_rx))
 		return dev_err_probe(dev, PTR_ERR(host->dma_chan_rx),
 				     "unable to request rx dma channel\n");
-	}
 
 	host->dma_chan_tx = dma_request_chan(dev, "tx");
 	if (IS_ERR(host->dma_chan_tx)) {

is a superior patch.

Best regards
Uwe

Re: [PATCH] mmc: pxamci: Fix passing NULL to PTR_ERR() in pxamci_probe()

Posted by Khalid Aziz 4 months ago

On 10/9/25 2:57 AM, Uwe Kleine-König wrote:
> On Tue, Oct 07, 2025 at 09:47:44PM +0530, Rakuram Eswaran wrote:
>> Smatch reported:
>> drivers/mmc/host/pxamci.c:709 pxamci_probe() warn: passing zero to 'PTR_ERR'
>>
>> Case 1:
>> When dma_request_chan() fails, host->dma_chan_rx is an ERR_PTR(),
>> but it is reset to NULL before using PTR_ERR(), resulting in PTR_ERR(0).
>> This mistakenly returns 0 instead of the real error code.
>>
>> Case 2:
>> When devm_clk_get() fails, host->clk is an ERR_PTR() resulting in the similar
>> issue like case 1.
>>
>> Store the error code before nullifying the pointers in both the cases.
> 
> Why is the pointer set to NULL at all? This is in both cases memory that
> is freed directly afterwards (as `host` is devm managed). So I'd claim

I am not sure that sounds right. Looking at the code for 
__devm_clk_get(), if devres_alloc() fails, it returns -ENOMEM. If any of 
the other steps after a successful devres_alloc() fail, code goes 
through possibly clk_put() if needed and then devres_free(). So the 
resources are already freed at this point before the return to 
pxamci_probe(). The only thing left to do is to set host->clk to NULL 
since it would be set to an error pointer at this point.

Am I missing something?

Thanks,
Khalid


> 
> diff --git a/drivers/mmc/host/pxamci.c b/drivers/mmc/host/pxamci.c
> index 26d03352af63..404f78198252 100644
> --- a/drivers/mmc/host/pxamci.c
> +++ b/drivers/mmc/host/pxamci.c
> @@ -652,10 +652,8 @@ static int pxamci_probe(struct platform_device *pdev)
>   	host->clkrt = CLKRT_OFF;
>   
>   	host->clk = devm_clk_get(dev, NULL);
> -	if (IS_ERR(host->clk)) {
> -		host->clk = NULL;
> +	if (IS_ERR(host->clk))
>   		return PTR_ERR(host->clk);
> -	}
>   
>   	host->clkrate = clk_get_rate(host->clk);
>   
> @@ -704,11 +702,9 @@ static int pxamci_probe(struct platform_device *pdev)
>   	platform_set_drvdata(pdev, mmc);
>   
>   	host->dma_chan_rx = dma_request_chan(dev, "rx");
> -	if (IS_ERR(host->dma_chan_rx)) {
> -		host->dma_chan_rx = NULL;
> +	if (IS_ERR(host->dma_chan_rx))
>   		return dev_err_probe(dev, PTR_ERR(host->dma_chan_rx),
>   				     "unable to request rx dma channel\n");
> -	}
>   
>   	host->dma_chan_tx = dma_request_chan(dev, "tx");
>   	if (IS_ERR(host->dma_chan_tx)) {
> 
> is a superior patch.
> 
> Best regards
> Uwe

Re: [PATCH] mmc: pxamci: Fix passing NULL to PTR_ERR() in pxamci_probe()

Posted by Binbin Zhou 4 months ago

On 2025/10/8 00:17, Rakuram Eswaran wrote:
> Smatch reported:
> drivers/mmc/host/pxamci.c:709 pxamci_probe() warn: passing zero to 'PTR_ERR'
>
> Case 1:
> When dma_request_chan() fails, host->dma_chan_rx is an ERR_PTR(),
> but it is reset to NULL before using PTR_ERR(), resulting in PTR_ERR(0).
> This mistakenly returns 0 instead of the real error code.
>
> Case 2:
> When devm_clk_get() fails, host->clk is an ERR_PTR() resulting in the similar
> issue like case 1.
>
> Store the error code before nullifying the pointers in both the cases.
>
> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> Closes: https://lore.kernel.org/r/202510041841.pRlunIfl-lkp@intel.com/
> Fixes: 58c40f3faf742c ("mmc: pxamci: Use devm_mmc_alloc_host() helper")
> Signed-off-by: Rakuram Eswaran <rakuram.e96@gmail.com>
LGTM.

Reviewed-by: Binbin Zhou <zhoubinbin@loongson.cn>

> ---
>
> Build and Analysis:
> This patch was compiled against the configuration file reported by
> 0day CI in the above link (config: s390-randconfig-r071-20251004) using
> `s390x-linux-gnu-gcc (Ubuntu 14.2.0-19ubuntu2) 14.2.0`.
>
> Static analysis was performed with Smatch to ensure the reported warning
> no longer reproduces after applying this fix.
>
> Command used for verification:
>    ARCH=s390 CROSS_COMPILE=s390x-linux-gnu- \
>    ~/project/smatch/smatch_scripts/kchecker ./drivers/mmc/host/pxamci.c
>
>   drivers/mmc/host/pxamci.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/host/pxamci.c b/drivers/mmc/host/pxamci.c
> index 26d03352af63..4fab693d3b32 100644
> --- a/drivers/mmc/host/pxamci.c
> +++ b/drivers/mmc/host/pxamci.c
> @@ -653,8 +653,9 @@ static int pxamci_probe(struct platform_device *pdev)
>   
>   	host->clk = devm_clk_get(dev, NULL);
>   	if (IS_ERR(host->clk)) {
> +		ret = PTR_ERR(host->clk);
>   		host->clk = NULL;
> -		return PTR_ERR(host->clk);
> +		return ret;
>   	}
>   
>   	host->clkrate = clk_get_rate(host->clk);
> @@ -705,8 +706,9 @@ static int pxamci_probe(struct platform_device *pdev)
>   
>   	host->dma_chan_rx = dma_request_chan(dev, "rx");
>   	if (IS_ERR(host->dma_chan_rx)) {
> +		ret = PTR_ERR(host->dma_chan_rx);
>   		host->dma_chan_rx = NULL;
> -		return dev_err_probe(dev, PTR_ERR(host->dma_chan_rx),
> +		return dev_err_probe(dev, ret,
>   				     "unable to request rx dma channel\n");
>   	}
>   
Thanks.
Binbin

Re: [PATCH] mmc: pxamci: Fix passing NULL to PTR_ERR() in pxamci_probe()

Posted by Khalid Aziz 4 months ago

On 10/7/25 10:17 AM, Rakuram Eswaran wrote:
> Smatch reported:
> drivers/mmc/host/pxamci.c:709 pxamci_probe() warn: passing zero to 'PTR_ERR'
> 
> Case 1:
> When dma_request_chan() fails, host->dma_chan_rx is an ERR_PTR(),
> but it is reset to NULL before using PTR_ERR(), resulting in PTR_ERR(0).
> This mistakenly returns 0 instead of the real error code.
> 
> Case 2:
> When devm_clk_get() fails, host->clk is an ERR_PTR() resulting in the similar
> issue like case 1.
> 
> Store the error code before nullifying the pointers in both the cases.
> 
> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> Closes: https://lore.kernel.org/r/202510041841.pRlunIfl-lkp@intel.com/
> Fixes: 58c40f3faf742c ("mmc: pxamci: Use devm_mmc_alloc_host() helper")
> Signed-off-by: Rakuram Eswaran <rakuram.e96@gmail.com>
> ---
> 
> Build and Analysis:
> This patch was compiled against the configuration file reported by
> 0day CI in the above link (config: s390-randconfig-r071-20251004) using
> `s390x-linux-gnu-gcc (Ubuntu 14.2.0-19ubuntu2) 14.2.0`.
> 
> Static analysis was performed with Smatch to ensure the reported warning
> no longer reproduces after applying this fix.
> 
> Command used for verification:
>    ARCH=s390 CROSS_COMPILE=s390x-linux-gnu- \
>    ~/project/smatch/smatch_scripts/kchecker ./drivers/mmc/host/pxamci.c
> 
>   drivers/mmc/host/pxamci.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mmc/host/pxamci.c b/drivers/mmc/host/pxamci.c
> index 26d03352af63..4fab693d3b32 100644
> --- a/drivers/mmc/host/pxamci.c
> +++ b/drivers/mmc/host/pxamci.c
> @@ -653,8 +653,9 @@ static int pxamci_probe(struct platform_device *pdev)
>   
>   	host->clk = devm_clk_get(dev, NULL);
>   	if (IS_ERR(host->clk)) {
> +		ret = PTR_ERR(host->clk);
>   		host->clk = NULL;
> -		return PTR_ERR(host->clk);
> +		return ret;
>   	}
>   
>   	host->clkrate = clk_get_rate(host->clk);
> @@ -705,8 +706,9 @@ static int pxamci_probe(struct platform_device *pdev)
>   
>   	host->dma_chan_rx = dma_request_chan(dev, "rx");
>   	if (IS_ERR(host->dma_chan_rx)) {
> +		ret = PTR_ERR(host->dma_chan_rx);
>   		host->dma_chan_rx = NULL;
> -		return dev_err_probe(dev, PTR_ERR(host->dma_chan_rx),
> +		return dev_err_probe(dev, ret,
>   				     "unable to request rx dma channel\n");
>   	}
>   

This looks good to me.

Reviewed-by: Khalid Aziz <khalid@kernel.org>

--
Khalid

Re: [PATCH] mmc: pxamci: Fix passing NULL to PTR_ERR() in pxamci_probe()

Posted by Dan Carpenter 4 months ago

On Tue, Oct 07, 2025 at 09:47:44PM +0530, Rakuram Eswaran wrote:
> Smatch reported:
> drivers/mmc/host/pxamci.c:709 pxamci_probe() warn: passing zero to 'PTR_ERR'
> 
> Case 1:
> When dma_request_chan() fails, host->dma_chan_rx is an ERR_PTR(),
> but it is reset to NULL before using PTR_ERR(), resulting in PTR_ERR(0).
> This mistakenly returns 0 instead of the real error code.
> 
> Case 2:
> When devm_clk_get() fails, host->clk is an ERR_PTR() resulting in the similar
> issue like case 1. 
> 
> Store the error code before nullifying the pointers in both the cases.
> 
> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> Closes: https://lore.kernel.org/r/202510041841.pRlunIfl-lkp@intel.com/
> Fixes: 58c40f3faf742c ("mmc: pxamci: Use devm_mmc_alloc_host() helper")
> Signed-off-by: Rakuram Eswaran <rakuram.e96@gmail.com>
> ---

Thanks!

Reviewed-by: Dan Carpenter <dan.carpenter@linaro.org>

regards,
dan carpenter