[PATCH v4 0/9] dmaengine: idxd: fix memory leak in error handling path

[PATCH v4 0/9] dmaengine: idxd: fix memory leak in error handling path

[Shuai Xue]

changes since v3:
- remove a blank line to fix checkpatch warning per Fenghua
- collect Reviewed-by tags from Fenghua


changes since v2:
- add to cc stable per Markus
- add patch 4 to fix memory leak in idxd_setup_internals per Fenghua
- collect Reviewed-by tag for patch 2 from Fenghua
- fix reference cnt in remove() per Fenghua

changes since v1:
- add Reviewed-by tag for patch 1-5 from Dave Jiang
- add fixes tag
- add patch 6 and 7 to fix memory leak in remove call per Vinicius

Shuai Xue (9):
  dmaengine: idxd: fix memory leak in error handling path of
    idxd_setup_wqs
  dmaengine: idxd: fix memory leak in error handling path of
    idxd_setup_engines
  dmaengine: idxd: fix memory leak in error handling path of
    idxd_setup_groups
  dmaengine: idxd: Add missing cleanup for early error out in
    idxd_setup_internals
  dmaengine: idxd: Add missing cleanups in cleanup internals
  dmaengine: idxd: fix memory leak in error handling path of idxd_alloc
  dmaengine: idxd: fix memory leak in error handling path of
    idxd_pci_probe
  dmaengine: idxd: Add missing idxd cleanup to fix memory leak in remove
    call
  dmaengine: idxd: Refactor remove call with idxd_cleanup() helper

 drivers/dma/idxd/init.c | 159 ++++++++++++++++++++++++++++------------
 1 file changed, 113 insertions(+), 46 deletions(-)

-- 
2.43.5

[PATCH v4 1/9] dmaengine: idxd: fix memory leak in error handling path of idxd_setup_wqs

[Shuai Xue]

Memory allocated for wqs is not freed if an error occurs during
idxd_setup_wqs(). To fix it, free the allocated memory in the reverse
order of allocation before exiting the function in case of an error.

Fixes: 7c5dd23e57c1 ("dmaengine: idxd: fix wq conf_dev 'struct device' lifetime")
Fixes: 700af3a0a26c ("dmaengine: idxd: add 'struct idxd_dev' as wrapper for conf_dev")
Fixes: de5819b99489 ("dmaengine: idxd: track enabled workqueues in bitmap")
Fixes: b0325aefd398 ("dmaengine: idxd: add WQ operation cap restriction support")
Cc: stable@vger.kernel.org
Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
Reviewed-by: Fenghua Yu <fenghuay@nvidia.com>
---
 drivers/dma/idxd/init.c | 30 +++++++++++++++++++++---------
 1 file changed, 21 insertions(+), 9 deletions(-)

diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c
index fca1d2924999..80fb189c9624 100644
--- a/drivers/dma/idxd/init.c
+++ b/drivers/dma/idxd/init.c
@@ -169,8 +169,8 @@ static int idxd_setup_wqs(struct idxd_device *idxd)
 
 	idxd->wq_enable_map = bitmap_zalloc_node(idxd->max_wqs, GFP_KERNEL, dev_to_node(dev));
 	if (!idxd->wq_enable_map) {
-		kfree(idxd->wqs);
-		return -ENOMEM;
+		rc = -ENOMEM;
+		goto err_bitmap;
 	}
 
 	for (i = 0; i < idxd->max_wqs; i++) {
@@ -189,10 +189,8 @@ static int idxd_setup_wqs(struct idxd_device *idxd)
 		conf_dev->bus = &dsa_bus_type;
 		conf_dev->type = &idxd_wq_device_type;
 		rc = dev_set_name(conf_dev, "wq%d.%d", idxd->id, wq->id);
-		if (rc < 0) {
-			put_device(conf_dev);
+		if (rc < 0)
 			goto err;
-		}
 
 		mutex_init(&wq->wq_lock);
 		init_waitqueue_head(&wq->err_queue);
@@ -203,7 +201,6 @@ static int idxd_setup_wqs(struct idxd_device *idxd)
 		wq->enqcmds_retries = IDXD_ENQCMDS_RETRIES;
 		wq->wqcfg = kzalloc_node(idxd->wqcfg_size, GFP_KERNEL, dev_to_node(dev));
 		if (!wq->wqcfg) {
-			put_device(conf_dev);
 			rc = -ENOMEM;
 			goto err;
 		}
@@ -211,9 +208,8 @@ static int idxd_setup_wqs(struct idxd_device *idxd)
 		if (idxd->hw.wq_cap.op_config) {
 			wq->opcap_bmap = bitmap_zalloc(IDXD_MAX_OPCAP_BITS, GFP_KERNEL);
 			if (!wq->opcap_bmap) {
-				put_device(conf_dev);
 				rc = -ENOMEM;
-				goto err;
+				goto err_opcap_bmap;
 			}
 			bitmap_copy(wq->opcap_bmap, idxd->opcap_bmap, IDXD_MAX_OPCAP_BITS);
 		}
@@ -224,12 +220,28 @@ static int idxd_setup_wqs(struct idxd_device *idxd)
 
 	return 0;
 
- err:
+err_opcap_bmap:
+	kfree(wq->wqcfg);
+
+err:
+	put_device(conf_dev);
+	kfree(wq);
+
 	while (--i >= 0) {
 		wq = idxd->wqs[i];
+		if (idxd->hw.wq_cap.op_config)
+			bitmap_free(wq->opcap_bmap);
+		kfree(wq->wqcfg);
 		conf_dev = wq_confdev(wq);
 		put_device(conf_dev);
+		kfree(wq);
+
 	}
+	bitmap_free(idxd->wq_enable_map);
+
+err_bitmap:
+	kfree(idxd->wqs);
+
 	return rc;
 }
 
