After device_register(), the lifetime of the embedded struct device is
expected to be managed through the device core reference counting.

In __dma_async_device_channel_register(), if device_register() fails,
the error path frees chan->dev directly instead of releasing the device
reference with put_device(). This bypasses the normal device lifetime
rules and may leave the reference count of the embedded struct device
unbalanced, resulting in a refcount leak.

The issue was identified by a static analysis tool I developed and
confirmed by manual review.

Fix this by using put_device() in the device_register() failure path and
let chan_dev_release() handle the final cleanup.

Fixes: d2fb0a043838 ("dmaengine: break out channel registration")
Cc: stable@vger.kernel.org
Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
---
v2:
  - note that the issue was identified by my static analysis tool
  - and confirmed by manual review

 drivers/dma/dmaengine.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
index ca13cd39330b..6bb1212ae0e1 100644
--- a/drivers/dma/dmaengine.c
+++ b/drivers/dma/dmaengine.c
@@ -1111,8 +1111,12 @@ static int __dma_async_device_channel_register(struct dma_device *device,
 
  err_out_ida:
 	ida_free(&device->chan_ida, chan->chan_id);
+	put_device(&chan->dev->device);
+	chan->dev = NULL;
+	goto err_free_local;
  err_free_dev:
 	kfree(chan->dev);
+	chan->dev = NULL;
  err_free_local:
 	free_percpu(chan->local);
 	chan->local = NULL;
-- 
2.43.0

On Mon, Apr 13, 2026 at 09:58:57PM +0800, Guangshuo Li wrote:
> After device_register(), the lifetime of the embedded struct device is
> expected to be managed through the device core reference counting.
>
> In __dma_async_device_channel_register(), if device_register() fails,
> the error path frees chan->dev directly instead of releasing the device
> reference with put_device(). This bypasses the normal device lifetime
> rules and may leave the reference count of the embedded struct device
> unbalanced, resulting in a refcount leak.
>
> The issue was identified by a static analysis tool I developed and
> confirmed by manual review.

I think it is meanless, no one reproduce this. Provide tools link if open
source. Or you descript how problem happen.

> diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
> index ca13cd39330b..6bb1212ae0e1 100644
> --- a/drivers/dma/dmaengine.c
> +++ b/drivers/dma/dmaengine.c
> @@ -1111,8 +1111,12 @@ static int __dma_async_device_channel_register(struct dma_device *device,
>
>   err_out_ida:
>  	ida_free(&device->chan_ida, chan->chan_id);
> +	put_device(&chan->dev->device);
> +	chan->dev = NULL;
> +	goto err_free_local;

avoid err path goto again

Frank

>   err_free_dev:
>  	kfree(chan->dev);
> +	chan->dev = NULL;
>   err_free_local:
>  	free_percpu(chan->local);
>  	chan->local = NULL;
> --
> 2.43.0
>

Hi Frank,
Thanks for reviewing.

On Mon, 20 Apr 2026 at 14:23, Frank Li <Frank.li@nxp.com> wrote:
>
>
> I think it is meanless, no one reproduce this. Provide tools link if open
> source. Or you descript how problem happen.
>
The issue was initially reported by a static analysis tool I am developing.
It is still under development and is not open source at this moment.

I also manually reviewed the code path. The problem happens because
device_register() is implemented as:

int device_register(struct device *dev)
{
device_initialize(dev);
return device_add(dev);
}

That means even if device_register() fails, it has already called
device_initialize() and initialized the device reference count to 1.

Also, the comment for device_register() explicitly says:

NOTE: _Never_ directly free @dev after calling this function, even
if it returned an error! Always use put_device() to give up the
reference initialized in this function instead.

In the current code, if device_register(&chan->dev->device) fails, the
error path falls through to:

kfree(chan->dev);

This bypasses the reference-count-based device release path and can lead to
a refcount leak.


> >   err_out_ida:
> >       ida_free(&device->chan_ida, chan->chan_id);
> > +     put_device(&chan->dev->device);
> > +     chan->dev = NULL;
> > +     goto err_free_local;
>
> avoid err path goto again
>
> >
Thanks for pointing this out. How about this:

err_out_ida:
  ida_free(&device->chan_ida, chan->chan_id);
+ put_device(&chan->dev->device);
+ chan->dev = NULL;
+ free_percpu(chan->local);
+ chan->local = NULL;
+ return rc;
+
 err_free_dev:
  kfree(chan->dev);
 err_free_local:
  free_percpu(chan->local);
  chan->local = NULL;
  return rc;

This way, put_device() is only used for the post-device_register()
failure path, while kfree(chan->dev) remains for the earlier failure
path, and the extra goto can be avoided as well.

Thanks,
Guangshuo