[PATCH 0/2] dmaengine: idxd: fix crashes in the probe error path

[PATCH 0/2] dmaengine: idxd: fix crashes in the probe error path

[Jingyi Li]

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

[PATCH 1/2] dmaengine: idxd: fix double free in idxd_free()

[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

[PATCH 2/2] dmaengine: idxd: fix destroy of NULL or already destroyed workqueue

[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