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

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

[Shuai Xue]

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 | 160 ++++++++++++++++++++++++++++------------
 1 file changed, 114 insertions(+), 46 deletions(-)

-- 
2.39.3

[PATCH v3 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>
---
 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 b946f78f85e1..8b775c4a43fc 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.39.3

Re: [PATCH v3 1/9] dmaengine: idxd: fix memory leak in error handling path of idxd_setup_wqs()

[Markus Elfring]

…
> +++ b/drivers/dma/idxd/init.c
…
> @@ -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;
>  		}

I suggest to move such an error code assignment also to the end of this function implementation.

Regards,
Markus

Re: [PATCH v3 1/9] dmaengine: idxd: fix memory leak in error handling path of idxd_setup_wqs()

[Shuai Xue]

在 2025/3/9 17:10, Markus Elfring 写道:
> …
>> +++ b/drivers/dma/idxd/init.c
> …
>> @@ -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;
>>   		}
> 
> I suggest to move such an error code assignment also to the end of this function implementation.
> 

I prefer to set error code before jumping to error handling label
so that we can extend/change the error code in any future path.

> Regards,
> Markus

Thanks for valuable comments.
Best Regards,
Shuai

Re: [v3 1/9] dmaengine: idxd: fix memory leak in error handling path of idxd_setup_wqs()

[Markus Elfring]

> I prefer to set error code before jumping to error handling label
> so that we can extend/change the error code in any future path.
I prefer to avoid duplicate source code another bit.

Regards,
Markus

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

[Fenghua Yu]

On 3/8/25 22:20, Shuai Xue wrote:
> 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>

Thanks.

-Fenghua

> ---
>   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 b946f78f85e1..8b775c4a43fc 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;
>   }
>   

[PATCH v3 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 8b775c4a43fc..635838a81e0f 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.39.3

[PATCH v3 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>
---
 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 635838a81e0f..fe4a14813bba 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.39.3

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

[Fenghua Yu]

On 3/8/25 22:20, Shuai Xue wrote:
> 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>

Thanks.

-Fenghua

> ---
>   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 635838a81e0f..fe4a14813bba 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;
>   }
>   

[PATCH v3 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>
---
 drivers/dma/idxd/init.c | 59 ++++++++++++++++++++++++++++++++++++-----
 1 file changed, 52 insertions(+), 7 deletions(-)

diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c
index fe4a14813bba..7334085939dc 100644
--- a/drivers/dma/idxd/init.c
+++ b/drivers/dma/idxd/init.c
@@ -155,6 +155,26 @@ 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 +265,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 +331,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 +458,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 +489,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.39.3

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

[Dave Jiang]

On 3/8/25 11:20 PM, 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: Dave Jiang <dave.jiang@intel.com>

> ---
>  drivers/dma/idxd/init.c | 59 ++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 52 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c
> index fe4a14813bba..7334085939dc 100644
> --- a/drivers/dma/idxd/init.c
> +++ b/drivers/dma/idxd/init.c
> @@ -155,6 +155,26 @@ 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 +265,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 +331,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 +458,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 +489,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 v3 4/9] dmaengine: idxd: Add missing cleanup for early error out in idxd_setup_internals

[Fenghua Yu]

On 3/8/25 22:20, 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>
> ---
>   drivers/dma/idxd/init.c | 59 ++++++++++++++++++++++++++++++++++++-----
>   1 file changed, 52 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c
> index fe4a14813bba..7334085939dc 100644
> --- a/drivers/dma/idxd/init.c
> +++ b/drivers/dma/idxd/init.c
> @@ -155,6 +155,26 @@ 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);
> +
Please remove this blank line here, warned by checkpatch.
> +	}
> +	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 +265,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 +331,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 +458,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 +489,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;
>   }

Reviewed-by: Fenghua Yu <fenghuay@nvidia.com>

Thanks.

-Fenghua

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

[Shuai Xue]

在 2025/4/3 07:26, Fenghua Yu 写道:
> 
> On 3/8/25 22:20, 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>
>> ---
>>   drivers/dma/idxd/init.c | 59 ++++++++++++++++++++++++++++++++++++-----
>>   1 file changed, 52 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c
>> index fe4a14813bba..7334085939dc 100644
>> --- a/drivers/dma/idxd/init.c
>> +++ b/drivers/dma/idxd/init.c
>> @@ -155,6 +155,26 @@ 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);
>> +
> Please remove this blank line here, warned by checkpatch.

