Linux-mtd Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] mtd: onenand: prevent memory leak in onenand_scan
@ 2019-09-25 15:43 Navid Emamdoost
  2019-09-27 15:24 ` Markus Elfring
  2019-09-30 21:37 ` Navid Emamdoost
  0 siblings, 2 replies; 8+ messages in thread
From: Navid Emamdoost @ 2019-09-25 15:43 UTC (permalink / raw)
  Cc: Vignesh Raghavendra, Richard Weinberger, smccaman, kjlu,
	linux-kernel, Marek Vasut, Kyungmin Park, emamd001, linux-mtd,
	Miquel Raynal, Brian Norris, David Woodhouse, Navid Emamdoost

In onenand_scan if scan_bbt fails the allocated buffers should be
released.

Signed-off-by: Navid Emamdoost <navid.emamdoost@gmail.com>
---
 drivers/mtd/nand/onenand/onenand_base.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/onenand/onenand_base.c b/drivers/mtd/nand/onenand/onenand_base.c
index 77bd32a683e1..79c01f42925a 100644
--- a/drivers/mtd/nand/onenand/onenand_base.c
+++ b/drivers/mtd/nand/onenand/onenand_base.c
@@ -3977,8 +3977,11 @@ int onenand_scan(struct mtd_info *mtd, int maxchips)
 	this->badblockpos = ONENAND_BADBLOCK_POS;
 
 	ret = this->scan_bbt(mtd);
-	if ((!FLEXONENAND(this)) || ret)
+	if ((!FLEXONENAND(this)) || ret) {
+		kfree(this->page_buf);
+		kfree(this->oob_buf);
 		return ret;
+	}
 
 	/* Change Flex-OneNAND boundaries if required */
 	for (i = 0; i < MAX_DIES; i++)
-- 
2.17.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] mtd: onenand: prevent memory leak in onenand_scan
  2019-09-25 15:43 [PATCH] mtd: onenand: prevent memory leak in onenand_scan Navid Emamdoost
@ 2019-09-27 15:24 ` Markus Elfring
  2019-09-30 21:37 ` Navid Emamdoost
  1 sibling, 0 replies; 8+ messages in thread
From: Markus Elfring @ 2019-09-27 15:24 UTC (permalink / raw)
  To: Navid Emamdoost, linux-mtd
  Cc: kernel-janitors, Vignesh Raghavendra, Richard Weinberger,
	Stephen A McCamant, Kangjie Lu, linux-kernel, Marek Vasut,
	Kyungmin Park, Navid Emamdoost, Miquel Raynal, Brian Norris,
	David Woodhouse

> In onenand_scan if scan_bbt fails the allocated buffers should be released.

Will an other change description be more appropriate?


How do you think about to add the tag “Fixes” here?

Regards,
Markus

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] mtd: onenand: prevent memory leak in onenand_scan
  2019-09-25 15:43 [PATCH] mtd: onenand: prevent memory leak in onenand_scan Navid Emamdoost
  2019-09-27 15:24 ` Markus Elfring
@ 2019-09-30 21:37 ` Navid Emamdoost
  2019-10-04 15:57   ` Miquel Raynal
  1 sibling, 1 reply; 8+ messages in thread
From: Navid Emamdoost @ 2019-09-30 21:37 UTC (permalink / raw)
  To: Kyungmin Park, Miquel Raynal, Richard Weinberger,
	Vignesh Raghavendra, Marek Vasut, Brian Norris, David Woodhouse
  Cc: linux-mtd, Navid Emamdoost, kjlu, linux-kernel, Stephen McCamant

Would you please take a look at this patch?

On Wed, Sep 25, 2019 at 10:43 AM Navid Emamdoost
<navid.emamdoost@gmail.com> wrote:
>
> In onenand_scan if scan_bbt fails the allocated buffers should be
> released.
>
> Signed-off-by: Navid Emamdoost <navid.emamdoost@gmail.com>
> ---
>  drivers/mtd/nand/onenand/onenand_base.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mtd/nand/onenand/onenand_base.c b/drivers/mtd/nand/onenand/onenand_base.c
> index 77bd32a683e1..79c01f42925a 100644
> --- a/drivers/mtd/nand/onenand/onenand_base.c
> +++ b/drivers/mtd/nand/onenand/onenand_base.c
> @@ -3977,8 +3977,11 @@ int onenand_scan(struct mtd_info *mtd, int maxchips)
>         this->badblockpos = ONENAND_BADBLOCK_POS;
>
>         ret = this->scan_bbt(mtd);
> -       if ((!FLEXONENAND(this)) || ret)
> +       if ((!FLEXONENAND(this)) || ret) {
> +               kfree(this->page_buf);
> +               kfree(this->oob_buf);
>                 return ret;
> +       }
>
>         /* Change Flex-OneNAND boundaries if required */
>         for (i = 0; i < MAX_DIES; i++)
> --
> 2.17.1
>


-- 
Navid.

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] mtd: onenand: prevent memory leak in onenand_scan
  2019-09-30 21:37 ` Navid Emamdoost
