[PATCH v1 0/5] dmaengine: idxd: fix memory leak in error handling path

[PATCH v1 0/5] dmaengine: idxd: fix memory leak in error handling path

[Shuai Xue]

Shuai Xue (5):
  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: fix memory leak in error handling path of idxd_alloc
  dmaengine: idxd: fix memory leak in error handling path of
    idxd_pci_probe

 drivers/dma/idxd/init.c | 62 ++++++++++++++++++++++++++++++++---------
 1 file changed, 49 insertions(+), 13 deletions(-)

-- 
2.39.3

[PATCH v1 1/5] 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.

Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com>
---
 drivers/dma/idxd/init.c | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c
index 234c1c658ec7..6772d9251cd7 100644
--- a/drivers/dma/idxd/init.c
+++ b/drivers/dma/idxd/init.c
@@ -167,8 +167,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,6 +189,7 @@ static int idxd_setup_wqs(struct idxd_device *idxd)
 		rc = dev_set_name(conf_dev, "wq%d.%d", idxd->id, wq->id);
 		if (rc < 0) {
 			put_device(conf_dev);
+			kfree(wq);
 			goto err;
 		}
 
@@ -202,6 +203,7 @@ static int idxd_setup_wqs(struct idxd_device *idxd)
 		wq->wqcfg = kzalloc_node(idxd->wqcfg_size, GFP_KERNEL, dev_to_node(dev));
 		if (!wq->wqcfg) {
 			put_device(conf_dev);
+			kfree(wq);
 			rc = -ENOMEM;
 			goto err;
 		}
@@ -209,7 +211,9 @@ 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) {
+				kfree(wq->wqcfg);
 				put_device(conf_dev);
+				kfree(wq);
 				rc = -ENOMEM;
 				goto err;
 			}
@@ -223,11 +227,21 @@ static int idxd_setup_wqs(struct idxd_device *idxd)
 	return 0;
 
  err:
-	while (--i >= 0) {
+	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 v1 1/5] dmaengine: idxd: fix memory leak in error handling path of idxd_setup_wqs

[Dave Jiang]

On 1/10/25 1:22 AM, 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.
> 
> Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>

> ---
>  drivers/dma/idxd/init.c | 20 +++++++++++++++++---
>  1 file changed, 17 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c
> index 234c1c658ec7..6772d9251cd7 100644
> --- a/drivers/dma/idxd/init.c
> +++ b/drivers/dma/idxd/init.c
> @@ -167,8 +167,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,6 +189,7 @@ static int idxd_setup_wqs(struct idxd_device *idxd)
>  		rc = dev_set_name(conf_dev, "wq%d.%d", idxd->id, wq->id);
>  		if (rc < 0) {
>  			put_device(conf_dev);
> +			kfree(wq);
>  			goto err;
>  		}
>  
> @@ -202,6 +203,7 @@ static int idxd_setup_wqs(struct idxd_device *idxd)
>  		wq->wqcfg = kzalloc_node(idxd->wqcfg_size, GFP_KERNEL, dev_to_node(dev));
>  		if (!wq->wqcfg) {
>  			put_device(conf_dev);
> +			kfree(wq);
>  			rc = -ENOMEM;
>  			goto err;
>  		}
> @@ -209,7 +211,9 @@ 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) {
> +				kfree(wq->wqcfg);
>  				put_device(conf_dev);
> +				kfree(wq);
>  				rc = -ENOMEM;
>  				goto err;
>  			}
> @@ -223,11 +227,21 @@ static int idxd_setup_wqs(struct idxd_device *idxd)
>  	return 0;
>  
>   err:
> -	while (--i >= 0) {
> +	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 v1 2/5] 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.

Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.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 6772d9251cd7..12df895dcbe9 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

Re: [PATCH v1 2/5] dmaengine: idxd: fix memory leak in error handling path of idxd_setup_engines

[Dave Jiang]

On 1/10/25 1:22 AM, Shuai Xue wrote:
> 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.
> 
> 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 6772d9251cd7..12df895dcbe9 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;
>  }
>  

[PATCH v1 3/5] 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.

Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.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 12df895dcbe9..04a7d7706e53 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 v1 3/5] dmaengine: idxd: fix memory leak in error handling path of idxd_setup_groups

[Dave Jiang]

On 1/10/25 1:22 AM, 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.
> 
> 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 12df895dcbe9..04a7d7706e53 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 v1 4/5] 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.

Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.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 04a7d7706e53..f0e3244d630d 100644
--- a/drivers/dma/idxd/init.c
+++ b/drivers/dma/idxd/init.c
@@ -565,28 +565,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 v1 4/5] dmaengine: idxd: fix memory leak in error handling path of idxd_alloc

[Dave Jiang]

On 1/10/25 1:22 AM, 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.
> 
> 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 04a7d7706e53..f0e3244d630d 100644
> --- a/drivers/dma/idxd/init.c
> +++ b/drivers/dma/idxd/init.c
> @@ -565,28 +565,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)

Re: [PATCH v1 4/5] dmaengine: idxd: fix memory leak in error handling path of idxd_alloc

[Dave Jiang]

On 1/10/25 1:22 AM, 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.
> 
> 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 04a7d7706e53..f0e3244d630d 100644
> --- a/drivers/dma/idxd/init.c
> +++ b/drivers/dma/idxd/init.c
> @@ -565,28 +565,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)

Re: [PATCH v1 4/5] dmaengine: idxd: fix memory leak in error handling path of idxd_alloc

[Shuai Xue]

在 2025/2/15 01:01, Dave Jiang 写道:
> 
> 
> On 1/10/25 1:22 AM, 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.
>>
>> Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com>
> 
> Reviewed-by: Dave Jiang <dave.jiang@intel.com>

