mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 0/2] dmaengine: idxd: fix crashes in the probe error path
@ 2026-10-28  0:37 Jingyi Li
  2026-10-28  0:37 ` [PATCH 1/2] dmaengine: idxd: fix double free in idxd_free() Jingyi Li
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Jingyi Li @ 2026-10-28  0:37 UTC (permalink / raw)
  To: vkoul, vinicius.gomes
  Cc: dave.jiang, xueshuai, fenghuay, Frank.Li, dmaengine, linux-kernel

When idxd_probe() fails, the cleanup in idxd_pci_probe_alloc() runs into
two bugs:

 1. idxd_free() frees opcap_bmap, the IDA id and idxd after put_device()
    has already freed them via idxd_conf_device_release() (double free).

 2. idxd_conf_device_release() calls destroy_workqueue() on a NULL
    workqueue when probe fails before idxd_setup_internals(), and on an
    already destroyed one after idxd_cleanup_internals() or the err_evl
    path of idxd_setup_internals().

We hit both in a kdump crash kernel, where the DSA device fails to reset
and reports HALTED. Depending on the kernel version, the crash kernel
either oopses in destroy_workqueue() (2) or corrupts the slab (1), and
no dump is saved.

Testing: the equivalent change was tested on 6.18.57 on 10 systems with
DSA, triggering kdump with idxd loaded in the crash kernel:

 - unpatched: 10/10 NULL pointer dereference in destroy_workqueue(),
   0/10 dumps saved
 - patch 1 only: no change, the oops in (2) happens first
 - both patches: 10/10 dumps saved; with slub_debug=FZPU, no slub
   reports and no oopses

On 6.18.18, which predates commit 3d33de353b1f, patch 1 alone fixed the
slab corruption (10/10 dumps, previously 0x6b poison faults with
slub_debug).

This series is build-tested with W=1 on top of 7.3-rc4.

Jingyi Li (2):
  dmaengine: idxd: fix double free in idxd_free()
  dmaengine: idxd: fix destroy of NULL or already destroyed workqueue

 drivers/dma/idxd/init.c  | 5 ++---
 drivers/dma/idxd/sysfs.c | 3 ++-
 2 files changed, 4 insertions(+), 4 deletions(-)


base-commit: 93f51579e7df248780214094418f205253383cc5
-- 
2.43.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/2] dmaengine: idxd: fix double free in idxd_free()
  2026-10-28  0:37 [PATCH 0/2] dmaengine: idxd: fix crashes in the probe error path Jingyi Li
@ 2026-10-28  0:37 ` Jingyi Li
  2026-10-28 15:04   ` Frank Li
  2026-10-28  0:37 ` [PATCH 2/2] dmaengine: idxd: fix destroy of NULL or already destroyed workqueue Jingyi Li
  2026-10-28 20:24 ` [PATCH 0/2] dmaengine: idxd: fix crashes in the probe error path Jingyi Li
  2 siblings, 1 reply; 6+ messages in thread
From: Jingyi Li @ 2026-10-28  0:37 UTC (permalink / raw)
  To: vkoul, vinicius.gomes
  Cc: dave.jiang, xueshuai, fenghuay, Frank.Li, dmaengine,
	linux-kernel, stable, Jingyi Li

idxd_free() calls put_device() on the conf device, which drops the last
reference and runs idxd_conf_device_release(). The release callback
already frees idxd->opcap_bmap, the IDA id and idxd itself, so the
subsequent bitmap_free(), ida_free() and kfree() in idxd_free() operate
on freed memory.

idxd_free() is called from the error path of idxd_pci_probe_alloc()
when idxd_probe() fails, for example when the device reports HALTED
after reset. We hit this in a kdump crash kernel, where the DSA device
fails to reset: the double free corrupts the slab and the crash kernel
dies before the dump is saved. With slub_debug=FZPU it shows up as
general protection faults on 0x6b6b6b6b6b6b6b6b.

Let the release callback do all the freeing.

Fixes: 90022b3a6981 ("dmaengine: idxd: fix memory leak in error handling path of idxd_pci_probe")
Cc: stable@vger.kernel.org
Signed-off-by: Jingyi Li <jinli@purestorage.com>
---
 drivers/dma/idxd/init.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c
index 4b827a329756..1577d22b299c 100644
--- a/drivers/dma/idxd/init.c
+++ b/drivers/dma/idxd/init.c
@@ -581,9 +581,6 @@ static void idxd_free(struct idxd_device *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)
-- 
2.43.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 2/2] dmaengine: idxd: fix destroy of NULL or already destroyed workqueue
  2026-10-28  0:37 [PATCH 0/2] dmaengine: idxd: fix crashes in the probe error path Jingyi Li
  2026-10-28  0:37 ` [PATCH 1/2] dmaengine: idxd: fix double free in idxd_free() Jingyi Li
