Re: [PATCH v2] dmaengine: Fix refcount leak in channel register error path

[Guangshuo Li]

Hi Frank,
Thanks for reviewing.

On Mon, 20 Apr 2026 at 14:23, Frank Li <Frank.li@xxxxxxx> 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