linux-kernel - Re: [PATCH] mmc: pxamci: Fix passing NULL to PTR_ERR() in pxamci

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ed5bcdca-9a6d-4144-acd7-1c1feeaadb0f@kernel.org>
Date: Tue, 14 Oct 2025 07:31:13 -0600
From: Khalid Aziz <khalid@...nel.org>
To: Dan Carpenter <dan.carpenter@...aro.org>,
 Uwe Kleine-König <u.kleine-koenig@...libre.com>
Cc: Rakuram Eswaran <rakuram.e96@...il.com>, chenhuacai@...nel.org,
 david.hunter.linux@...il.com, linux-kernel-mentees@...ts.linux.dev,
 linux-kernel@...r.kernel.org, linux-mmc@...r.kernel.org, lkp@...el.com,
 skhan@...uxfoundation.org, ulf.hansson@...aro.org, zhoubinbin@...ngson.cn
Subject: Re: [PATCH] mmc: pxamci: Fix passing NULL to PTR_ERR() in
 pxamci_probe()

On 10/14/25 6:27 AM, Dan Carpenter wrote:
> On Mon, Oct 13, 2025 at 04:54:13PM -0600, Khalid Aziz wrote:
>>> @@ -703,20 +705,15 @@ 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;
>>> +	host->dma_chan_rx = devm_dma_request_chan(dev, "rx");
>>> +	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)) {
>>> -		dev_err(dev, "unable to request tx dma channel\n");
>>> -		ret = PTR_ERR(host->dma_chan_tx);
>>> -		host->dma_chan_tx = NULL;
>>> -		goto out;
>>> -	}
>>> +	host->dma_chan_tx = devm_dma_request_chan(dev, "tx");
>>> +	if (IS_ERR(host->dma_chan_tx))
>>> +		return dev_err_probe(dev, PTR_ERR(host->dma_chan_tx),
>>> +				     "unable to request tx dma channel\n");
>>
>> We should still release DMA rx channel before returning here.
>>
>>>    	if (host->pdata) {
>>>    		host->detect_delay_ms = host->pdata->detect_delay_ms;
>>> @@ -724,25 +721,21 @@ static int pxamci_probe(struct platform_device *pdev)
>>>    		host->power = devm_gpiod_get_optional(dev, "power", GPIOD_OUT_LOW);
>>>    		if (IS_ERR(host->power)) {
>>>    			ret = PTR_ERR(host->power);
>>> -			dev_err(dev, "Failed requesting gpio_power\n");
>>> -			goto out;
>>> +			return dev_err_probe(dev, ret, "Failed requesting gpio_power\n");
>>
>> Don't we need to release DMA Rx and Tx channels before we return from here?
>>
>>>    		}
>>>    		/* FIXME: should we pass detection delay to debounce? */
>>>    		ret = mmc_gpiod_request_cd(mmc, "cd", 0, false, 0);
>>> -		if (ret && ret != -ENOENT) {
>>> -			dev_err(dev, "Failed requesting gpio_cd\n");
>>> -			goto out;
>>> -		}
>>> +		if (ret && ret != -ENOENT)
>>> +			return dev_err_probe(dev, ret, "Failed requesting gpio_cd\n");
>>
>> Same here
>>
>>>    		if (!host->pdata->gpio_card_ro_invert)
>>>    			mmc->caps2 |= MMC_CAP2_RO_ACTIVE_HIGH;
>>>    		ret = mmc_gpiod_request_ro(mmc, "wp", 0, 0);
>>> -		if (ret && ret != -ENOENT) {
>>> -			dev_err(dev, "Failed requesting gpio_ro\n");
>>> -			goto out;
>>> -		}
>>> +		if (ret && ret != -ENOENT)
>>> +			return dev_err_probe(dev, ret, "Failed requesting gpio_ro\n");
>>
>> and here.
>>
>> Looking at Documentation/driver-api/driver-model/devres.rst,
>> dma_request_chan() is not devres managed interface and thus will not be
>> released automatically. Do you agree?
>>
> 
> The patch changes dma_request_chan() to devm_dma_request_chan().
> 

Ah, yes. It does. That works then.

Thanks,
Khalid

> regards,
> dan carpenter
>