@ 2019-10-04 15:57   ` Miquel Raynal
  2019-10-04 17:19     ` [PATCH v2] " Navid Emamdoost
  2019-10-04 17:20     ` [PATCH] " Navid Emamdoost
  0 siblings, 2 replies; 8+ messages in thread
From: Miquel Raynal @ 2019-10-04 15:57 UTC (permalink / raw)
  To: Navid Emamdoost
  Cc: Vignesh Raghavendra, Richard Weinberger, kjlu, linux-kernel,
	Marek Vasut, Kyungmin Park, Navid Emamdoost, linux-mtd,
	Stephen McCamant, Brian Norris, David Woodhouse

Hi Navid,

Navid Emamdoost <navid.emamdoost@gmail.com> wrote on Mon, 30 Sep 2019
16:37:17 -0500:

> Would you please take a look at this patch?
> 
> On Wed, Sep 25, 2019 at 10:43 AM Navid Emamdoost
> <navid.emamdoost@gmail.com> wrote:
> >
> > In onenand_scan if scan_bbt fails the allocated buffers should be
> > released.
> >
> > Signed-off-by: Navid Emamdoost <navid.emamdoost@gmail.com>
> > ---
> >  drivers/mtd/nand/onenand/onenand_base.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/mtd/nand/onenand/onenand_base.c b/drivers/mtd/nand/onenand/onenand_base.c
> > index 77bd32a683e1..79c01f42925a 100644
> > --- a/drivers/mtd/nand/onenand/onenand_base.c
> > +++ b/drivers/mtd/nand/onenand/onenand_base.c
> > @@ -3977,8 +3977,11 @@ int onenand_scan(struct mtd_info *mtd, int maxchips)
> >         this->badblockpos = ONENAND_BADBLOCK_POS;
> >
> >         ret = this->scan_bbt(mtd);
> > -       if ((!FLEXONENAND(this)) || ret)
> > +       if ((!FLEXONENAND(this)) || ret) {
> > +               kfree(this->page_buf);

Apparently you missed:

#ifdef CONFIG_MTD_ONENAND_VERIFY_WRITE
        kfree(this->verify_buf);
#endif

> > +               kfree(this->oob_buf);
> >                 return ret;
> > +       }
> >
> >         /* Change Flex-OneNAND boundaries if required */
> >         for (i = 0; i < MAX_DIES; i++)
> > --
> > 2.17.1
> >  
> 
> 

Thanks,
Miquèl

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2] mtd: onenand: prevent memory leak in onenand_scan
  2019-10-04 15:57   ` Miquel Raynal
@ 2019-10-04 17:19     ` " Navid Emamdoost
  2019-10-05 14:04       ` Markus Elfring
  2019-10-07  8:54       ` Miquel Raynal
  2019-10-04 17:20     ` [PATCH] " Navid Emamdoost
  1 sibling, 2 replies; 8+ messages in thread
From: Navid Emamdoost @ 2019-10-04 17:19 UTC (permalink / raw)
  To: miquel.raynal
  Cc: Vignesh Raghavendra, Richard Weinberger, kjlu, linux-kernel,
	Marek Vasut, Kyungmin Park, emamd001, linux-mtd, smccaman,
	Brian Norris, David Woodhouse, Navid Emamdoost

In onenand_scan if scan_bbt fails the allocated buffers for oob_buf,
verify_buf, and page_buf should be released.

Fixes: 5988af231978 ("mtd: Flex-OneNAND support")
Signed-off-by: Navid Emamdoost <navid.emamdoost@gmail.com>
---
Changes in v2:
	-- added release for this->verify_buf (thanks to Miquel Raynal
for the hint).
---
 drivers/mtd/nand/onenand/onenand_base.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/onenand/onenand_base.c b/drivers/mtd/nand/onenand/onenand_base.c
