All of lore.kernel.org
 help / color / mirror / Atom feed
From: Boris Brezillon <boris.brezillon@free-electrons.com>
To: Colin King <colin.king@canonical.com>
Cc: Richard Weinberger <richard@nod.at>,
	David Woodhouse <dwmw2@infradead.org>,
	Brian Norris <computersforpeace@gmail.com>,
	Marek Vasut <marek.vasut@gmail.com>,
	Cyrille Pitchen <cyrille.pitchen@wedev4u.fr>,
	linux-mtd@lists.infradead.org, kernel-janitors@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] mtd: nand: fix memory leak on ep on error exit returns
Date: Wed, 13 Dec 2017 21:02:41 +0000	[thread overview]
Message-ID: <20171213220241.29bbe131@bbrezillon> (raw)
In-Reply-To: <20171213204909.28287-1-colin.king@canonical.com>

Hi Colin,

On Wed, 13 Dec 2017 20:49:09 +0000
Colin King <colin.king@canonical.com> wrote:

> From: Colin Ian King <colin.king@canonical.com>
> 
> There are two error return paths that are not kfree'ing ep that
> lead to memory leaks.  Fix this by exiting on error via the
> ext_out exit path that performs the necessary kfree.

Well, given that no one implements the ->exec_op() hook yet, the
xxxx_op() functions always return 0 right now, but I agree, we
should fix the generic case. 

> 
> Detected by CoverityScan, CID#1462747 ("Resource Leak")
> 
> Fixes: b83ea87958c5 ("mtd: nand: provide several helpers to do common NAND operations")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>

If you don't mind, I'd like to squash these changes in the original
commit.

Thanks,

Boris

> ---
>  drivers/mtd/nand/nand_base.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> index eb810d5d44e7..1c5126d1db8c 100644
> --- a/drivers/mtd/nand/nand_base.c
> +++ b/drivers/mtd/nand/nand_base.c
> @@ -5046,14 +5046,14 @@ static int nand_flash_detect_ext_param_page(struct nand_chip *chip,
>  	/* Send our own NAND_CMD_PARAM. */
>  	ret = nand_read_param_page_op(chip, 0, NULL, 0);
>  	if (ret)
> -		return ret;
> +		goto ext_out;
>  
>  	/* Use the Change Read Column command to skip the ONFI param pages. */
>  	ret = nand_change_read_column_op(chip,
>  					 sizeof(*p) * p->num_of_param_pages,
>  					 ep, len, true);
>  	if (ret)
> -		return ret;
> +		goto ext_out;
>  
>  	ret = -EINVAL;
>  	if ((onfi_crc16(ONFI_CRC_BASE, ((uint8_t *)ep) + 2, len - 2)


WARNING: multiple messages have this Message-ID (diff)
From: Boris Brezillon <boris.brezillon@free-electrons.com>
To: Colin King <colin.king@canonical.com>
Cc: Richard Weinberger <richard@nod.at>,
	David Woodhouse <dwmw2@infradead.org>,
	Brian Norris <computersforpeace@gmail.com>,
	Marek Vasut <marek.vasut@gmail.com>,
	Cyrille Pitchen <cyrille.pitchen@wedev4u.fr>,
	linux-mtd@lists.infradead.org, kernel-janitors@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] mtd: nand: fix memory leak on ep on error exit returns
Date: Wed, 13 Dec 2017 22:02:41 +0100	[thread overview]
Message-ID: <20171213220241.29bbe131@bbrezillon> (raw)
In-Reply-To: <20171213204909.28287-1-colin.king@canonical.com>

Hi Colin,

On Wed, 13 Dec 2017 20:49:09 +0000
Colin King <colin.king@canonical.com> wrote:

> From: Colin Ian King <colin.king@canonical.com>
> 
> There are two error return paths that are not kfree'ing ep that
> lead to memory leaks.  Fix this by exiting on error via the
> ext_out exit path that performs the necessary kfree.

Well, given that no one implements the ->exec_op() hook yet, the
xxxx_op() functions always return 0 right now, but I agree, we
should fix the generic case. 

> 
> Detected by CoverityScan, CID#1462747 ("Resource Leak")
> 
> Fixes: b83ea87958c5 ("mtd: nand: provide several helpers to do common NAND operations")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>

If you don't mind, I'd like to squash these changes in the original
commit.

Thanks,

Boris

> ---
>  drivers/mtd/nand/nand_base.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> index eb810d5d44e7..1c5126d1db8c 100644
> --- a/drivers/mtd/nand/nand_base.c
> +++ b/drivers/mtd/nand/nand_base.c
> @@ -5046,14 +5046,14 @@ static int nand_flash_detect_ext_param_page(struct nand_chip *chip,
>  	/* Send our own NAND_CMD_PARAM. */
>  	ret = nand_read_param_page_op(chip, 0, NULL, 0);
>  	if (ret)
> -		return ret;
> +		goto ext_out;
>  
>  	/* Use the Change Read Column command to skip the ONFI param pages. */
>  	ret = nand_change_read_column_op(chip,
>  					 sizeof(*p) * p->num_of_param_pages,
>  					 ep, len, true);
>  	if (ret)
> -		return ret;
> +		goto ext_out;
>  
>  	ret = -EINVAL;
>  	if ((onfi_crc16(ONFI_CRC_BASE, ((uint8_t *)ep) + 2, len - 2)

  reply	other threads:[~2017-12-13 21:02 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-12-13 20:49 [PATCH] mtd: nand: fix memory leak on ep on error exit returns Colin King
2017-12-13 21:02 ` Boris Brezillon [this message]
2017-12-13 21:02   ` Boris Brezillon
2017-12-13 21:09   ` Colin Ian King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20171213220241.29bbe131@bbrezillon \
    --to=boris.brezillon@free-electrons.com \
    --cc=colin.king@canonical.com \
    --cc=computersforpeace@gmail.com \
    --cc=cyrille.pitchen@wedev4u.fr \
    --cc=dwmw2@infradead.org \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=marek.vasut@gmail.com \
    --cc=richard@nod.at \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.