Re: [PATCH] dmaengine: idxd: fix use-after-free in idxd_free() and idxd_alloc() error paths

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] Unconditional call to `destroy_workqueue(idxd->wq)` in `idxd_conf_device_release()` causes a guaranteed kernel panic on early probe or allocation failures.
- [High] Late probe failures trigger sequential manual cleanup (`idxd_cleanup_internals`) and `idxd_conf_device_release()`, resulting in massive double-frees and a double-destroy of the workqueue.
- [High] Sub-component configuration and cleanup functions (`idxd_clean_wqs`, `idxd_clean_groups`, `idxd_clean_engines`) suffer from the exact same double-free anti-pattern that the patch intended to fix in the main device.
--

commit 14c25ba1a63a6d7ab71af3fd079660a9cd76f396
Author: Bogdan Codres <bogdan.codres@windriver.com>

dmaengine: idxd: fix use-after-free in idxd_free() and idxd_alloc() error paths

This commit removes redundant cleanup calls in the error paths of idxd_alloc
and idxd_free. By relying on put_device to synchronously trigger the release
handler idxd_conf_device_release, which already frees these resources, it
prevents double-free scenarios during initialization failures.

> diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c
> index f1cfc7790d950..e2b5fe072d4ac 100644
> --- a/drivers/dma/idxd/init.c
> +++ b/drivers/dma/idxd/init.c
[ ... ]
> @@ -601,15 +601,18 @@ static void idxd_read_caps(struct idxd_device *idxd)
[ ... ]
> +/*
> + * Release an idxd device that was allocated (device_initialize() was called)
> + * but never successfully registered. put_device() drops the last reference and
> + * triggers idxd_conf_device_release() which frees all resources including the
> + * ida, opcap_bmap, and the idxd structure itself.
> + */
>  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);
>  }

[Severity: High]
This is a pre-existing issue, but does this same double-free pattern still
exist for late probe failures?

If idxd_probe() fails late in the initialization sequence, the error path
calls idxd_cleanup(), which delegates to idxd_cleanup_internals().

In drivers/dma/idxd/init.c:idxd_cleanup_internals():
    ...
    destroy_workqueue(idxd->wq);

The error path then falls through to idxd_free(), which calls put_device().
This triggers idxd_conf_device_release() in drivers/dma/idxd/sysfs.c, which
blindly executes destroy_workqueue(idxd->wq) and kfree() on internal arrays
a second time. Can this cause a double-destroy and memory corruption?

[Severity: High]
This isn't a bug introduced by this patch, but do the sub-component cleanup
functions suffer from the same double-free pattern?

Looking at idxd_clean_wqs() in drivers/dma/idxd/init.c:
    ...
    put_device(conf_dev);
    kfree(wq);

The put_device() call drops the reference to zero and invokes the release
handler idxd_conf_wq_release(), which already performs kfree(wq). Does
calling kfree(wq) immediately after put_device() result in a double-free?
Similar patterns appear to exist in idxd_clean_groups() and
idxd_clean_engines().

[ ... ]
> @@ -649,13 +652,16 @@ static struct idxd_device *idxd_alloc(struct pci_dev *pdev, struct idxd_driver_d
>  	return idxd;
>  
>  err_name:
> +	/* device_initialize() was called, so put_device() will trigger
> +	 * idxd_conf_device_release() which frees ida, opcap_bmap, and idxd.
> +	 * Do not fall through to err_opcap/err_ida.
> +	 */
>  	put_device(conf_dev);
> -	bitmap_free(idxd->opcap_bmap);
> +	return NULL;

[Severity: High]
This is a pre-existing issue, but will this cause a NULL pointer dereference
during early failures?

If dev_set_name() fails, idxd->wq has not been allocated yet and remains NULL.
The error path calls put_device(conf_dev) which invokes the release callback:

drivers/dma/idxd/sysfs.c:idxd_conf_device_release() {
    ...
    destroy_workqueue(idxd->wq);

Since the upstream kernel's destroy_workqueue() does not safely handle NULL
pointers, won't this unconditionally crash the system on early probe or
allocation failures?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260615103932.61828-2-bogdan.codres@windriver.com?part=1