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

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

[Yuho Choi]

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@gmail.com>
---
 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);
 	put_device(fdev);
+	return rc;
 failed_ida:
 failed_set_pasid:
 	if (device_user_pasid_enabled(idxd))
-- 
2.50.1 (Apple Git-155)

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

[Frank Li]

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@gmail.com>
> ---
>  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)
>

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@nxp.com> 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@gmail.com>
> > ---
> >  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)
> >