index 77bd32a683e1..6329ada3f15c 100644
--- a/drivers/mtd/nand/onenand/onenand_base.c
+++ b/drivers/mtd/nand/onenand/onenand_base.c
@@ -3977,8 +3977,14 @@ int onenand_scan(struct mtd_info *mtd, int maxchips)
 	this->badblockpos = ONENAND_BADBLOCK_POS;
 
 	ret = this->scan_bbt(mtd);
-	if ((!FLEXONENAND(this)) || ret)
+	if ((!FLEXONENAND(this)) || ret) {
+		kfree(this->oob_buf);
+#ifdef CONFIG_MTD_ONENAND_VERIFY_WRITE
+		kfree(this->verify_buf);
+#endif
+		kfree(this->page_buf);
 		return ret;
+	}
 
 	/* Change Flex-OneNAND boundaries if required */
 	for (i = 0; i < MAX_DIES; i++)
-- 
2.17.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] mtd: onenand: prevent memory leak in onenand_scan
  2019-10-04 15:57   ` Miquel Raynal
  2019-10-04 17:19     ` [PATCH v2] " Navid Emamdoost
@ 2019-10-04 17:20     ` " Navid Emamdoost
  1 sibling, 0 replies; 8+ messages in thread
From: Navid Emamdoost @ 2019-10-04 17:20 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Vignesh Raghavendra, Richard Weinberger, Kangjie Lu, LKML,
	Marek Vasut, Kyungmin Park, Navid Emamdoost, linux-mtd,
	Stephen McCamant, Brian Norris, David Woodhouse

Thanks Miquel for pointing that out. v2 was sent.

Thank you,
Navid.

On Fri, Oct 4, 2019 at 10:57 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
> Hi Navid,
>
> Navid Emamdoost <navid.emamdoost@gmail.com> wrote on Mon, 30 Sep 2019
> 16:37:17 -0500:
>
> > Would you please take a look at this patch?
> >
> > On Wed, Sep 25, 2019 at 10:43 AM Navid Emamdoost
> > <navid.emamdoost@gmail.com> wrote:
> > >
> > > In onenand_scan if scan_bbt fails the allocated buffers should be
> > > released.
> > >
> > > Signed-off-by: Navid Emamdoost <navid.emamdoost@gmail.com>
> > > ---
> > >  drivers/mtd/nand/onenand/onenand_base.c | 5 ++++-
> > >  1 file changed, 4 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/mtd/nand/onenand/onenand_base.c b/drivers/mtd/nand/onenand/onenand_base.c
> > > index 77bd32a683e1..79c01f42925a 100644
> > > --- a/drivers/mtd/nand/onenand/onenand_base.c
> > > +++ b/drivers/mtd/nand/onenand/onenand_base.c
> > > @@ -3977,8 +3977,11 @@ int onenand_scan(struct mtd_info *mtd, int maxchips)
> > >         this->badblockpos = ONENAND_BADBLOCK_POS;
> > >
> > >         ret = this->scan_bbt(mtd);
> > > -       if ((!FLEXONENAND(this)) || ret)
> > > +       if ((!FLEXONENAND(this)) || ret) {
> > > +               kfree(this->page_buf);
>
> Apparently you missed:
>
> #ifdef CONFIG_MTD_ONENAND_VERIFY_WRITE
>         kfree(this->verify_buf);
> #endif
>
> > > +               kfree(this->oob_buf);
> > >                 return ret;
> > > +       }
> > >
> > >         /* Change Flex-OneNAND boundaries if required */
> > >         for (i = 0; i < MAX_DIES; i++)
> > > --
> > > 2.17.1
> > >
> >
> >
>
> Thanks,
> Miquèl



-- 
Navid.

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] mtd: onenand: prevent memory leak in onenand_scan
  2019-10-04 17:19     ` [PATCH v2] " Navid Emamdoost
@ 2019-10-05 14:04       ` Markus Elfring
  2019-10-07  8:54       ` Miquel Raynal
  1 sibling, 0 replies; 8+ messages in thread
From: Markus Elfring @ 2019-10-05 14:04 UTC (permalink / raw)
  To: Navid Emamdoost, linux-mtd
  Cc: kernel-janitors, Vignesh Raghavendra, Richard Weinberger,
	Miquel Raynal, Kangjie Lu, linux-kernel, Marek Vasut,
	Kyungmin Park, Navid Emamdoost, Stephen McCamant, Brian Norris,
	David Woodhouse

