Re: [PATCH v1] dmaengine: idxd: fix deadlock and double free in idxd_cdev_open()

[최유호]

Dear Frank,

Thanks. I can rework this in v2 to use auto cleanup for fdev instead
of explicitly calling
put_device() on the error path.

I plan to keep the change narrow and limit it to the fdev lifetime.
The idea is to return directly
from the failed_dev_add/failed_dev_name path after unlocking
wq->wq_lock, so that the
auto cleanup runs only after the mutex has been released and it won't
fall through into
the later ctx cleanup path.

```
static int idxd_cdev_open(...)
{
    struct device *dev, *fdev __free(put_device) = NULL;
    ...
    fdev = user_ctx_dev(ctx);
    ...
    rc = dev_set_name(fdev, "file%d", ctx->id);
    if (rc < 0) {
        dev_warn(dev, "set name failure\n");
        goto failed_dev_name;
    }

    rc = device_add(fdev);
    if (rc < 0) {
        dev_warn(dev, "file device add failure\n");
        goto failed_dev_add;
    }

    idxd_wq_get(wq);
    fdev = NULL;
    mutex_unlock(&wq->wq_lock);
    return 0;

failed_dev_add:
failed_dev_name:
    mutex_unlock(&wq->wq_lock);
    return rc;
...
```

If you have a specific auto-cleanup pattern in mind, please let me
know and I can follow
that in v2.

Best regards,
Yuho Choi

On Mon, 20 Apr 2026 at 02:02, Frank Li <Frank.li@xxxxxxx> wrote:
>
> On Thu, Apr 16, 2026 at 06:19:57PM -0400, Yuho Choi wrote:
> > The failed_dev_add and failed_dev_name error paths in idxd_cdev_open()
> > call put_device(fdev) while still holding wq->wq_lock. This triggers
> > idxd_file_dev_release() synchronously, which calls
> > mutex_lock(&wq->wq_lock) — deadlocking on the same mutex.
> >
> > Additionally, the original code fell through from failed_dev_add and
> > failed_dev_name to the failed: label, which called kfree(ctx) a second
> > time after idxd_file_dev_release() had already freed it. The subsequent
> > idxd_xa_pasid_remove(ctx) then uses the freed pointer.
> >
> > Fix both issues by releasing wq_lock before put_device(fdev) and
> > returning immediately, so the release callback acquires the lock without
> > contention and no further cleanup is attempted on the freed context.
> >
> > Fixes: e6fd6d7e5f0fe ("dmaengine: idxd: add a device to represent the file opened")
> > Signed-off-by: Yuho Choi <dbgh9129@xxxxxxxxx>
> > ---
> >  drivers/dma/idxd/cdev.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/dma/idxd/cdev.c b/drivers/dma/idxd/cdev.c
> > index 0366c7cf35020..19a449333782b 100644
> > --- a/drivers/dma/idxd/cdev.c
> > +++ b/drivers/dma/idxd/cdev.c
> > @@ -307,7 +307,9 @@ static int idxd_cdev_open(struct inode *inode, struct file *filp)
> >
> >  failed_dev_add:
> >  failed_dev_name:
> > +     mutex_unlock(&wq->wq_lock);
>
> Can you use auto cleanup to fix this problem?
>
> Frank
>
> >       put_device(fdev);
> > +     return rc;
> >  failed_ida:
> >  failed_set_pasid:
> >       if (device_user_pasid_enabled(idxd))
> > --
> > 2.50.1 (Apple Git-155)
> >