Re: [PATCH v2] 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 2 potential issue(s) to consider:

Pre-existing issues:
- [Critical] Widespread double-free vulnerabilities and use-after-free of internal arrays and workqueues on probe failure paths.
- [High] Kernel panic via `destroy_workqueue(NULL)` in `idxd_conf_device_release()` during early allocation failures.
--

commit 5374b010ac47ca9ae57388435db01f775b2c068e
Author: Bogdan Codres <bogdan.codres@windriver.com>

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

This patch fixes a use-after-free bug in the error paths of idxd_free() and
idxd_alloc(). It removes duplicate frees by letting put_device() handle all
resource cleanup via the release callback.

> diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c
> index f1cfc7790d950..def37db80d5b8 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));

[Severity: Critical]
This is a pre-existing issue, but does centralizing the cleanup around
put_device() here expose a double-free during probe failures?

If idxd_probe() fails after partial initialization (like a failure in
idxd_device_load_config()), it explicitly calls idxd_cleanup_internals(idxd)
which frees several pointers and destroys the workqueue:

idxd_cleanup_internals() {
    ...
    destroy_workqueue(idxd->wq);
}

However, it doesn't set these pointers to NULL. When the error propagates
and eventually calls idxd_free(), put_device() triggers
idxd_conf_device_release() which attempts to free those same pointers again:

idxd_conf_device_release() {
    ...
    destroy_workqueue(idxd->wq);
    kfree(idxd->groups);
    kfree(idxd->wqs);
    kfree(idxd->engines);
}

Could this result in slab corruption and a double-destroy of the workqueue?

Also, sub-cleanup functions like idxd_clean_wqs() exhibit a similar pattern
where put_device() triggers idxd_conf_wq_release() (freeing the wq, wqcfg,
and opcap_bmap), but the cleanup loop immediately calls kfree(wq) and
bitmap_free() again.

> -	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)

[ ... ]

> @@ -649,8 +652,12 @@ 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);

[Severity: High]
This isn't a bug introduced by this patch, but will this put_device() call
trigger a kernel panic if early allocation fails?

If idxd_alloc() jumps to the err_name label (for instance, if
dev_set_name() fails), idxd->wq has not been allocated yet. Because
device_initialize() was called, put_device() synchronously invokes the
release callback:

idxd_conf_device_release() {
    ...
    destroy_workqueue(idxd->wq);
}

Since destroy_workqueue() does not check for NULL before dereferencing its
argument, does this result in a deterministic panic when idxd->wq is NULL?

> -	bitmap_free(idxd->opcap_bmap);
> +	return NULL;
>  err_opcap:
>  	ida_free(&idxd_ida, idxd->id);
>  err_ida:

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