-- 
2.43.5

[PATCH v4 2/9] dmaengine: idxd: fix memory leak in error handling path of idxd_setup_engines

[Shuai Xue]

Memory allocated for engines is not freed if an error occurs during
idxd_setup_engines(). To fix it, free the allocated memory in the
reverse order of allocation before exiting the function in case of an
error.

Fixes: 75b911309060 ("dmaengine: idxd: fix engine conf_dev lifetime")
Cc: stable@vger.kernel.org
Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
Reviewed-by: Fenghua Yu <fenghuay@nvidia.com>
---
 drivers/dma/idxd/init.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c
index 80fb189c9624..ff6ec3c0f604 100644
--- a/drivers/dma/idxd/init.c
+++ b/drivers/dma/idxd/init.c
@@ -275,6 +275,7 @@ static int idxd_setup_engines(struct idxd_device *idxd)
 		rc = dev_set_name(conf_dev, "engine%d.%d", idxd->id, engine->id);
 		if (rc < 0) {
 			put_device(conf_dev);
+			kfree(engine);
 			goto err;
 		}
 
@@ -288,7 +289,10 @@ static int idxd_setup_engines(struct idxd_device *idxd)
 		engine = idxd->engines[i];
 		conf_dev = engine_confdev(engine);
 		put_device(conf_dev);
+		kfree(engine);
 	}
+	kfree(idxd->engines);
+
 	return rc;
 }
 
-- 
2.43.5

[PATCH v4 3/9] dmaengine: idxd: fix memory leak in error handling path of idxd_setup_groups

[Shuai Xue]

Memory allocated for groups is not freed if an error occurs during
idxd_setup_groups(). To fix it, free the allocated memory in the reverse
order of allocation before exiting the function in case of an error.

Fixes: defe49f96012 ("dmaengine: idxd: fix group conf_dev lifetime")
Cc: stable@vger.kernel.org
Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
Reviewed-by: Fenghua Yu <fenghuay@nvidia.com>
---
 drivers/dma/idxd/init.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c
index ff6ec3c0f604..7f0a26e2e0a5 100644
--- a/drivers/dma/idxd/init.c
+++ b/drivers/dma/idxd/init.c
@@ -326,6 +326,7 @@ static int idxd_setup_groups(struct idxd_device *idxd)
 		rc = dev_set_name(conf_dev, "group%d.%d", idxd->id, group->id);
 		if (rc < 0) {
 			put_device(conf_dev);
+			kfree(group);
 			goto err;
 		}
 
@@ -350,7 +351,10 @@ static int idxd_setup_groups(struct idxd_device *idxd)
 	while (--i >= 0) {
 		group = idxd->groups[i];
 		put_device(group_confdev(group));
+		kfree(group);
 	}
+	kfree(idxd->groups);
+
 	return rc;
 }
 
-- 
2.43.5

[PATCH v4 4/9] dmaengine: idxd: Add missing cleanup for early error out in idxd_setup_internals

[Shuai Xue]

The idxd_setup_internals() is missing some cleanup when things fail in
the middle.

Add the appropriate cleanup routines:

- cleanup groups
- cleanup enginces
- cleanup wqs

to make sure it exits gracefully.

Fixes: defe49f96012 ("dmaengine: idxd: fix group conf_dev lifetime")
Cc: stable@vger.kernel.org
Suggested-by: Fenghua Yu <fenghuay@nvidia.com>
Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com>
Reviewed-by: Fenghua Yu <fenghuay@nvidia.com>
---
 drivers/dma/idxd/init.c | 58 ++++++++++++++++++++++++++++++++++++-----
 1 file changed, 51 insertions(+), 7 deletions(-)

diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c
index 7f0a26e2e0a5..a40fb2fd5006 100644
--- a/drivers/dma/idxd/init.c
+++ b/drivers/dma/idxd/init.c
@@ -155,6 +155,25 @@ static void idxd_cleanup_interrupts(struct idxd_device *idxd)
 	pci_free_irq_vectors(pdev);
 }
 
+static void idxd_clean_wqs(struct idxd_device *idxd)
+{
+	struct idxd_wq *wq;
+	struct device *conf_dev;
+	int i;
+
+	for (i = 0; i < idxd->max_wqs; i++) {
+		wq = idxd->wqs[i];
+		if (idxd->hw.wq_cap.op_config)
+			bitmap_free(wq->opcap_bmap);
+		kfree(wq->wqcfg);
+		conf_dev = wq_confdev(wq);
+		put_device(conf_dev);
+		kfree(wq);
+	}
+	bitmap_free(idxd->wq_enable_map);
+	kfree(idxd->wqs);
+}
+
 static int idxd_setup_wqs(struct idxd_device *idxd)
 {
 	struct device *dev = &idxd->pdev->dev;
@@ -245,6 +264,21 @@ static int idxd_setup_wqs(struct idxd_device *idxd)
 	return rc;
 }
 
+static void idxd_clean_engines(struct idxd_device *idxd)
+{
+	struct idxd_engine *engine;
+	struct device *conf_dev;
+	int i;
+
+	for (i = 0; i < idxd->max_engines; i++) {
+		engine = idxd->engines[i];
+		conf_dev = engine_confdev(engine);
+		put_device(conf_dev);
+		kfree(engine);
+	}
+	kfree(idxd->engines);
+}
+
 static int idxd_setup_engines(struct idxd_device *idxd)
 {
 	struct idxd_engine *engine;
@@ -296,6 +330,19 @@ static int idxd_setup_engines(struct idxd_device *idxd)
 	return rc;
 }
 
