idxd: Fix Intel Data Streaming Accelerator double-free on error path

[PATCH] idxd: Fix Intel Data Streaming Accelerator double-free on error path

Posted by Daniel J Blueman 1 week, 3 days ago

During IDXD driver probe unwind from an earlier resource allocation
failure, multiple use-after-free codepaths are taken leading to attempted
double-free of ID allocator entries and memory allocations, eg:

ida_free called for id=64 which is not allocated.
WARNING: lib/idr.c:594 at ida_free+0x1af/0x1f4, CPU#900: kworker/900:1/11863
...
Call Trace:
<TASK>
? ida_destroy+0x258/0x258
idxd_pci_probe_alloc+0x342e/0x348c
? multi_u64_to_bmap+0xc9/0xc9
? queued_read_unlock+0x1e/0x1e
? __schedule+0x2e43/0x2ee6
? idxd_reset_done+0x12ca/0x12ca
idxd_pci_probe+0x15/0x17
...

Fix this by releasing these allocations only after use and once.

Validated on 8 socket and 16 socket (XNC node controller) Intel Saphire
Rapids XCC systems with no KASAN, Kmemleak or lockdep reports.

Signed-off-by: Daniel J Blueman <daniel@quora.org>
Cc: stable@vger.kernel.org

---
 drivers/dma/idxd/init.c  | 21 +--------------------
 drivers/dma/idxd/sysfs.c |  1 -
 2 files changed, 1 insertion(+), 21 deletions(-)

diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c
index 2acc34b3daff..5d2b869df745 100644
--- a/drivers/dma/idxd/init.c
+++ b/drivers/dma/idxd/init.c
@@ -167,13 +167,9 @@ static void idxd_clean_wqs(struct idxd_device *idxd)
 		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)
@@ -277,9 +273,7 @@ static void idxd_clean_engines(struct idxd_device *idxd)
 		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)
@@ -341,9 +335,7 @@ static void idxd_clean_groups(struct idxd_device *idxd)
 	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)
@@ -590,17 +582,6 @@ 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;
@@ -1239,7 +1220,7 @@ int idxd_pci_probe_alloc(struct idxd_device *idxd, struct pci_dev *pdev,
  err:
 	pci_iounmap(pdev, idxd->reg_base);
  err_iomap:
-	idxd_free(idxd);
+	put_device(idxd_confdev(idxd));
  err_idxd_alloc:
 	pci_disable_device(pdev);
 	return rc;
diff --git a/drivers/dma/idxd/sysfs.c b/drivers/dma/idxd/sysfs.c
index 9f0701021af0..819f2024ba0b 100644
--- a/drivers/dma/idxd/sysfs.c
+++ b/drivers/dma/idxd/sysfs.c
@@ -1818,7 +1818,6 @@ static void idxd_conf_device_release(struct device *dev)
 	kfree(idxd->engines);
 	kfree(idxd->evl);
 	kmem_cache_destroy(idxd->evl_cache);
-	ida_free(&idxd_ida, idxd->id);
 	bitmap_free(idxd->opcap_bmap);
 	kfree(idxd);
 }
-- 
2.43.0

Re: [PATCH] idxd: Fix Intel Data Streaming Accelerator double-free on error path

Posted by Vinicius Costa Gomes 1 week, 3 days ago

Hi Daniel,

Daniel J Blueman <daniel@quora.org> writes:

> During IDXD driver probe unwind from an earlier resource allocation
> failure, multiple use-after-free codepaths are taken leading to attempted
> double-free of ID allocator entries and memory allocations, eg:
>
> ida_free called for id=64 which is not allocated.
> WARNING: lib/idr.c:594 at ida_free+0x1af/0x1f4, CPU#900: kworker/900:1/11863
> ...
> Call Trace:
> <TASK>
> ? ida_destroy+0x258/0x258
> idxd_pci_probe_alloc+0x342e/0x348c
> ? multi_u64_to_bmap+0xc9/0xc9
> ? queued_read_unlock+0x1e/0x1e
> ? __schedule+0x2e43/0x2ee6
> ? idxd_reset_done+0x12ca/0x12ca
> idxd_pci_probe+0x15/0x17
> ...
>
> Fix this by releasing these allocations only after use and once.
>
> Validated on 8 socket and 16 socket (XNC node controller) Intel Saphire
> Rapids XCC systems with no KASAN, Kmemleak or lockdep reports.

