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

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 [off-list ref]
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.

[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