+static void idxd_clean_groups(struct idxd_device *idxd)
+{
+	struct idxd_group *group;
+	int i;
+
+	for (i = 0; i < idxd->max_groups; i++) {
+		group = idxd->groups[i];
+		put_device(group_confdev(group));
+		kfree(group);
+	}
+	kfree(idxd->groups);
+}
+
 static int idxd_setup_groups(struct idxd_device *idxd)
 {
 	struct device *dev = &idxd->pdev->dev;
@@ -410,7 +457,7 @@ static int idxd_init_evl(struct idxd_device *idxd)
 static int idxd_setup_internals(struct idxd_device *idxd)
 {
 	struct device *dev = &idxd->pdev->dev;
-	int rc, i;
+	int rc;
 
 	init_waitqueue_head(&idxd->cmd_waitq);
 
@@ -441,14 +488,11 @@ static int idxd_setup_internals(struct idxd_device *idxd)
  err_evl:
 	destroy_workqueue(idxd->wq);
  err_wkq_create:
-	for (i = 0; i < idxd->max_groups; i++)
-		put_device(group_confdev(idxd->groups[i]));
+	idxd_clean_groups(idxd);
  err_group:
-	for (i = 0; i < idxd->max_engines; i++)
-		put_device(engine_confdev(idxd->engines[i]));
+	idxd_clean_engines(idxd);
  err_engine:
-	for (i = 0; i < idxd->max_wqs; i++)
-		put_device(wq_confdev(idxd->wqs[i]));
+	idxd_clean_wqs(idxd);
  err_wqs:
 	return rc;
 }
-- 
2.43.5

[PATCH v4 5/9] dmaengine: idxd: Add missing cleanups in cleanup internals

[Shuai Xue]

The idxd_cleanup_internals() function only decreases the reference count
of groups, engines, and wqs but is missing the step to release memory
resources.

To fix this, use the cleanup helper to properly release the memory
resources.

Fixes: ddf742d4f3f1 ("dmaengine: idxd: Add missing cleanup for early error out in probe call")
Cc: stable@vger.kernel.org
Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com>
Reviewed-by: Fenghua Yu <fenghuay@nvidia.com>
---
 drivers/dma/idxd/init.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c
index a40fb2fd5006..f8129d2d53f1 100644
--- a/drivers/dma/idxd/init.c
+++ b/drivers/dma/idxd/init.c
@@ -407,14 +407,9 @@ static int idxd_setup_groups(struct idxd_device *idxd)
 
 static void idxd_cleanup_internals(struct idxd_device *idxd)
 {
-	int i;
-
-	for (i = 0; i < idxd->max_groups; i++)
-		put_device(group_confdev(idxd->groups[i]));
-	for (i = 0; i < idxd->max_engines; i++)
-		put_device(engine_confdev(idxd->engines[i]));
-	for (i = 0; i < idxd->max_wqs; i++)
-		put_device(wq_confdev(idxd->wqs[i]));
+	idxd_clean_groups(idxd);
+	idxd_clean_engines(idxd);
+	idxd_clean_wqs(idxd);
 	destroy_workqueue(idxd->wq);
 }
 
-- 
2.43.5

[PATCH v4 6/9] dmaengine: idxd: fix memory leak in error handling path of idxd_alloc

[Shuai Xue]

Memory allocated for idxd is not freed if an error occurs during
idxd_alloc(). To fix it, free the allocated memory in the reverse order
of allocation before exiting the function in case of an error.

Fixes: a8563a33a5e2 ("dmanegine: idxd: reformat opcap output to match bitmap_parse() input")
Cc: stable@vger.kernel.org
Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
Reviewed-by: Fenghua Yu <fenghuay@nvidia.com>
---
 drivers/dma/idxd/init.c | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c
index f8129d2d53f1..302d8983ed8c 100644
--- a/drivers/dma/idxd/init.c
+++ b/drivers/dma/idxd/init.c
@@ -604,28 +604,34 @@ static struct idxd_device *idxd_alloc(struct pci_dev *pdev, struct idxd_driver_d
 	idxd_dev_set_type(&idxd->idxd_dev, idxd->data->type);
 	idxd->id = ida_alloc(&idxd_ida, GFP_KERNEL);
 	if (idxd->id < 0)
-		return NULL;
+		goto err_ida;
 
 	idxd->opcap_bmap = bitmap_zalloc_node(IDXD_MAX_OPCAP_BITS, GFP_KERNEL, dev_to_node(dev));
-	if (!idxd->opcap_bmap) {
-		ida_free(&idxd_ida, idxd->id);
-		return NULL;
-	}
+	if (!idxd->opcap_bmap)
+		goto err_opcap;
 
 	device_initialize(conf_dev);
 	conf_dev->parent = dev;
 	conf_dev->bus = &dsa_bus_type;
 	conf_dev->type = idxd->data->dev_type;
 	rc = dev_set_name(conf_dev, "%s%d", idxd->data->name_prefix, idxd->id);
-	if (rc < 0) {
-		put_device(conf_dev);
-		return NULL;
-	}
+	if (rc < 0)
+		goto err_name;
 
 	spin_lock_init(&idxd->dev_lock);
 	spin_lock_init(&idxd->cmd_lock);
 
 	return idxd;
+
+err_name:
+	put_device(conf_dev);
+	bitmap_free(idxd->opcap_bmap);
+err_opcap:
+	ida_free(&idxd_ida, idxd->id);
+err_ida:
+	kfree(idxd);
+
+	return NULL;
 }
 
 static int idxd_enable_system_pasid(struct idxd_device *idxd)
-- 
2.43.5

[PATCH v4 7/9] dmaengine: idxd: fix memory leak in error handling path of idxd_pci_probe

[Shuai Xue]

Memory allocated for idxd is not freed if an error occurs during
idxd_pci_probe(). To fix it, free the allocated memory in the reverse
order of allocation before exiting the function in case of an error.

Fixes: bfe1d56091c1 ("dmaengine: idxd: Init and probe for Intel data accelerators")
Cc: stable@vger.kernel.org
Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
Reviewed-by: Fenghua Yu <fenghuay@nvidia.com>
---
 drivers/dma/idxd/init.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c
index 302d8983ed8c..f2b5b17538c0 100644
--- a/drivers/dma/idxd/init.c
+++ b/drivers/dma/idxd/init.c
@@ -587,6 +587,17 @@ static void idxd_read_caps(struct idxd_device *idxd)
 		idxd->hw.iaa_cap.bits = ioread64(idxd->reg_base + IDXD_IAACAP_OFFSET);
 }
 
