[PATCH] dmaengine: idxd: Fix dereference on uninitialized pointer conf_dev

Colin Ian King posted 1 patch 1 month, 3 weeks ago
drivers/dma/idxd/init.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] dmaengine: idxd: Fix dereference on uninitialized pointer conf_dev
Posted by Colin Ian King 1 month, 3 weeks ago
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
Re: [PATCH] dmaengine: idxd: Fix dereference on uninitialized pointer conf_dev
Posted by Dan Carpenter 1 month, 3 weeks ago
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
Re: [PATCH] dmaengine: idxd: Fix dereference on uninitialized pointer conf_dev
Posted by Colin King (gmail) 1 month, 3 weeks ago
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
Re: [PATCH] dmaengine: idxd: Fix dereference on uninitialized pointer conf_dev
Posted by Dan Carpenter 1 month, 3 weeks ago
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;
 		}