| Message ID | 20200920082838.GA813@amd |
|---|---|
| State | New |
| Headers | show |
| Series | [4.19] dmaengine: at_hdmac: Fix memory leak | expand |
On Sun, Sep 20, 2020 at 10:28:38AM +0200, Pavel Machek wrote: > Date: Sun, 20 Sep 2020 10:28:38 +0200 > From: Pavel Machek <pavel@ucw.cz> > To: linux-kernel@vger.kernel.org, dmaengine@vger.kernel.org, > linux-arm-kernel@lists.infradead.org, dan.j.williams@intel.com, > vkoul@kernel.org, ludovic.desroches@microchip.com, stable@vger.kernel.org, > Greg KH <greg@kroah.com> > Subject: [PATCH 4.19] 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) <pavel@denx.de> Acked-by: Ludovic Desroches <ludovic.desroches@microchip.com> Thanks. > > diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c > index 86427f6ba78c..0847b2055857 100644 > --- a/drivers/dma/at_hdmac.c > +++ b/drivers/dma/at_hdmac.c > @@ -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; > > -- > (english) http://www.livejournal.com/~pavelmachek > (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Hi, Pavel, On 9/20/20 11:28 AM, Pavel Machek wrote: > 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) <pavel@denx.de> > > diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c > index 86427f6ba78c..0847b2055857 100644 > --- a/drivers/dma/at_hdmac.c > +++ b/drivers/dma/at_hdmac.c > @@ -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 stable@vger.kernel.org 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
On 9/23/20 11:13 AM, Tudor.Ambarus@microchip.com wrote: > Hi, Pavel, > > On 9/20/20 11:28 AM, Pavel Machek wrote: >> 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) <pavel@denx.de> >> >> diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c >> index 86427f6ba78c..0847b2055857 100644 >> --- a/drivers/dma/at_hdmac.c >> +++ b/drivers/dma/at_hdmac.c >> @@ -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") this last commit should have been commit e097eb7473d9 ("dmaengine: at_hdmac: add missing kfree() call in at_dma_xlate()") bad copy and paste :) > > 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 stable@vger.kernel.org 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 > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel >
On Wed, Sep 23, 2020 at 08:19:18AM +0000, Tudor.Ambarus@microchip.com wrote: > On 9/23/20 11:13 AM, Tudor.Ambarus@microchip.com wrote: > > Hi, Pavel, > > > > On 9/20/20 11:28 AM, Pavel Machek wrote: > >> 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) <pavel@denx.de> > >> > >> diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c > >> index 86427f6ba78c..0847b2055857 100644 > >> --- a/drivers/dma/at_hdmac.c > >> +++ b/drivers/dma/at_hdmac.c > >> @@ -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") > > 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
On 1/4/21 1:02 PM, Greg KH wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > On Wed, Sep 23, 2020 at 08:19:18AM +0000, Tudor.Ambarus@microchip.com wrote: >> On 9/23/20 11:13 AM, Tudor.Ambarus@microchip.com wrote: >>> Hi, Pavel, >>> >>> On 9/20/20 11:28 AM, Pavel Machek wrote: >>>> 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) <pavel@denx.de> >>>> >>>> diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c >>>> index 86427f6ba78c..0847b2055857 100644 >>>> --- a/drivers/dma/at_hdmac.c >>>> +++ b/drivers/dma/at_hdmac.c >>>> @@ -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") >> >> 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
On Tue, Jan 05, 2021 at 08:03:20AM +0000, Tudor.Ambarus@microchip.com wrote: > On 1/4/21 1:02 PM, Greg KH wrote: > > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > > > On Wed, Sep 23, 2020 at 08:19:18AM +0000, Tudor.Ambarus@microchip.com wrote: > >> On 9/23/20 11:13 AM, Tudor.Ambarus@microchip.com wrote: > >>> Hi, Pavel, > >>> > >>> On 9/20/20 11:28 AM, Pavel Machek wrote: > >>>> 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) <pavel@denx.de> > >>>> > >>>> diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c > >>>> index 86427f6ba78c..0847b2055857 100644 > >>>> --- a/drivers/dma/at_hdmac.c > >>>> +++ b/drivers/dma/at_hdmac.c > >>>> @@ -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") > >> > >> 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. Thanks, will go queue those up now. greg k-h
diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c index 86427f6ba78c..0847b2055857 100644 --- a/drivers/dma/at_hdmac.c +++ b/drivers/dma/at_hdmac.c @@ -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;
This fixes memory leak in at_hdmac. Mainline does not have the same problem. Signed-off-by: Pavel Machek (CIP) <pavel@denx.de>