dmaengine.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] dmaengine: idxd: Fix dereference on uninitialized pointer conf_dev
@ 2025-08-11  9:58 Colin Ian King
  2025-08-11 10:16 ` Dan Carpenter
  0 siblings, 1 reply; 4+ messages in thread
From: Colin Ian King @ 2025-08-11  9:58 UTC (permalink / raw)
  To: Vinicius Costa Gomes, Dave Jiang, Vinod Koul, Fenghua Yu,
	Shuai Xue, dmaengine
  Cc: kernel-janitors, linux-kernel

Currently if the allocation for wq fails on the initial iteration in
the setup loop the error exit path to err will call put_device on
an uninitialized pointer conf_dev. Fix this by initializing conf_dev
to NULL, note that put_device will ignore a NULL device pointer so no
null pointer dereference issues occur on this call.

Fixes: 3fd2f4bc010c ("dmaengine: idxd: fix memory leak in error handling path of idxd_setup_wqs")

Signed-off-by: Colin Ian King <colin.i.king@gmail.com>
---
 drivers/dma/idxd/init.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c
index 35bdefd3728b..2b61f26af1f6 100644
--- a/drivers/dma/idxd/init.c
+++ b/drivers/dma/idxd/init.c
@@ -178,7 +178,7 @@ static int idxd_setup_wqs(struct idxd_device *idxd)
 {
 	struct device *dev = &idxd->pdev->dev;
 	struct idxd_wq *wq;
-	struct device *conf_dev;
+	struct device *conf_dev = NULL;
 	int i, rc;
 
 	idxd->wqs = kcalloc_node(idxd->max_wqs, sizeof(struct idxd_wq *),
-- 
2.50.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] dmaengine: idxd: Fix dereference on uninitialized pointer conf_dev
  2025-08-11  9:58 [PATCH] dmaengine: idxd: Fix dereference on uninitialized pointer conf_dev Colin Ian King
@ 2025-08-11 10:16 ` Dan Carpenter
  2025-08-11 10:46   ` Dan Carpenter
  2025-08-11 11:25   ` Colin King (gmail)
  0 siblings, 2 replies; 4+ messages in thread
From: Dan Carpenter @ 2025-08-11 10:16 UTC (permalink / raw)
  To: Colin Ian King, Shuai Xue
  Cc: Vinicius Costa Gomes, Dave Jiang, Vinod Koul, Fenghua Yu,
	Shuai Xue, dmaengine, kernel-janitors, linux-kernel

On Mon, Aug 11, 2025 at 10:58:36AM +0100, Colin Ian King wrote:
> Currently if the allocation for wq fails on the initial iteration in
> the setup loop the error exit path to err will call put_device on
> an uninitialized pointer conf_dev. Fix this by initializing conf_dev
> to NULL, note that put_device will ignore a NULL device pointer so no
> null pointer dereference issues occur on this call.
> 
> Fixes: 3fd2f4bc010c ("dmaengine: idxd: fix memory leak in error handling path of idxd_setup_wqs")
> 
> Signed-off-by: Colin Ian King <colin.i.king@gmail.com>
> ---

No.  This isn't the right fix.  I basically wrote out the correct fix
in my bug report:
https://lore.kernel.org/all/aDQt3_rZjX-VuHJW@stanley.mountain/
Shuai Xue sent a fix as well but that patch wasn't right either but I
didn't review it until now.

It's easiest if I send the fix and give you Reported-by credit.

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] dmaengine: idxd: Fix dereference on uninitialized pointer conf_dev
  2025-08-11 10:16 ` Dan Carpenter
@ 2025-08-11 10:46   ` Dan Carpenter
  2025-08-11 11:25   ` Colin King (gmail)
  1 sibling, 0 replies; 4+ messages in thread
From: Dan Carpenter @ 2025-08-11 10:46 UTC (permalink / raw)
  To: Colin Ian King, Shuai Xue
  Cc: Vinicius Costa Gomes, Dave Jiang, Vinod Koul, Fenghua Yu,
	dmaengine, kernel-janitors, linux-kernel

Actually the error handling wasn't so bad.  It's just that one error path
which is buggy.  The idxd->max_wqs variable probably can't be <= 0 (I
haven't checked, but I assume it can't).  Anyway, I've sent my prefered
fix but an alternative would be to do the below.

regards,
dan carpenter

diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c
index 35bdefd3728b..b603d7dacf3a 100644
--- a/drivers/dma/idxd/init.c
+++ b/drivers/dma/idxd/init.c
@@ -195,6 +195,7 @@ static int idxd_setup_wqs(struct idxd_device *idxd)
 	for (i = 0; i < idxd->max_wqs; i++) {
 		wq = kzalloc_node(sizeof(*wq), GFP_KERNEL, dev_to_node(dev));
 		if (!wq) {
+			conf_dev = NULL;
 			rc = -ENOMEM;
 			goto err;
 		}

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] dmaengine: idxd: Fix dereference on uninitialized pointer conf_dev
  2025-08-11 10:16 ` Dan Carpenter
  2025-08-11 10:46   ` Dan Carpenter
@ 2025-08-11 11:25   ` Colin King (gmail)
  1 sibling, 0 replies; 4+ messages in thread
From: Colin King (gmail) @ 2025-08-11 11:25 UTC (permalink / raw)
  To: Dan Carpenter, Shuai Xue
  Cc: Vinicius Costa Gomes, Dave Jiang, Vinod Koul, Fenghua Yu,
	dmaengine, kernel-janitors, linux-kernel


[-- Attachment #1.1.1: Type: text/plain, Size: 1079 bytes --]

On 11/08/2025 11:16, Dan Carpenter wrote:
> On Mon, Aug 11, 2025 at 10:58:36AM +0100, Colin Ian King wrote:
>> Currently if the allocation for wq fails on the initial iteration in
>> the setup loop the error exit path to err will call put_device on
>> an uninitialized pointer conf_dev. Fix this by initializing conf_dev
>> to NULL, note that put_device will ignore a NULL device pointer so no
>> null pointer dereference issues occur on this call.
>>
>> Fixes: 3fd2f4bc010c ("dmaengine: idxd: fix memory leak in error handling path of idxd_setup_wqs")
>>
>> Signed-off-by: Colin Ian King <colin.i.king@gmail.com>
>> ---
> 
> No.  This isn't the right fix.  I basically wrote out the correct fix
> in my bug report:
> https://lore.kernel.org/all/aDQt3_rZjX-VuHJW@stanley.mountain/
> Shuai Xue sent a fix as well but that patch wasn't right either but I
> didn't review it until now.
> 
> It's easiest if I send the fix and give you Reported-by credit.
> 
> regards,
> dan carpenter
> 

Thanks Dan, always appreciate your input to these issues.

Colin

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 4901 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2025-08-11 11:25 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-11  9:58 [PATCH] dmaengine: idxd: Fix dereference on uninitialized pointer conf_dev Colin Ian King
2025-08-11 10:16 ` Dan Carpenter
2025-08-11 10:46   ` Dan Carpenter
2025-08-11 11:25   ` Colin King (gmail)

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).