> In onenand_scan if scan_bbt fails the allocated buffers for oob_buf,
> verify_buf, and page_buf should be released.

Can a wording like the following be nicer for the change description?

  Release the memory for the buffers “oob_buf”, “verify_buf” and “page_buf”
  after a call of the function “scan_bbt” failed in the implementation
  of the function “onenand_scan”.


> Fixes: 5988af231978 ("mtd: Flex-OneNAND support")
> ---
> Changes in v2:
> for the hint).

Did you take another review comment into account for this patch change log?


> ---

Please replace the delimiter at this place by a blank line in subsequent messages.


>  drivers/mtd/nand/onenand/onenand_base.c | 8 +++++++-
…


Regards,
Markus

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] mtd: onenand: prevent memory leak in onenand_scan
  2019-10-04 17:19     ` [PATCH v2] " Navid Emamdoost
  2019-10-05 14:04       ` Markus Elfring
@ 2019-10-07  8:54       ` Miquel Raynal
  1 sibling, 0 replies; 8+ messages in thread
From: Miquel Raynal @ 2019-10-07  8:54 UTC (permalink / raw)
  To: Navid Emamdoost
  Cc: Vignesh Raghavendra, Richard Weinberger, kjlu, linux-kernel,
	Marek Vasut, Kyungmin Park, emamd001, linux-mtd, smccaman,
	Brian Norris, David Woodhouse

Hi Navid,

Navid Emamdoost <navid.emamdoost@gmail.com> wrote on Fri,  4 Oct 2019
12:19:05 -0500:

> In onenand_scan if scan_bbt fails the allocated buffers for oob_buf,
> verify_buf, and page_buf should be released.
> 
> Fixes: 5988af231978 ("mtd: Flex-OneNAND support")

Missing Cc: stable@vger.kernel.org

> Signed-off-by: Navid Emamdoost <navid.emamdoost@gmail.com>
> ---
> Changes in v2:
> 	-- added release for this->verify_buf (thanks to Miquel Raynal
> for the hint).
> ---

These three dashes are not needed.

>  drivers/mtd/nand/onenand/onenand_base.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/nand/onenand/onenand_base.c b/drivers/mtd/nand/onenand/onenand_base.c
> index 77bd32a683e1..6329ada3f15c 100644
> --- a/drivers/mtd/nand/onenand/onenand_base.c
> +++ b/drivers/mtd/nand/onenand/onenand_base.c
> @@ -3977,8 +3977,14 @@ int onenand_scan(struct mtd_info *mtd, int maxchips)
>  	this->badblockpos = ONENAND_BADBLOCK_POS;
>  
>  	ret = this->scan_bbt(mtd);
> -	if ((!FLEXONENAND(this)) || ret)
> +	if ((!FLEXONENAND(this)) || ret) {
> +		kfree(this->oob_buf);
> +#ifdef CONFIG_MTD_ONENAND_VERIFY_WRITE
> +		kfree(this->verify_buf);
> +#endif

Sorry for the ping-pong but actually, only the oob_buf and page_buf
have been introduced by the commit 5988af you point in the Fixes tag.

To help stable kernels maintainers I suggest you free the verify_buf
in a second patch which fixes:

4a8ce0b03071 mtd: onenand: allocate verify buffer in the core

> +		kfree(this->page_buf);
>  		return ret;
> +	}
>  
>  	/* Change Flex-OneNAND boundaries if required */
>  	for (i = 0; i < MAX_DIES; i++)

Thanks,
Miquèl

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, back to index

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-25 15:43 [PATCH] mtd: onenand: prevent memory leak in onenand_scan Navid Emamdoost
2019-09-27 15:24 ` Markus Elfring
2019-09-30 21:37 ` Navid Emamdoost
2019-10-04 15:57   ` Miquel Raynal
2019-10-04 17:19     ` [PATCH v2] " Navid Emamdoost
2019-10-05 14:04       ` Markus Elfring
2019-10-07  8:54       ` Miquel Raynal
2019-10-04 17:20     ` [PATCH] " Navid Emamdoost

Linux-mtd Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-mtd/0 linux-mtd/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-mtd linux-mtd/ https://lore.kernel.org/linux-mtd \
		linux-mtd@lists.infradead.org linux-mtd@archiver.kernel.org
	public-inbox-index linux-mtd

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.infradead.lists.linux-mtd


AGPL code for this site: git clone https://public-inbox.org/ public-inbox