[PATCH v2] dmaengine: idxd: fix double free in idxd_alloc() error path

Guangshuo Li posted 1 patch 2 months ago
drivers/dma/idxd/init.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH v2] dmaengine: idxd: fix double free in idxd_alloc() error path
Posted by Guangshuo Li 2 months ago
When dev_set_name() fails after device_initialize(), idxd_alloc()
calls put_device(conf_dev).

For these devices, conf_dev->type is set from idxd->data->dev_type,
which resolves to dsa_device_type or iax_device_type, and both use
idxd_conf_device_release() as their release callback. That release
callback frees idxd, idxd->opcap_bmap, and releases idxd->id, but
the current error path then frees those resources again directly,
causing a double free.

The issue was identified by a static analysis tool I developed and
confirmed by manual review.

Keep the cleanup in idxd_conf_device_release() after put_device() and
avoid freeing idxd-managed resources again in idxd_alloc().

Fixes: 46a5cca76c76 ("dmaengine: idxd: fix memory leak in error handling path of idxd_alloc")
Cc: stable@vger.kernel.org
Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
---
v2:
  - note that the issue was identified by my static analysis tool
  - and confirmed by manual review

 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 f1bd9812c90d..20505e14ef9f 100644
--- a/drivers/dma/idxd/init.c
+++ b/drivers/dma/idxd/init.c
@@ -631,7 +631,7 @@ static struct idxd_device *idxd_alloc(struct pci_dev *pdev, struct idxd_driver_d
 
 err_name:
 	put_device(conf_dev);
-	bitmap_free(idxd->opcap_bmap);
+	return NULL;
 err_opcap:
 	ida_free(&idxd_ida, idxd->id);
 err_ida:
-- 
2.43.0
Re: [PATCH v2] dmaengine: idxd: fix double free in idxd_alloc() error path
Posted by Vinicius Costa Gomes 1 month, 3 weeks ago
Guangshuo Li <lgs201920130244@gmail.com> writes:

> When dev_set_name() fails after device_initialize(), idxd_alloc()
> calls put_device(conf_dev).
>
> For these devices, conf_dev->type is set from idxd->data->dev_type,
> which resolves to dsa_device_type or iax_device_type, and both use
> idxd_conf_device_release() as their release callback. That release
> callback frees idxd, idxd->opcap_bmap, and releases idxd->id, but
> the current error path then frees those resources again directly,
> causing a double free.
>
> The issue was identified by a static analysis tool I developed and
> confirmed by manual review.
>
> Keep the cleanup in idxd_conf_device_release() after put_device() and
> avoid freeing idxd-managed resources again in idxd_alloc().
>
> Fixes: 46a5cca76c76 ("dmaengine: idxd: fix memory leak in error handling path of idxd_alloc")
> Cc: stable@vger.kernel.org
> Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
> ---

On the review of 'v1', you agreed to the comments I made, but they are
neither reflected in the code nor in the series organization.


-- 
Vinicius
Re: [PATCH v2] dmaengine: idxd: fix double free in idxd_alloc() error path
Posted by Guangshuo Li 1 month, 3 weeks ago
Hi Vinicius,

Thanks for reviewing.

On Thu, 23 Apr 2026 at 05:56, Vinicius Costa Gomes
<vinicius.gomes@intel.com> wrote:
>
> On the review of 'v1', you agreed to the comments I made, but they are
> neither reflected in the code nor in the series organization.
>

You're right — my v2 did not incorporate the broader issues you pointed out.

At the moment I don't have a good fix for the similar patterns in
idxd_clean_wqs(), idxd_clean_engines(), idxd_clean_groups(), and
idxd_free(). Do you have any suggestion on the preferred way to
restructure those cleanup paths?

Thanks,
Guangshuo
Re: [PATCH v2] dmaengine: idxd: fix double free in idxd_alloc() error path
Posted by Vinicius Costa Gomes 1 month, 2 weeks ago
Guangshuo Li <lgs201920130244@gmail.com> writes:

> Hi Vinicius,
>
> Thanks for reviewing.
>
> On Thu, 23 Apr 2026 at 05:56, Vinicius Costa Gomes
> <vinicius.gomes@intel.com> wrote:
>>
>> On the review of 'v1', you agreed to the comments I made, but they are
>> neither reflected in the code nor in the series organization.
>>
>
> You're right — my v2 did not incorporate the broader issues you pointed out.
>
> At the moment I don't have a good fix for the similar patterns in
> idxd_clean_wqs(), idxd_clean_engines(), idxd_clean_groups(), and
> idxd_free(). Do you have any suggestion on the preferred way to
> restructure those cleanup paths?
>

The idea is that the explicit free's
(kfree()/bitmap_free()/ida_free()/etc) should be removed and instead
rely on device_put() doing the right thing on the _release() path.

Just not sure if we need to check that the workqueue was already created
before calling destroy_workqueue().


Cheers,
-- 
Vinicius