Make sure to drop the reference taken when looking up the DMA platform
device during of_dma_xlate() when releasing channel resources.

Note that commit 3832b78b3ec2 ("dmaengine: at_hdmac: add missing
put_device() call in at_dma_xlate()") fixed the leak in a couple of
error paths but the reference is still leaking on successful allocation.

Fixes: bbe89c8e3d59 ("at_hdmac: move to generic DMA binding")
Fixes: 3832b78b3ec2 ("dmaengine: at_hdmac: add missing put_device() call in at_dma_xlate()")
Cc: stable@vger.kernel.org	# 3.10: 3832b78b3ec2
Cc: Yu Kuai <yukuai3@huawei.com>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/dma/at_hdmac.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c
index 2d147712cbc6..dffe5becd6c3 100644
--- a/drivers/dma/at_hdmac.c
+++ b/drivers/dma/at_hdmac.c
@@ -1765,6 +1765,7 @@ static int atc_alloc_chan_resources(struct dma_chan *chan)
 static void atc_free_chan_resources(struct dma_chan *chan)
 {
 	struct at_dma_chan	*atchan = to_at_dma_chan(chan);
+	struct at_dma_slave	*atslave;
 
 	BUG_ON(atc_chan_is_enabled(atchan));
 
@@ -1774,8 +1775,12 @@ static void atc_free_chan_resources(struct dma_chan *chan)
 	/*
 	 * Free atslave allocated in at_dma_xlate()
 	 */
-	kfree(chan->private);
-	chan->private = NULL;
+	atslave = chan->private;
+	if (atslave) {
+		put_device(atslave->dma_dev);
+		kfree(atslave);
+		chan->private = NULL;
+	}
 
 	dev_vdbg(chan2dev(chan), "free_chan_resources: done\n");
 }
-- 
2.51.0

On Mon, Nov 17, 2025 at 05:12:43PM +0100, Johan Hovold wrote:
> Make sure to drop the reference taken when looking up the DMA platform
> device during of_dma_xlate() when releasing channel resources.
> 
> Note that commit 3832b78b3ec2 ("dmaengine: at_hdmac: add missing
> put_device() call in at_dma_xlate()") fixed the leak in a couple of
> error paths but the reference is still leaking on successful allocation.

...

> -	kfree(chan->private);
> -	chan->private = NULL;
> +	atslave = chan->private;
> +	if (atslave) {
> +		put_device(atslave->dma_dev);
> +		kfree(atslave);
> +		chan->private = NULL;
> +	}

It can also be

	atslave = chan->private;
	if (atslave)
		put_device(atslave->dma_dev);
	kfree(chan->private);
	chan->private = NULL;

which makes patch shorter.

In any case I'm wondering if the asynchronous nature of put_device() can
collide with chan->private = NULL assignment. I think as long as it's not
used inside ->release() of the device, we are fine.

-- 
With Best Regards,
Andy Shevchenko

On Mon, Nov 17, 2025 at 06:08:51PM +0100, Andy Shevchenko wrote:
> On Mon, Nov 17, 2025 at 05:12:43PM +0100, Johan Hovold wrote:
> > Make sure to drop the reference taken when looking up the DMA platform
> > device during of_dma_xlate() when releasing channel resources.
> > 
> > Note that commit 3832b78b3ec2 ("dmaengine: at_hdmac: add missing
> > put_device() call in at_dma_xlate()") fixed the leak in a couple of
> > error paths but the reference is still leaking on successful allocation.
> 
> ...
> 
> > -	kfree(chan->private);
> > -	chan->private = NULL;
> > +	atslave = chan->private;
> > +	if (atslave) {
> > +		put_device(atslave->dma_dev);
> > +		kfree(atslave);
> > +		chan->private = NULL;
> > +	}
> 
> It can also be
> 
> 	atslave = chan->private;
> 	if (atslave)
> 		put_device(atslave->dma_dev);
> 	kfree(chan->private);
> 	chan->private = NULL;
> 
> which makes patch shorter.

Perhaps, but it would be less readable.

> In any case I'm wondering if the asynchronous nature of put_device() can
> collide with chan->private = NULL assignment. I think as long as it's not
> used inside ->release() of the device, we are fine.

It's just another reference to the dma device which is not going away
while it's channels are being tore down (or we would have bigger
problems).

Johan