I received two Reviewed-by from you for this patch, is this (the second) for
Patch 5/5.

Thanks.
Shuai

[PATCH v1 5/5] 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.

Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com>
---
 drivers/dma/idxd/init.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c
index f0e3244d630d..9b44f5d38d3a 100644
--- a/drivers/dma/idxd/init.c
+++ b/drivers/dma/idxd/init.c
@@ -548,6 +548,14 @@ 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)
+{
+	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;
@@ -820,7 +828,7 @@ static int idxd_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
  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 v1 5/5] dmaengine: idxd: fix memory leak in error handling path of idxd_pci_probe

[Vinicius Costa Gomes]

Shuai Xue <xueshuai@linux.alibaba.com> writes:

> 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.
>
> Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com>
> ---
>  drivers/dma/idxd/init.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c
> index f0e3244d630d..9b44f5d38d3a 100644
> --- a/drivers/dma/idxd/init.c
> +++ b/drivers/dma/idxd/init.c
> @@ -548,6 +548,14 @@ 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)
> +{
> +	put_device(idxd_confdev(idxd));
> +	bitmap_free(idxd->opcap_bmap);
> +	ida_free(&idxd_ida, idxd->id);
> +	kfree(idxd);
> +}
> +

I was expecting this function to be called from idxd_remove() as well.
Perhaps on a separate patch?

>  static struct idxd_device *idxd_alloc(struct pci_dev *pdev, struct idxd_driver_data *data)
>  {
>  	struct device *dev = &pdev->dev;
> @@ -820,7 +828,7 @@ static int idxd_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>   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
>

Cheers,
-- 
Vinicius

Re: [PATCH v1 5/5] dmaengine: idxd: fix memory leak in error handling path of idxd_pci_probe

[Shuai Xue]

在 2025/2/15 07:41, Vinicius Costa Gomes 写道:
> Shuai Xue <xueshuai@linux.alibaba.com> writes:
> 
>> 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.
>>
>> Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com>
>> ---
>>   drivers/dma/idxd/init.c | 10 +++++++++-
>>   1 file changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c
>> index f0e3244d630d..9b44f5d38d3a 100644
>> --- a/drivers/dma/idxd/init.c
>> +++ b/drivers/dma/idxd/init.c
>> @@ -548,6 +548,14 @@ 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)
>> +{
>> +	put_device(idxd_confdev(idxd));
>> +	bitmap_free(idxd->opcap_bmap);
>> +	ida_free(&idxd_ida, idxd->id);
>> +	kfree(idxd);
>> +}
>> +
> 
> I was expecting this function to be called from idxd_remove() as well.
> Perhaps on a separate patch?

You are right, I missed the idxd_remove() to destroy idxd device.
Will add a new patch in next version.

> 
>>   static struct idxd_device *idxd_alloc(struct pci_dev *pdev, struct idxd_driver_data *data)
>>   {
>>   	struct device *dev = &pdev->dev;
>> @@ -820,7 +828,7 @@ static int idxd_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>>    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
>>
> 
> Cheers,

Thanks.
Shuai

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

[Shuai Xue]

在 2025/1/10 16:22, Shuai Xue 写道:
> Shuai Xue (5):
>    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: fix memory leak in error handling path of idxd_alloc
>    dmaengine: idxd: fix memory leak in error handling path of
>      idxd_pci_probe
> 
>   drivers/dma/idxd/init.c | 62 ++++++++++++++++++++++++++++++++---------
>   1 file changed, 49 insertions(+), 13 deletions(-)
> 

Hi, all,

Gentle Ping.

Thanks.
Shuai

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

[Dave Jiang]

On 2/13/25 11:43 PM, Shuai Xue wrote:
> 
> 
> 在 2025/1/10 16:22, Shuai Xue 写道:
>> Shuai Xue (5):
>>    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: fix memory leak in error handling path of idxd_alloc
>>    dmaengine: idxd: fix memory leak in error handling path of
>>      idxd_pci_probe
>>
>>   drivers/dma/idxd/init.c | 62 ++++++++++++++++++++++++++++++++---------
>>   1 file changed, 49 insertions(+), 13 deletions(-)
>>
> 
> Hi, all,
> 
> Gentle Ping.

- Fenghua, who has left Intel
+ Vinicius, who is the current maintainer


> 
> Thanks.
> Shuai

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

[Markus Elfring]

…
>  drivers/dma/idxd/init.c | 62 ++++++++++++++++++++++++++++++++---------
…

How do you think about to add any tags (like “Fixes” and “Cc”)
for the presented patches accordingly?
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.14-rc2#n145

Regards,
Markus

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

[Dave Jiang]

On 2/14/25 10:15 AM, Markus Elfring wrote:
> …
>>  drivers/dma/idxd/init.c | 62 ++++++++++++++++++++++++++++++++---------
> …
> 
> How do you think about to add any tags (like “Fixes” and “Cc”)
> for the presented patches accordingly?
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.14-rc2#n145

I actually thought about asking for that. But I believe the fixes are against code that just went in for 6.14 so backporting won't be necessary. 

> 
> Regards,
> Markus

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

[Shuai Xue]

在 2025/2/15 01:22, Dave Jiang 写道:
> 
> 
> On 2/14/25 10:15 AM, Markus Elfring wrote:
>> …
>>>   drivers/dma/idxd/init.c | 62 ++++++++++++++++++++++++++++++++---------
>> …
>>
>> How do you think about to add any tags (like “Fixes” and “Cc”)
>> for the presented patches accordingly?
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.14-rc2#n145
> 
> I actually thought about asking for that. But I believe the fixes are against code that just went in for 6.14 so backporting won't be necessary.

Will add fixes tags in next version.

> 
>>
>> Regards,
>> Markus

Thanks.
Shuai