@ 2026-10-28  0:37 ` Jingyi Li
  2026-10-28 15:07   ` Frank Li
  2026-10-28 20:24 ` [PATCH 0/2] dmaengine: idxd: fix crashes in the probe error path Jingyi Li
  2 siblings, 1 reply; 6+ messages in thread
From: Jingyi Li @ 2026-10-28  0:37 UTC (permalink / raw)
  To: vkoul, vinicius.gomes
  Cc: dave.jiang, xueshuai, fenghuay, Frank.Li, dmaengine,
	linux-kernel, stable, Jingyi Li

idxd_conf_device_release() unconditionally calls
destroy_workqueue(idxd->wq). This is wrong in two cases:

 - If idxd_probe() fails before idxd_setup_internals() has created the
   workqueue, for example when the device reports HALTED after reset,
   idxd->wq is still NULL and the final put_device() in the probe error
   path oopses:

   BUG: kernel NULL pointer dereference, address: 00000000000000d0
   RIP: 0010:destroy_workqueue+0xe/0x300
   Call Trace:
    <TASK>
    idxd_conf_device_release+0x15/0x90 [idxd]
    device_release+0x3c/0x90
    kobject_put+0x7e/0x1e0
    idxd_pci_probe_alloc+0x300/0x15e0 [idxd]
    local_pci_probe+0x22/0x60
    pci_device_probe+0xd4/0x260

 - idxd_cleanup_internals() and the err_evl path of
   idxd_setup_internals() destroy the workqueue but leave idxd->wq
   pointing to it, so the release callback destroys it a second time.

Clear idxd->wq after the early destroys and skip the destroy in the
release callback when there is no workqueue.

We hit the NULL case in a kdump crash kernel, where the DSA device
fails to reset and the oops kills the crash kernel before the dump is
saved.

Fixes: 3d33de353b1f ("dmaengine: idxd: Fix not releasing workqueue on .release()")
Cc: stable@vger.kernel.org
Signed-off-by: Jingyi Li <jinli@purestorage.com>
---
 drivers/dma/idxd/init.c  | 2 ++
 drivers/dma/idxd/sysfs.c | 3 ++-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c
index 1577d22b299c..5962cacef060 100644
--- a/drivers/dma/idxd/init.c
+++ b/drivers/dma/idxd/init.c
@@ -389,6 +389,7 @@ static void idxd_cleanup_internals(struct idxd_device *idxd)
 	idxd_clean_engines(idxd);
 	idxd_clean_wqs(idxd);
 	destroy_workqueue(idxd->wq);
+	idxd->wq = NULL;
 }
 
 static int idxd_init_evl(struct idxd_device *idxd)
@@ -460,6 +461,7 @@ static int idxd_setup_internals(struct idxd_device *idxd)
 
  err_evl:
 	destroy_workqueue(idxd->wq);
+	idxd->wq = NULL;
  err_wkq_create:
 	idxd_clean_groups(idxd);
  err_group:
diff --git a/drivers/dma/idxd/sysfs.c b/drivers/dma/idxd/sysfs.c
index 6d251095c350..d5ffc641c856 100644
--- a/drivers/dma/idxd/sysfs.c
+++ b/drivers/dma/idxd/sysfs.c
@@ -1836,7 +1836,8 @@ static void idxd_conf_device_release(struct device *dev)
 {
 	struct idxd_device *idxd = confdev_to_idxd(dev);
 
-	destroy_workqueue(idxd->wq);
+	if (idxd->wq)
+		destroy_workqueue(idxd->wq);
 	kfree(idxd->groups);
 	bitmap_free(idxd->wq_enable_map);
 	kfree(idxd->wqs);
-- 
2.43.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] dmaengine: idxd: fix double free in idxd_free()
  2026-10-28  0:37 ` [PATCH 1/2] dmaengine: idxd: fix double free in idxd_free() Jingyi Li