+static void idxd_free(struct idxd_device *idxd)
+{
+	if (!idxd)
+		return;
+
+	put_device(idxd_confdev(idxd));
+	bitmap_free(idxd->opcap_bmap);
+	ida_free(&idxd_ida, idxd->id);
+	kfree(idxd);
+}
+
 static struct idxd_device *idxd_alloc(struct pci_dev *pdev, struct idxd_driver_data *data)
 {
 	struct device *dev = &pdev->dev;
@@ -1255,7 +1266,7 @@ int idxd_pci_probe_alloc(struct idxd_device *idxd, struct pci_dev *pdev,
  err:
 	pci_iounmap(pdev, idxd->reg_base);
  err_iomap:
-	put_device(idxd_confdev(idxd));
+	idxd_free(idxd);
  err_idxd_alloc:
 	pci_disable_device(pdev);
 	return rc;
-- 
2.43.5

[PATCH v4 8/9] dmaengine: idxd: Add missing idxd cleanup to fix memory leak in remove call

[Shuai Xue]

The remove call stack is missing idxd cleanup to free bitmap, ida and
the idxd_device. Call idxd_free() helper routines to make sure we exit
gracefully.

Fixes: bfe1d56091c1 ("dmaengine: idxd: Init and probe for Intel data accelerators")
Cc: stable@vger.kernel.org
Suggested-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com>
Reviewed-by: Fenghua Yu <fenghuay@nvidia.com>
---
 drivers/dma/idxd/init.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c
index f2b5b17538c0..974b926bd930 100644
--- a/drivers/dma/idxd/init.c
+++ b/drivers/dma/idxd/init.c
@@ -1335,6 +1335,7 @@ static void idxd_remove(struct pci_dev *pdev)
 	destroy_workqueue(idxd->wq);
 	perfmon_pmu_remove(idxd);
 	put_device(idxd_confdev(idxd));
+	idxd_free(idxd);
 }
 
 static struct pci_driver idxd_pci_driver = {
-- 
2.43.5

[PATCH v4 9/9] dmaengine: idxd: Refactor remove call with idxd_cleanup() helper

[Shuai Xue]

The idxd_cleanup() helper cleans up perfmon, interrupts, internals and
so on. Refactor remove call with the idxd_cleanup() helper to avoid code
duplication. Note, this also fixes the missing put_device() for idxd
groups, enginces and wqs.

Fixes: bfe1d56091c1 ("dmaengine: idxd: Init and probe for Intel data accelerators")
Cc: stable@vger.kernel.org
Suggested-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com>
Reviewed-by: Fenghua Yu <fenghuay@nvidia.com>
---
 drivers/dma/idxd/init.c | 14 ++------------
 1 file changed, 2 insertions(+), 12 deletions(-)

diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c
index 974b926bd930..760b7d81fcd8 100644
--- a/drivers/dma/idxd/init.c
+++ b/drivers/dma/idxd/init.c
@@ -1308,7 +1308,6 @@ static void idxd_shutdown(struct pci_dev *pdev)
 static void idxd_remove(struct pci_dev *pdev)
 {
 	struct idxd_device *idxd = pci_get_drvdata(pdev);
-	struct idxd_irq_entry *irq_entry;
 
 	idxd_unregister_devices(idxd);
 	/*
@@ -1321,21 +1320,12 @@ static void idxd_remove(struct pci_dev *pdev)
 	get_device(idxd_confdev(idxd));
 	device_unregister(idxd_confdev(idxd));
 	idxd_shutdown(pdev);
-	if (device_pasid_enabled(idxd))
-		idxd_disable_system_pasid(idxd);
 	idxd_device_remove_debugfs(idxd);
-
-	irq_entry = idxd_get_ie(idxd, 0);
-	free_irq(irq_entry->vector, irq_entry);
-	pci_free_irq_vectors(pdev);
+	idxd_cleanup(idxd);
 	pci_iounmap(pdev, idxd->reg_base);
-	if (device_user_pasid_enabled(idxd))
-		idxd_disable_sva(pdev);
-	pci_disable_device(pdev);
-	destroy_workqueue(idxd->wq);
-	perfmon_pmu_remove(idxd);
 	put_device(idxd_confdev(idxd));
 	idxd_free(idxd);
+	pci_disable_device(pdev);
 }
 
 static struct pci_driver idxd_pci_driver = {
-- 
2.43.5

Re: [PATCH v4 0/9] dmaengine: idxd: fix memory leak in error handling path

[Shuai Xue]

在 2025/4/4 20:02, Shuai Xue 写道:
> changes since v3:
> - remove a blank line to fix checkpatch warning per Fenghua
> - collect Reviewed-by tags from Fenghua
> 

Hi, all,

Gentle ping.

Thanks.
Shuai

Re: [PATCH v4 4/9] dmaengine: idxd: Add missing cleanup for early error out in idxd_setup_internals

[Dave Jiang]

On 4/4/25 5:02 AM, Shuai Xue wrote:
> The idxd_setup_internals() is missing some cleanup when things fail in
> the middle.
> 
> Add the appropriate cleanup routines:
> 
> - cleanup groups
> - cleanup enginces
> - cleanup wqs
> 
> to make sure it exits gracefully.
> 
> Fixes: defe49f96012 ("dmaengine: idxd: fix group conf_dev lifetime")
> Cc: stable@vger.kernel.org
> Suggested-by: Fenghua Yu <fenghuay@nvidia.com>
> Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com>
> Reviewed-by: Fenghua Yu <fenghuay@nvidia.com>

Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> ---
>  drivers/dma/idxd/init.c | 58 ++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 51 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c
> index 7f0a26e2e0a5..a40fb2fd5006 100644
> --- a/drivers/dma/idxd/init.c
> +++ b/drivers/dma/idxd/init.c
> @@ -155,6 +155,25 @@ static void idxd_cleanup_interrupts(struct idxd_device *idxd)
>  	pci_free_irq_vectors(pdev);
>  }
>  
> +static void idxd_clean_wqs(struct idxd_device *idxd)
> +{
> +	struct idxd_wq *wq;
> +	struct device *conf_dev;
> +	int i;
> +
> +	for (i = 0; i < idxd->max_wqs; i++) {
> +		wq = idxd->wqs[i];
> +		if (idxd->hw.wq_cap.op_config)
> +			bitmap_free(wq->opcap_bmap);
> +		kfree(wq->wqcfg);
> +		conf_dev = wq_confdev(wq);
> +		put_device(conf_dev);
> +		kfree(wq);
> +	}
> +	bitmap_free(idxd->wq_enable_map);
> +	kfree(idxd->wqs);
> +}
> +
>  static int idxd_setup_wqs(struct idxd_device *idxd)
>  {
>  	struct device *dev = &idxd->pdev->dev;
> @@ -245,6 +264,21 @@ static int idxd_setup_wqs(struct idxd_device *idxd)
>  	return rc;
>  }
>  
> +static void idxd_clean_engines(struct idxd_device *idxd)
> +{
> +	struct idxd_engine *engine;
> +	struct device *conf_dev;
> +	int i;
> +
> +	for (i = 0; i < idxd->max_engines; i++) {
> +		engine = idxd->engines[i];
> +		conf_dev = engine_confdev(engine);
> +		put_device(conf_dev);
> +		kfree(engine);
> +	}
> +	kfree(idxd->engines);
> +}
> +
>  static int idxd_setup_engines(struct idxd_device *idxd)
>  {
>  	struct idxd_engine *engine;
> @@ -296,6 +330,19 @@ static int idxd_setup_engines(struct idxd_device *idxd)
>  	return rc;
>  }
>  
> +static void idxd_clean_groups(struct idxd_device *idxd)
> +{
> +	struct idxd_group *group;
> +	int i;
> +
> +	for (i = 0; i < idxd->max_groups; i++) {
> +		group = idxd->groups[i];
> +		put_device(group_confdev(group));
> +		kfree(group);
> +	}
> +	kfree(idxd->groups);
> +}
> +
>  static int idxd_setup_groups(struct idxd_device *idxd)
>  {
>  	struct device *dev = &idxd->pdev->dev;
> @@ -410,7 +457,7 @@ static int idxd_init_evl(struct idxd_device *idxd)
>  static int idxd_setup_internals(struct idxd_device *idxd)
>  {
>  	struct device *dev = &idxd->pdev->dev;
> -	int rc, i;
> +	int rc;
>  
>  	init_waitqueue_head(&idxd->cmd_waitq);
>  
> @@ -441,14 +488,11 @@ static int idxd_setup_internals(struct idxd_device *idxd)
>   err_evl:
>  	destroy_workqueue(idxd->wq);
>   err_wkq_create:
> -	for (i = 0; i < idxd->max_groups; i++)
> -		put_device(group_confdev(idxd->groups[i]));
> +	idxd_clean_groups(idxd);
>   err_group:
> -	for (i = 0; i < idxd->max_engines; i++)
> -		put_device(engine_confdev(idxd->engines[i]));
> +	idxd_clean_engines(idxd);
>   err_engine:
> -	for (i = 0; i < idxd->max_wqs; i++)
> -		put_device(wq_confdev(idxd->wqs[i]));
> +	idxd_clean_wqs(idxd);
>   err_wqs:
>  	return rc;
>  }

Re: [PATCH v4 5/9] dmaengine: idxd: Add missing cleanups in cleanup internals

[Dave Jiang]

On 4/4/25 5:02 AM, Shuai Xue wrote:
> The idxd_cleanup_internals() function only decreases the reference count
> of groups, engines, and wqs but is missing the step to release memory
> resources.
> 
> To fix this, use the cleanup helper to properly release the memory
> resources.
> 
> Fixes: ddf742d4f3f1 ("dmaengine: idxd: Add missing cleanup for early error out in probe call")
> Cc: stable@vger.kernel.org
> Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com>
> Reviewed-by: Fenghua Yu <fenghuay@nvidia.com>

Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> ---
>  drivers/dma/idxd/init.c | 11 +++--------
>  1 file changed, 3 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c
> index a40fb2fd5006..f8129d2d53f1 100644
> --- a/drivers/dma/idxd/init.c
> +++ b/drivers/dma/idxd/init.c
> @@ -407,14 +407,9 @@ static int idxd_setup_groups(struct idxd_device *idxd)
>  
>  static void idxd_cleanup_internals(struct idxd_device *idxd)
>  {
> -	int i;
> -
> -	for (i = 0; i < idxd->max_groups; i++)
> -		put_device(group_confdev(idxd->groups[i]));
> -	for (i = 0; i < idxd->max_engines; i++)
> -		put_device(engine_confdev(idxd->engines[i]));
> -	for (i = 0; i < idxd->max_wqs; i++)
> -		put_device(wq_confdev(idxd->wqs[i]));
> +	idxd_clean_groups(idxd);
> +	idxd_clean_engines(idxd);
> +	idxd_clean_wqs(idxd);
>  	destroy_workqueue(idxd->wq);
>  }
>  

Re: [PATCH v4 8/9] dmaengine: idxd: Add missing idxd cleanup to fix memory leak in remove call

[Dave Jiang]

On 4/4/25 5:02 AM, Shuai Xue wrote:
> The remove call stack is missing idxd cleanup to free bitmap, ida and
> the idxd_device. Call idxd_free() helper routines to make sure we exit
> gracefully.
> 
> Fixes: bfe1d56091c1 ("dmaengine: idxd: Init and probe for Intel data accelerators")
> Cc: stable@vger.kernel.org
> Suggested-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
> Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com>
> Reviewed-by: Fenghua Yu <fenghuay@nvidia.com>

Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> ---
>  drivers/dma/idxd/init.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c
> index f2b5b17538c0..974b926bd930 100644
> --- a/drivers/dma/idxd/init.c
> +++ b/drivers/dma/idxd/init.c
> @@ -1335,6 +1335,7 @@ static void idxd_remove(struct pci_dev *pdev)
>  	destroy_workqueue(idxd->wq);
>  	perfmon_pmu_remove(idxd);
>  	put_device(idxd_confdev(idxd));
> +	idxd_free(idxd);
>  }
>  
>  static struct pci_driver idxd_pci_driver = {

Re: [PATCH v4 9/9] dmaengine: idxd: Refactor remove call with idxd_cleanup() helper

[Dave Jiang]

On 4/4/25 5:02 AM, Shuai Xue wrote:
> The idxd_cleanup() helper cleans up perfmon, interrupts, internals and
> so on. Refactor remove call with the idxd_cleanup() helper to avoid code
> duplication. Note, this also fixes the missing put_device() for idxd
> groups, enginces and wqs.
> 
> Fixes: bfe1d56091c1 ("dmaengine: idxd: Init and probe for Intel data accelerators")
> Cc: stable@vger.kernel.org
> Suggested-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
> Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com>
> Reviewed-by: Fenghua Yu <fenghuay@nvidia.com>

Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> ---
>  drivers/dma/idxd/init.c | 14 ++------------
>  1 file changed, 2 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c
> index 974b926bd930..760b7d81fcd8 100644
> --- a/drivers/dma/idxd/init.c
> +++ b/drivers/dma/idxd/init.c
> @@ -1308,7 +1308,6 @@ static void idxd_shutdown(struct pci_dev *pdev)
>  static void idxd_remove(struct pci_dev *pdev)
>  {
>  	struct idxd_device *idxd = pci_get_drvdata(pdev);
> -	struct idxd_irq_entry *irq_entry;
>  
>  	idxd_unregister_devices(idxd);
>  	/*
> @@ -1321,21 +1320,12 @@ static void idxd_remove(struct pci_dev *pdev)
>  	get_device(idxd_confdev(idxd));
>  	device_unregister(idxd_confdev(idxd));
>  	idxd_shutdown(pdev);
> -	if (device_pasid_enabled(idxd))
> -		idxd_disable_system_pasid(idxd);
>  	idxd_device_remove_debugfs(idxd);
> -
> -	irq_entry = idxd_get_ie(idxd, 0);
> -	free_irq(irq_entry->vector, irq_entry);
> -	pci_free_irq_vectors(pdev);
> +	idxd_cleanup(idxd);
>  	pci_iounmap(pdev, idxd->reg_base);
> -	if (device_user_pasid_enabled(idxd))
> -		idxd_disable_sva(pdev);
> -	pci_disable_device(pdev);
> -	destroy_workqueue(idxd->wq);
> -	perfmon_pmu_remove(idxd);
>  	put_device(idxd_confdev(idxd));
>  	idxd_free(idxd);
> +	pci_disable_device(pdev);
>  }
>  
>  static struct pci_driver idxd_pci_driver = {

Re: [PATCH v4 0/9] dmaengine: idxd: fix memory leak in error handling path

[Vinod Koul]

On Fri, 04 Apr 2025 20:02:08 +0800, Shuai Xue wrote:
> changes since v3:
> - remove a blank line to fix checkpatch warning per Fenghua
> - collect Reviewed-by tags from Fenghua
> 
> 
> changes since v2:
> - add to cc stable per Markus
> - add patch 4 to fix memory leak in idxd_setup_internals per Fenghua
> - collect Reviewed-by tag for patch 2 from Fenghua
> - fix reference cnt in remove() per Fenghua
> 
> [...]

Applied, thanks!

[1/9] dmaengine: idxd: fix memory leak in error handling path of idxd_setup_wqs
      commit: 3fd2f4bc010cdfbc07dd21018dc65bd9370eb7a4
[2/9] dmaengine: idxd: fix memory leak in error handling path of idxd_setup_engines
      commit: 817bced19d1dbdd0b473580d026dc0983e30e17b
[3/9] dmaengine: idxd: fix memory leak in error handling path of idxd_setup_groups
      commit: aa6f4f945b10eac57aed46154ae7d6fada7fccc7
[4/9] dmaengine: idxd: Add missing cleanup for early error out in idxd_setup_internals
      commit: 61259fb96e023f7299c442c48b13e72c441fc0f2
[5/9] dmaengine: idxd: Add missing cleanups in cleanup internals
      commit: 61d651572b6c4fe50c7b39a390760f3a910c7ccf
[6/9] dmaengine: idxd: fix memory leak in error handling path of idxd_alloc
      commit: 46a5cca76c76c86063000a12936f8e7875295838
[7/9] dmaengine: idxd: fix memory leak in error handling path of idxd_pci_probe
      commit: 90022b3a6981ec234902be5dbf0f983a12c759fc
[8/9] dmaengine: idxd: Add missing idxd cleanup to fix memory leak in remove call
      commit: d5449ff1b04dfe9ed8e455769aa01e4c2ccf6805
[9/9] dmaengine: idxd: Refactor remove call with idxd_cleanup() helper
      commit: a409e919ca321cc0e28f8abf96fde299f0072a81

Best regards,
-- 
~Vinod

Re: [PATCH v4 0/9] dmaengine: idxd: fix memory leak in error handling path

[Guenter Roeck]

Hi,

On Fri, Apr 04, 2025 at 08:02:08PM +0800, Shuai Xue wrote:
> changes since v3:
> - remove a blank line to fix checkpatch warning per Fenghua
> - collect Reviewed-by tags from Fenghua
> 
> 
> changes since v2:
> - add to cc stable per Markus
> - add patch 4 to fix memory leak in idxd_setup_internals per Fenghua
> - collect Reviewed-by tag for patch 2 from Fenghua
> - fix reference cnt in remove() per Fenghua
> 
> changes since v1:
> - add Reviewed-by tag for patch 1-5 from Dave Jiang
> - add fixes tag
> - add patch 6 and 7 to fix memory leak in remove call per Vinicius
> 
> Shuai Xue (9):
>   dmaengine: idxd: fix memory leak in error handling path of
>     idxd_setup_wqs
>   dmaengine: idxd: fix memory leak in error handling path of
>     idxd_setup_engines
>   dmaengine: idxd: fix memory leak in error handling path of
>     idxd_setup_groups
>   dmaengine: idxd: Add missing cleanup for early error out in
>     idxd_setup_internals
>   dmaengine: idxd: Add missing cleanups in cleanup internals
>   dmaengine: idxd: fix memory leak in error handling path of idxd_alloc
>   dmaengine: idxd: fix memory leak in error handling path of
>     idxd_pci_probe
>   dmaengine: idxd: Add missing idxd cleanup to fix memory leak in remove
>     call
>   dmaengine: idxd: Refactor remove call with idxd_cleanup() helper
> 
>  drivers/dma/idxd/init.c | 159 ++++++++++++++++++++++++++++------------
>  1 file changed, 113 insertions(+), 46 deletions(-)
> 

This patch series, as applied to 6.6 and 6.12 kernels, results in a variety
of warning backtraces and crashes when unloading idxd the driver, such as

da_free called for id=0 which is not allocated.
refcount_t: underflow; use-after-free.
list_add corruption. next->prev should be prev (ff11d2ed9908ecd0), but was ff11d2ed8a5d0ba0. (next=ff11d2ed8a5d0ba0).

Looking into it, I see that many resources are now released from functions
such as idxd_cleanup() and idxd_free(). At the same time, the calls to
put_device(idxd_confdev(idxd)) trigger calls to idxd_conf_device_release()
which releases the same resources. On top of that,
put_device(idxd_confdev(idxd)) is now called from idxd_remove() _and_ from
idxd_free() [which is called from idxd_remove()], on top of the put_device()
called from device_unregister() itself.

Does this actually work in the upstream kernel ? What prevents duplicate
release of resources from idxd_free(), idxd_cleanup(), and
idxd_conf_device_release() ? And why would idxd_remove() need the extra
put_device() call ?

Sorry if I am missing something, I just try to understand the logic behind
this patch series and why it triggers crashes and warning backtraces in 6.6
and 6.12 kernels.

Thanks,
Guenter