Can you confirm that you still see this issue after you apply the series
I sent last week?



Cheers,
-- 
Vinicius

Re: [PATCH] idxd: Fix Intel Data Streaming Accelerator double-free on error path

Posted by Dave Jiang 1 week, 3 days ago


On 1/27/26 12:52 AM, Daniel J Blueman wrote:
> During IDXD driver probe unwind from an earlier resource allocation
> failure, multiple use-after-free codepaths are taken leading to attempted
> double-free of ID allocator entries and memory allocations, eg:
> 
> ida_free called for id=64 which is not allocated.
> WARNING: lib/idr.c:594 at ida_free+0x1af/0x1f4, CPU#900: kworker/900:1/11863
> ...
> Call Trace:
> <TASK>
> ? ida_destroy+0x258/0x258
> idxd_pci_probe_alloc+0x342e/0x348c
> ? multi_u64_to_bmap+0xc9/0xc9
> ? queued_read_unlock+0x1e/0x1e
> ? __schedule+0x2e43/0x2ee6
> ? idxd_reset_done+0x12ca/0x12ca
> idxd_pci_probe+0x15/0x17
> ...
> 
> Fix this by releasing these allocations only after use and once.
> 
> Validated on 8 socket and 16 socket (XNC node controller) Intel Saphire
> Rapids XCC systems with no KASAN, Kmemleak or lockdep reports.
> 
> Signed-off-by: Daniel J Blueman <daniel@quora.org>
> Cc: stable@vger.kernel.org
> 

Can you provide a Fixes tag please?

DJ

> ---
>  drivers/dma/idxd/init.c  | 21 +--------------------
>  drivers/dma/idxd/sysfs.c |  1 -
>  2 files changed, 1 insertion(+), 21 deletions(-)
> 
> diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c
> index 2acc34b3daff..5d2b869df745 100644
> --- a/drivers/dma/idxd/init.c
> +++ b/drivers/dma/idxd/init.c
> @@ -167,13 +167,9 @@ static void idxd_clean_wqs(struct idxd_device *idxd)
>  		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)
> @@ -277,9 +273,7 @@ static void idxd_clean_engines(struct idxd_device *idxd)
>  		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)
> @@ -341,9 +335,7 @@ static void idxd_clean_groups(struct idxd_device *idxd)
>  	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)
> @@ -590,17 +582,6 @@ 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;
> @@ -1239,7 +1220,7 @@ int idxd_pci_probe_alloc(struct idxd_device *idxd, struct pci_dev *pdev,
>   err:
>  	pci_iounmap(pdev, idxd->reg_base);
>   err_iomap:
> -	idxd_free(idxd);
> +	put_device(idxd_confdev(idxd));
>   err_idxd_alloc:
>  	pci_disable_device(pdev);
>  	return rc;
> diff --git a/drivers/dma/idxd/sysfs.c b/drivers/dma/idxd/sysfs.c
> index 9f0701021af0..819f2024ba0b 100644
> --- a/drivers/dma/idxd/sysfs.c
> +++ b/drivers/dma/idxd/sysfs.c
> @@ -1818,7 +1818,6 @@ static void idxd_conf_device_release(struct device *dev)
>  	kfree(idxd->engines);
>  	kfree(idxd->evl);
>  	kmem_cache_destroy(idxd->evl_cache);
> -	ida_free(&idxd_ida, idxd->id);
>  	bitmap_free(idxd->opcap_bmap);
>  	kfree(idxd);
>  }