@ 2026-10-28 15:04   ` Frank Li
  0 siblings, 0 replies; 6+ messages in thread
From: Frank Li @ 2026-10-28 15:04 UTC (permalink / raw)
  To: Jingyi Li
  Cc: vkoul, vinicius.gomes, dave.jiang, xueshuai, fenghuay, Frank.Li,
	dmaengine, linux-kernel, stable

On Wed, Oct 28, 2026 at 12:37:06AM +0000, Jingyi Li wrote:
> [You don't often get email from jinli@purestorage.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> idxd_free() calls put_device() on the conf device, which drops the last
> reference and runs idxd_conf_device_release(). The release callback
> already frees idxd->opcap_bmap, the IDA id and idxd itself, so the
> subsequent bitmap_free(), ida_free() and kfree() in idxd_free() operate
> on freed memory.
>
> idxd_free() is called from the error path of idxd_pci_probe_alloc()
> when idxd_probe() fails, for example when the device reports HALTED
> after reset. We hit this in a kdump crash kernel, where the DSA device
> fails to reset: the double free corrupts the slab and the crash kernel
> dies before the dump is saved. With slub_debug=FZPU it shows up as
> general protection faults on 0x6b6b6b6b6b6b6b6b.
>
> Let the release callback do all the freeing.
>
> Fixes: 90022b3a6981 ("dmaengine: idxd: fix memory leak in error handling path of idxd_pci_probe")
> Cc: stable@vger.kernel.org
> Signed-off-by: Jingyi Li <jinli@purestorage.com>
> ---

Similar fixes at
https://patchwork.kernel.org/project/linux-dmaengine/patch/20261012-wt-idxd-fixes-v1-2-30a35a445077@intel.com/

Frank
>  drivers/dma/idxd/init.c | 3 ---
>  1 file changed, 3 deletions(-)
>
> diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c
> index 4b827a329756..1577d22b299c 100644
> --- a/drivers/dma/idxd/init.c
> +++ b/drivers/dma/idxd/init.c
> @@ -581,9 +581,6 @@ static void idxd_free(struct idxd_device *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)
> --
> 2.43.0
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] dmaengine: idxd: fix destroy of NULL or already destroyed workqueue
  2026-10-28  0:37 ` [PATCH 2/2] dmaengine: idxd: fix destroy of NULL or already destroyed workqueue Jingyi Li
