dmaengine: at_hdmac: Fix memory leak

PATCH In review Pavel Machek 4 participants 7 messages Sep 20, 2020 – Jan 6, 2021

Subsystems (2)

Stable branch dmaengine

lore

Share & tools

lore

Pavel Machek

author

2020-09-20 08:28 UTC #0 lore raw

PATCH 1/1 dmaengine: at_hdmac: Fix memory leak

This fixes memory leak in at_hdmac. Mainline does not have the same problem.

Signed-off-by Pavel Machek (CIP) <[email protected]>

drivers/dma/at_hdmac.c +3 -1

index 86427f6ba78c..0847b2055857 100644
@@ -1714,8 +1714,10 @@ static struct dma_chan *at_dma_xlate(struct of_phandle_args *dma_spec,
 	atslave->dma_dev = &dmac_pdev->dev;
 
 	chan = dma_request_channel(mask, at_dma_filter, atslave);
-	if (!chan)
+	if (!chan) {
+		kfree(atslave);
 		return NULL;
+	}
 
 	atchan = to_at_dma_chan(chan);
 	atchan->per_if = dma_spec->args[0] & 0xff;

Signature · 2 lines

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Ludovic Desroches

Acked-by on v1 1/1

2020-09-23 06:39 UTC #1 lore raw

Quoted 9 lines · Pavel Machek

Date: Sun, 20 Sep 2020 10:28:38 +0200
From: Pavel Machek <[email protected]>
To: [email protected], [email protected],
[email protected], [email protected], [email protected], [email protected], [email protected], Greg KH <[email protected]>
Subject: [PATCH 4.19] dmaengine: at_hdmac: Fix memory leak

Quoted 4 lines

This fixes memory leak in at_hdmac. Mainline does not have the same problem.
Signed-off-by Pavel Machek (CIP) <[email protected]>

Acked-by Ludovic Desroches <[email protected]>

Thanks.

Quoted 21 lines

drivers/dma/at_hdmac.c +3 -1

index 86427f6ba78c..0847b2055857 100644
@@ -1714,8 +1714,10 @@ static struct dma_chan *at_dma_xlate(struct of_phandle_args *dma_spec,
 	atslave->dma_dev = &dmac_pdev->dev;
 
 	chan = dma_request_channel(mask, at_dma_filter, atslave);
-	if (!chan)
+	if (!chan) {
+		kfree(atslave);
 		return NULL;
+	}
 
 	atchan = to_at_dma_chan(chan);
 	atchan->per_if = dma_spec->args[0] & 0xff;

Signature · 3 lines

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[email protected]

2020-09-23 08:13 UTC #2 lore raw

In reply to Pavel Machek

Hi, Pavel,

Quoted 12 lines · Pavel Machek

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
This fixes memory leak in at_hdmac. Mainline does not have the same problem.
Signed-off-by Pavel Machek (CIP) <[email protected]>
drivers/dma/at_hdmac.c +0 -0
index 86427f6ba78c..0847b2055857 100644

drivers/dma/at_hdmac.c static struct dma_chan *at_dma_xlate(struct of_phandle_args *dma_spec, · line 1720

Quoted 8 lines

drivers/dma/at_hdmac.c +3 -1

@@ -1714,8 +1714,10 @@ static struct dma_chan *at_dma_xlate(struct of_phandle_args *dma_spec,
        atslave->dma_dev = &dmac_pdev->dev;
 
        chan = dma_request_channel(mask, at_dma_filter, atslave);
-       if (!chan)
+       if (!chan) {
+               kfree(atslave);
                return NULL;
+       }

Thanks for submitting this to stable. While the fix is good, you can instead cherry-pick the commit that hit upstream. In order to do that cleanly on top of v4.19.145, you have to pick two other fixes:

commit a6e7f19c9100 ("dmaengine: at_hdmac: Substitute kzalloc with kmalloc") commit 3832b78b3ec2 ("dmaengine: at_hdmac: add missing put_device() call in at_dma_xlate()") commit a6e7f19c9100 ("dmaengine: at_hdmac: Substitute kzalloc with kmalloc")

There are also some locking/deadlock fixes in mainline for this driver, depending on the time you can allocate for this, the list of patches can increase. I should have Cc'ed [email protected] in the first place, my bad.

Also it may worth to read the rules for submitting to stable at:
https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html

Cheers,
ta

[email protected]

2020-09-23 08:19 UTC #3 lore raw

Quoted 28 lines · On 9/23/20 11:13 AM, [email protected] wrote:

Hi, Pavel,
Quoted 12 lines · Pavel Machek
EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
This fixes memory leak in at_hdmac. Mainline does not have the same problem.
Signed-off-by Pavel Machek (CIP) <[email protected]>
drivers/dma/at_hdmac.c +0 -0
index 86427f6ba78c..0847b2055857 100644
drivers/dma/at_hdmac.c static struct dma_chan *at_dma_xlate(struct of_phandle_args *dma_spec, · line 1720
Quoted 8 lines
drivers/dma/at_hdmac.c +3 -1
@@ -1714,8 +1714,10 @@ static struct dma_chan *at_dma_xlate(struct of_phandle_args *dma_spec,
        atslave->dma_dev = &dmac_pdev->dev;
 
        chan = dma_request_channel(mask, at_dma_filter, atslave);
-       if (!chan)
+       if (!chan) {
+               kfree(atslave);
                return NULL;
+       }
Thanks for submitting this to stable. While the fix is good, you can instead cherry-pick the commit that hit upstream. In order to do that cleanly on top of v4.19.145, you have to pick two other fixes:

Quoted 3 lines

commit a6e7f19c9100 ("dmaengine: at_hdmac: Substitute kzalloc with kmalloc") commit 3832b78b3ec2 ("dmaengine: at_hdmac: add missing put_device() call in at_dma_xlate()") commit a6e7f19c9100 ("dmaengine: at_hdmac: Substitute kzalloc with kmalloc")

this last commit should have been
commit e097eb7473d9 ("dmaengine: at_hdmac: add missing kfree() call in at_dma_xlate()")

bad copy and paste :)

Quoted 16 lines

There are also some locking/deadlock fixes in mainline for this driver, depending on the time you can allocate for this, the list of patches can increase. I should have Cc'ed [email protected] in the first place, my bad.
Also it may worth to read the rules for submitting to stable at:
https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
Cheers,
ta
_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Greg KH

maintainer

2021-01-04 11:02 UTC #4 lore raw

Quoted 29 lines · On Wed, Sep 23, 2020 at 08:19:18AM +0000, [email protected] wrote:

Quoted 28 lines · On 9/23/20 11:13 AM, [email protected] wrote:
Hi, Pavel,
Quoted 12 lines · Pavel Machek
EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
This fixes memory leak in at_hdmac. Mainline does not have the same problem.
Signed-off-by Pavel Machek (CIP) <[email protected]>
drivers/dma/at_hdmac.c +0 -0
index 86427f6ba78c..0847b2055857 100644
drivers/dma/at_hdmac.c static struct dma_chan *at_dma_xlate(struct of_phandle_args *dma_spec, · line 1720
Quoted 8 lines
drivers/dma/at_hdmac.c +3 -1
@@ -1714,8 +1714,10 @@ static struct dma_chan *at_dma_xlate(struct of_phandle_args *dma_spec,
        atslave->dma_dev = &dmac_pdev->dev;
 
        chan = dma_request_channel(mask, at_dma_filter, atslave);
-       if (!chan)
+       if (!chan) {
+               kfree(atslave);
                return NULL;
+       }
Thanks for submitting this to stable. While the fix is good, you can instead cherry-pick the commit that hit upstream. In order to do that cleanly on top of v4.19.145, you have to pick two other fixes:

Quoted 8 lines

Quoted 3 lines
commit a6e7f19c9100 ("dmaengine: at_hdmac: Substitute kzalloc with kmalloc") commit 3832b78b3ec2 ("dmaengine: at_hdmac: add missing put_device() call in at_dma_xlate()") commit a6e7f19c9100 ("dmaengine: at_hdmac: Substitute kzalloc with kmalloc")
this last commit should have been
commit e097eb7473d9 ("dmaengine: at_hdmac: add missing kfree() call in at_dma_xlate()")
bad copy and paste :)

So are all 3 of those needed on both 5.4.y and 4.19.y to resolve this issue?

thanks,

greg k-h

[email protected]

2021-01-05 08:04 UTC #5 lore raw

Quoted 35 lines · Greg KH

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
Quoted 32 lines · On Wed, Sep 23, 2020 at 08:19:18AM +0000, [email protected] wrote:
Quoted 30 lines · On 9/23/20 11:13 AM, [email protected] wrote:
Hi, Pavel,
Quoted 12 lines · Pavel Machek
EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
This fixes memory leak in at_hdmac. Mainline does not have the same problem.
Signed-off-by Pavel Machek (CIP) <[email protected]>
drivers/dma/at_hdmac.c +0 -0
index 86427f6ba78c..0847b2055857 100644
drivers/dma/at_hdmac.c static struct dma_chan *at_dma_xlate(struct of_phandle_args *dma_spec, · line 1720
Quoted 8 lines
drivers/dma/at_hdmac.c +3 -1
@@ -1714,8 +1714,10 @@ static struct dma_chan *at_dma_xlate(struct of_phandle_args *dma_spec,
        atslave->dma_dev = &dmac_pdev->dev;
 
        chan = dma_request_channel(mask, at_dma_filter, atslave);
-       if (!chan)
+       if (!chan) {
+               kfree(atslave);
                return NULL;
+       }
Thanks for submitting this to stable. While the fix is good, you can instead cherry-pick the commit that hit upstream. In order to do that cleanly on top of v4.19.145, you have to pick two other fixes:
commit a6e7f19c9100 ("dmaengine: at_hdmac: Substitute kzalloc with kmalloc")
commit 3832b78b3ec2 ("dmaengine: at_hdmac: add missing put_device() call in at_dma_xlate()")>>> commit a6e7f19c9100 ("dmaengine: at_hdmac: Substitute kzalloc with kmalloc")

Quoted 8 lines

Quoted 4 lines
this last commit should have been
commit e097eb7473d9 ("dmaengine: at_hdmac: add missing kfree() call in at_dma_xlate()")
bad copy and paste :)
So are all 3 of those needed on both 5.4.y and 4.19.y to resolve this issue?

Yes. I've just cherry-picked all three commits on both v5.4.86 and v4.19.164, everything looks ok. I also compiled using sama5_defconfig, all good.

The order in which they should be taken is 1/ a6e7f19c9100, then 2/ 3832b78b3ec2, and 3/ e097eb7473d9.

Cheers,
ta

Greg KH

maintainer

2021-01-06 16:52 UTC #6 lore raw

Quoted 45 lines · On Tue, Jan 05, 2021 at 08:03:20AM +0000, [email protected] wrote:

Quoted 35 lines · Greg KH
EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
Quoted 32 lines · On Wed, Sep 23, 2020 at 08:19:18AM +0000, [email protected] wrote:
Quoted 30 lines · On 9/23/20 11:13 AM, [email protected] wrote:
Hi, Pavel,
Quoted 12 lines · Pavel Machek
EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
This fixes memory leak in at_hdmac. Mainline does not have the same problem.
Signed-off-by Pavel Machek (CIP) <[email protected]>
drivers/dma/at_hdmac.c +0 -0
index 86427f6ba78c..0847b2055857 100644
drivers/dma/at_hdmac.c static struct dma_chan *at_dma_xlate(struct of_phandle_args *dma_spec, · line 1720
Quoted 8 lines
drivers/dma/at_hdmac.c +3 -1
@@ -1714,8 +1714,10 @@ static struct dma_chan *at_dma_xlate(struct of_phandle_args *dma_spec,
        atslave->dma_dev = &dmac_pdev->dev;
 
        chan = dma_request_channel(mask, at_dma_filter, atslave);
-       if (!chan)
+       if (!chan) {
+               kfree(atslave);
                return NULL;
+       }
Thanks for submitting this to stable. While the fix is good, you can instead cherry-pick the commit that hit upstream. In order to do that cleanly on top of v4.19.145, you have to pick two other fixes:
commit a6e7f19c9100 ("dmaengine: at_hdmac: Substitute kzalloc with kmalloc")
commit 3832b78b3ec2 ("dmaengine: at_hdmac: add missing put_device() call in at_dma_xlate()")>>> commit a6e7f19c9100 ("dmaengine: at_hdmac: Substitute kzalloc with kmalloc")
Quoted 8 lines
Quoted 4 lines
this last commit should have been
commit e097eb7473d9 ("dmaengine: at_hdmac: add missing kfree() call in at_dma_xlate()")
bad copy and paste :)
So are all 3 of those needed on both 5.4.y and 4.19.y to resolve this issue?

Quoted 5 lines

Yes. I've just cherry-picked all three commits on both v5.4.86 and v4.19.164, everything looks ok. I also compiled using sama5_defconfig, all good.
The order in which they should be taken is 1/ a6e7f19c9100, then 2/ 3832b78b3ec2, and 3/ e097eb7473d9.

Thanks, will go queue those up now.

greg k-h