[PATCH] dmaengine: idxd: Free the minor number when cdev setup fails

[PATCH] dmaengine: idxd: Free the minor number when cdev setup fails

[SeungUk Nam]

idxd_wq_add_cdev() allocates a minor number from cdev_ctx->minor_ida for
the new struct idxd_cdev. When dev_set_name() or cdev_device_add()
fails, the error path calls put_device(), and the release callback
idxd_cdev_dev_release() only frees idxd_cdev, so the minor number is
never returned to the ida. Leaked numbers accumulate across bind/unbind
cycles, up to MINORMASK, for as long as the module stays loaded.

Free the minor number before put_device() drops the last reference on
that path.

Fixes: c311f5e92484 ("dmaengine: idxd: Fix freeing the allocated ida too late")
Signed-off-by: SeungUk Nam <tmddnr49449@gmail.com>
---

Found by code inspection; the error path was not exercised at runtime.

 drivers/dma/idxd/cdev.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/dma/idxd/cdev.c b/drivers/dma/idxd/cdev.c
index 82b07cf942ef..f1af7f0d25d3 100644
--- a/drivers/dma/idxd/cdev.c
+++ b/drivers/dma/idxd/cdev.c
@@ -573,6 +573,7 @@ int idxd_wq_add_cdev(struct idxd_wq *wq)
 	return 0;
 
  err:
+	ida_free(&cdev_ctx->minor_ida, minor);
 	put_device(dev);
 	wq->idxd_cdev = NULL;
 	return rc;
-- 
2.34.1

Re: [PATCH] dmaengine: idxd: Free the minor number when cdev setup fails

[sashiko-bot]

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

Pre-existing issues:
- [High] Preexisting Use-After-Free of the `wq` structure when the device is unbound while userspace holds the cdev open.
- [High] Concurrent open() during device teardown or initialization failure causes a NULL pointer dereference (TOCTOU).
--

commit dcf7a1148089e42b5600f7c8fd58bdaab680d5cb
Author: SeungUk Nam <tmddnr49449@gmail.com>
Subject: dmaengine: idxd: Free the minor number when cdev setup fails

idxd_wq_add_cdev() allocates a minor number from cdev_ctx->minor_ida for
the new struct idxd_cdev. When dev_set_name() or cdev_device_add() fails,
the minor number is never returned to the ida. This patch frees the minor
number before put_device() drops the last reference on that path.

> diff --git a/drivers/dma/idxd/cdev.c b/drivers/dma/idxd/cdev.c
> index 82b07cf942ef8..f1af7f0d25d34 100644
> --- a/drivers/dma/idxd/cdev.c
> +++ b/drivers/dma/idxd/cdev.c
> @@ -573,6 +573,7 @@ int idxd_wq_add_cdev(struct idxd_wq *wq)
>  	return 0;
>  
>   err:
> +	ida_free(&cdev_ctx->minor_ida, minor);
>  	put_device(dev);
>  	wq->idxd_cdev = NULL;

[Severity: High]
This is a pre-existing issue, but can this lead to a NULL pointer dereference
during a concurrent open()?

A TOCTOU race condition exists between idxd_cdev_open() and device teardown or
initialization failure. When cdev_device_add() successfully exposes the cdev
but then encounters an error, a racing open() call gets a reference to the
cdev and calls idxd_cdev_open(). The open() thread sleeps on
mutex_lock(&wq->wq_lock) because the initialization failure thread holds it.

The initialization failure thread then sets wq->idxd_cdev = NULL here and
drops the mutex. The open() thread wakes up, fetches the NULL idxd_cdev, and
passes it to the cdev_dev() macro:

idxd_cdev_open() {
    ...
    idxd_cdev = wq->idxd_cdev;
    ...
    fdev->parent = cdev_dev(idxd_cdev);
    ...
    rc = device_add(fdev);

This invalid pointer is assigned to fdev->parent, which is subsequently
dereferenced in device_add() via get_device(fdev->parent).


[Severity: High]
This is also a pre-existing issue, but does this missing reference management
cause a Use-After-Free of the wq structure?

When userspace opens the cdev, idxd_cdev_open() fetches the wq and increments
its client_count via idxd_wq_get(wq), but fails to take a kobject reference on
the device (e.g., get_device(&wq->idxd_dev.conf_dev)).

Later, if the device is unbound (e.g., via sysfs or hot-unplug), the device is
unregistered and the wq structure is freed. Since the cdev file descriptor is
still open, when userspace eventually closes it, idxd_cdev_release() blindly
dereferences the freed wq pointer:

idxd_cdev_release() {
    ...
    struct idxd_wq *wq = ctx->wq;
    struct idxd_device *idxd = wq->idxd;
    struct device *dev = &idxd->pdev->dev;

>  	return rc;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260910074832.15729-1-tmddnr49449@gmail.com?part=1