@ 2026-10-28 15:07   ` Frank Li
  0 siblings, 0 replies; 6+ messages in thread
From: Frank Li @ 2026-10-28 15:07 UTC (permalink / raw)
  To: Jingyi Li
  Cc: vkoul, vinicius.gomes, dave.jiang, xueshuai, fenghuay, Frank.Li,
	dmaengine, linux-kernel, stable

On Wed, Oct 28, 2026 at 12:37:07AM +0000, Jingyi Li wrote:
> [You don't often get email from jinli@purestorage.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> idxd_conf_device_release() unconditionally calls
> destroy_workqueue(idxd->wq). This is wrong in two cases:
>
>  - If idxd_probe() fails before idxd_setup_internals() has created the
>    workqueue, for example when the device reports HALTED after reset,
>    idxd->wq is still NULL and the final put_device() in the probe error
>    path oopses:
>
>    BUG: kernel NULL pointer dereference, address: 00000000000000d0
>    RIP: 0010:destroy_workqueue+0xe/0x300
>    Call Trace:
>     <TASK>
>     idxd_conf_device_release+0x15/0x90 [idxd]
>     device_release+0x3c/0x90
>     kobject_put+0x7e/0x1e0
>     idxd_pci_probe_alloc+0x300/0x15e0 [idxd]
>     local_pci_probe+0x22/0x60
>     pci_device_probe+0xd4/0x260
>
>  - idxd_cleanup_internals() and the err_evl path of
>    idxd_setup_internals() destroy the workqueue but leave idxd->wq
>    pointing to it, so the release callback destroys it a second time.
>
> Clear idxd->wq after the early destroys and skip the destroy in the
> release callback when there is no workqueue.
>
> We hit the NULL case in a kdump crash kernel, where the DSA device
> fails to reset and the oops kills the crash kernel before the dump is
> saved.
>
> Fixes: 3d33de353b1f ("dmaengine: idxd: Fix not releasing workqueue on .release()")
> Cc: stable@vger.kernel.org
> Signed-off-by: Jingyi Li <jinli@purestorage.com>
> ---

This one better than
https://lore.kernel.org/dmaengine/20261012-wt-idxd-fixes-v1-1-30a35a445077@intel.com/

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>  drivers/dma/idxd/init.c  | 2 ++
>  drivers/dma/idxd/sysfs.c | 3 ++-
>  2 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c
> index 1577d22b299c..5962cacef060 100644
> --- a/drivers/dma/idxd/init.c
> +++ b/drivers/dma/idxd/init.c
> @@ -389,6 +389,7 @@ static void idxd_cleanup_internals(struct idxd_device *idxd)
>         idxd_clean_engines(idxd);
>         idxd_clean_wqs(idxd);
>         destroy_workqueue(idxd->wq);
> +       idxd->wq = NULL;
>  }
>
>  static int idxd_init_evl(struct idxd_device *idxd)
> @@ -460,6 +461,7 @@ static int idxd_setup_internals(struct idxd_device *idxd)
>
>   err_evl:
>         destroy_workqueue(idxd->wq);
> +       idxd->wq = NULL;
>   err_wkq_create:
>         idxd_clean_groups(idxd);
>   err_group:
> diff --git a/drivers/dma/idxd/sysfs.c b/drivers/dma/idxd/sysfs.c
> index 6d251095c350..d5ffc641c856 100644
> --- a/drivers/dma/idxd/sysfs.c
> +++ b/drivers/dma/idxd/sysfs.c
> @@ -1836,7 +1836,8 @@ static void idxd_conf_device_release(struct device *dev)
>  {
>         struct idxd_device *idxd = confdev_to_idxd(dev);
>
> -       destroy_workqueue(idxd->wq);
> +       if (idxd->wq)
> +               destroy_workqueue(idxd->wq);
>         kfree(idxd->groups);
>         bitmap_free(idxd->wq_enable_map);
>         kfree(idxd->wqs);
> --
> 2.43.0
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 0/2] dmaengine: idxd: fix crashes in the probe error path
  2026-10-28  0:37 [PATCH 0/2] dmaengine: idxd: fix crashes in the probe error path Jingyi Li
  2026-10-28  0:37 ` [PATCH 1/2] dmaengine: idxd: fix double free in idxd_free() Jingyi Li
  2026-10-28  0:37 ` [PATCH 2/2] dmaengine: idxd: fix destroy of NULL or already destroyed workqueue Jingyi Li
@ 2026-10-28 20:24 ` Jingyi Li
  2 siblings, 0 replies; 6+ messages in thread
From: Jingyi Li @ 2026-10-28 20:24 UTC (permalink / raw)
  To: vkoul, vinicius.gomes
  Cc: Frank.Li, dave.jiang, xueshuai, fenghuay, dmaengine, linux-kernel

Hi Frank, Vinod,

Thanks Frank for the review and for pointing me to Vinicius's series
"dmaengine: idxd: More error handling fixes" [1]. Patches 1-3 of that
series cover both patches here:

- 1/2 (double free in idxd_free()) is covered by their 2/6, which
  makes the same change to idxd_free().

- 2/2 (destroy of a NULL or already destroyed workqueue) is covered
  by their 1/6 and 3/6 together. 1/6 adds the same NULL check in
  idxd_conf_device_release(), for a probe failure before idxd->wq is
  allocated. 3/6 removes the early destroy_workqueue() calls, so the
  release callback is the only place the workqueue is destroyed.

So please drop this series in favor of theirs. The one caveat is that
1/6 alone only covers the "never allocated" case; without 3/6, a
failure after idxd_setup_internals() still destroys the workqueue
twice. If 1/6 and 3/6 don't both go in (including to stable), 2/2
here would still be needed.

I've replied on their series with test results.

[1] https://lore.kernel.org/dmaengine/20261012-wt-idxd-fixes-v1-0-30a35a445077@intel.com/

Thanks,
Jingyi

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-10-28 20:24 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-28  0:37 [PATCH 0/2] dmaengine: idxd: fix crashes in the probe error path Jingyi Li
2026-10-28  0:37 ` [PATCH 1/2] dmaengine: idxd: fix double free in idxd_free() Jingyi Li
2026-10-28 15:04   ` Frank Li
2026-10-28  0:37 ` [PATCH 2/2] dmaengine: idxd: fix destroy of NULL or already destroyed workqueue Jingyi Li
2026-10-28 15:07   ` Frank Li
2026-10-28 20:24 ` [PATCH 0/2] dmaengine: idxd: fix crashes in the probe error path Jingyi Li

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®