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: <aliep4j5jmbdixu5cmqztoxwp3jv4r4hi63qpvhughepsepzb3@qh3mwgryf5ny>
Date: Fri, 10 Oct 2025 11:59:23 +0200
From: Uwe Kleine-König <u.kleine-koenig@...libre.com>
To: Rakuram Eswaran <rakuram.e96@...il.com>
Cc: chenhuacai@...nel.org, dan.carpenter@...aro.org, 
	david.hunter.linux@...il.com, khalid@...nel.org, 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()

Hello Rakuram,

On Thu, Oct 09, 2025 at 08:57:38PM +0530, Rakuram Eswaran wrote:
> Your suggestion makes perfect sense — since host is devm-managed, 
> explicitly assigning its members to NULL has no effect. 
> I’ll remove those two redundant lines in v2 as you suggested.
> 
> I had one small clarification regarding the remaining host->dma_chan_tx = NULL;
> in the TX DMA error path. Since that branch uses goto out, 
> the cleanup section below may call dma_release_channel() on both RX 
> and TX pointers. Setting TX to NULL there seems like a defensive step 
> to avoid accidentally passing an ERR_PTR() to dma_release_channel() 
> — is that understanding correct?

Ah right, so either keep host->dma_chan_tx = NULL or improve the error
handling like:

diff --git a/drivers/mmc/host/pxamci.c b/drivers/mmc/host/pxamci.c
index 26d03352af63..e5068cc55fb2 100644
--- a/drivers/mmc/host/pxamci.c
+++ b/drivers/mmc/host/pxamci.c
@@ -715,7 +715,7 @@ static int pxamci_probe(struct platform_device *pdev)
 		dev_err(dev, "unable to request tx dma channel\n");
 		ret = PTR_ERR(host->dma_chan_tx);
 		host->dma_chan_tx = NULL;
-		goto out;
+		goto out_dma_tx;
 	}
 
 	if (host->pdata) {
@@ -765,10 +765,11 @@ static int pxamci_probe(struct platform_device *pdev)
 	return 0;
 
 out:
-	if (host->dma_chan_rx)
-		dma_release_channel(host->dma_chan_rx);
 	if (host->dma_chan_tx)
 		dma_release_channel(host->dma_chan_tx);
+out_dma_tx:
+	if (host->dma_chan_rx)
+		dma_release_channel(host->dma_chan_rx);
 	return ret;
 }
 
> Also, I noticed that in the build configuration downloaded from the LKP report 
> link (CONFIG_DMA_ENGINE isn’t defined), the kernel uses the stub inline 
> version of dma_release_channel() from include/linux/dmaengine.h, 
> which becomes a no-op. 
> 
> From what I understand, when the DMA engine framework isn’t enabled, 
> these APIs compile as no-ops through their inline stubs. 
> Please correct me if I’m misunderstanding how this works.
> 
> Please let me know if this reasoning aligns with what you had in mind.

Sounds right.

Best regards
Uwe

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)