I checked with checkpatch, but it does not warn (:

./scripts/checkpatch.pl idxd-fix-v3/0004-dmaengine-idxd-Add-missing-cleanup-for-early-error-o.patch
total: 0 errors, 0 warnings, 91 lines checked

idxd-fix-v3/0004-dmaengine-idxd-Add-missing-cleanup-for-early-error-o.patch has no obvious style problems and is ready for submission.


Will send a new version to fix it and collect all you reviewed-by tag,
Thanks.

>> +    }
>> +    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 +265,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 +331,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 +458,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 +489,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;
>>   }
> 
> Reviewed-by: Fenghua Yu <fenghuay@nvidia.com>
> 
> Thanks.
> 
> -Fenghua


Thanks.

Cheers,
Shuai

[PATCH v3 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>
---
 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 7334085939dc..cf5dc981be32 100644
--- a/drivers/dma/idxd/init.c
+++ b/drivers/dma/idxd/init.c
@@ -408,14 +408,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.39.3

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

[Dave Jiang]

On 3/8/25 11:20 PM, 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: 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 7334085939dc..cf5dc981be32 100644
> --- a/drivers/dma/idxd/init.c
> +++ b/drivers/dma/idxd/init.c
> @@ -408,14 +408,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 v3 5/9] dmaengine: idxd: Add missing cleanups in cleanup internals

[Fenghua Yu]

On 3/8/25 22:20, 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>

Thanks.

-Fenghua

> ---
>   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 7334085939dc..cf5dc981be32 100644
> --- a/drivers/dma/idxd/init.c
> +++ b/drivers/dma/idxd/init.c
> @@ -408,14 +408,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);
>   }
>   

[PATCH v3 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>
---
 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 cf5dc981be32..71d92c05c0c6 100644
--- a/drivers/dma/idxd/init.c
+++ b/drivers/dma/idxd/init.c
@@ -605,28 +605,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.39.3

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

[Fenghua Yu]

On 3/8/25 22:20, Shuai Xue wrote:
> 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>

Thanks.

-Fenghua

> ---
>   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 cf5dc981be32..71d92c05c0c6 100644
> --- a/drivers/dma/idxd/init.c
> +++ b/drivers/dma/idxd/init.c
> @@ -605,28 +605,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)

[PATCH v3 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>
---
 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 71d92c05c0c6..890b2bbd2c5e 100644
--- a/drivers/dma/idxd/init.c
+++ b/drivers/dma/idxd/init.c
@@ -588,6 +588,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;
@@ -1257,7 +1268,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.39.3

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

[Fenghua Yu]

On 3/8/25 22:20, Shuai Xue wrote:
> 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>

Thanks.

-Fenghua

> ---
>   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 71d92c05c0c6..890b2bbd2c5e 100644
> --- a/drivers/dma/idxd/init.c
> +++ b/drivers/dma/idxd/init.c
> @@ -588,6 +588,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;
> @@ -1257,7 +1268,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;

[PATCH v3 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>
---
 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 890b2bbd2c5e..ecb8d534fac4 100644
--- a/drivers/dma/idxd/init.c
+++ b/drivers/dma/idxd/init.c
@@ -1337,6 +1337,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.39.3

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

[Dave Jiang]

On 3/8/25 11:20 PM, 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: 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 890b2bbd2c5e..ecb8d534fac4 100644
> --- a/drivers/dma/idxd/init.c
> +++ b/drivers/dma/idxd/init.c
> @@ -1337,6 +1337,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 v3 8/9] dmaengine: idxd: Add missing idxd cleanup to fix memory leak in remove call

[Fenghua Yu]

On 3/8/25 22:20, 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>

Thanks.

-Fenghua

> ---
>   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 890b2bbd2c5e..ecb8d534fac4 100644
> --- a/drivers/dma/idxd/init.c
> +++ b/drivers/dma/idxd/init.c
> @@ -1337,6 +1337,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 = {

[PATCH v3 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>
---
 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 ecb8d534fac4..22b411b470be 100644
--- a/drivers/dma/idxd/init.c
+++ b/drivers/dma/idxd/init.c
@@ -1310,7 +1310,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);
 	/*
@@ -1323,21 +1322,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.39.3

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

[Dave Jiang]

On 3/8/25 11:20 PM, 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: 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 ecb8d534fac4..22b411b470be 100644
> --- a/drivers/dma/idxd/init.c
> +++ b/drivers/dma/idxd/init.c
> @@ -1310,7 +1310,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);
>  	/*
> @@ -1323,21 +1322,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 v3 9/9] dmaengine: idxd: Refactor remove call with idxd_cleanup() helper

[Fenghua Yu]

On 3/8/25 22:20, 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>

Thanks.

-Fenghua

> ---
>   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 ecb8d534fac4..22b411b470be 100644
> --- a/drivers/dma/idxd/init.c
> +++ b/drivers/dma/idxd/init.c
> @@ -1310,7 +1310,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);
>   	/*
> @@ -1323,21 +1322,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 = {