From: Miquel Raynal <miquel.raynal@bootlin.com>
To: Colin King <colin.king@canonical.com>
Cc: Richard Weinberger <richard@nod.at>,
Vignesh Raghavendra <vigneshr@ti.com>,
linux-mtd@lists.infradead.org, kernel-janitors@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH][next] mtd: rawnand: ensure return variable is initialized
Date: Thu, 27 May 2021 17:03:09 +0200 [thread overview]
Message-ID: <20210527170309.4d99bc31@xps13> (raw)
In-Reply-To: <20210527145048.795954-1-colin.king@canonical.com>
Hi Colin,
Colin King <colin.king@canonical.com> wrote on Thu, 27 May 2021
15:50:48 +0100:
> From: Colin Ian King <colin.king@canonical.com>
>
> Currently there are corner cases where spec_times is NULL and
> chip->parameters.onfi or when best_mode is zero where ret is
^
something is missing here, the sentence is not clear
> not assigned a value and an uninitialized return value can be
> returned. Fix this by ensuring ret is initialized to -EINVAL.
I don't see how this situation can happen.
In both cases, no matter the value of best_mode, the for loop will
always execute at least one time (mode 0) so ret will be populated.
Maybe the robot does not know that best_mode cannot be negative and
should be defined unsigned, but the current patch is invalid.
> Addresses-Coverity: ("Uninitialized scalar variable")
> Fixes: 9d3194bf2aef ("mtd: rawnand: Allow SDR timings to be nacked")
> Fixes: a9ecc8c814e9 ("mtd: rawnand: Choose the best timings, NV-DDR included")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
> drivers/mtd/nand/raw/nand_base.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> index 57a583149cc0..18db742f650c 100644
> --- a/drivers/mtd/nand/raw/nand_base.c
> +++ b/drivers/mtd/nand/raw/nand_base.c
> @@ -926,7 +926,7 @@ int nand_choose_best_sdr_timings(struct nand_chip *chip,
> struct nand_sdr_timings *spec_timings)
> {
> const struct nand_controller_ops *ops = chip->controller->ops;
> - int best_mode = 0, mode, ret;
> + int best_mode = 0, mode, ret = -EINVAL;
>
> iface->type = NAND_SDR_IFACE;
>
> @@ -977,7 +977,7 @@ int nand_choose_best_nvddr_timings(struct nand_chip *chip,
> struct nand_nvddr_timings *spec_timings)
> {
> const struct nand_controller_ops *ops = chip->controller->ops;
> - int best_mode = 0, mode, ret;
> + int best_mode = 0, mode, ret = 0;
>
> iface->type = NAND_NVDDR_IFACE;
>
Thanks,
Miquèl
WARNING: multiple messages have this Message-ID (diff)
From: Miquel Raynal <miquel.raynal@bootlin.com>
To: Colin King <colin.king@canonical.com>
Cc: Richard Weinberger <richard@nod.at>,
Vignesh Raghavendra <vigneshr@ti.com>,
linux-mtd@lists.infradead.org, kernel-janitors@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH][next] mtd: rawnand: ensure return variable is initialized
Date: Thu, 27 May 2021 17:03:09 +0200 [thread overview]
Message-ID: <20210527170309.4d99bc31@xps13> (raw)
In-Reply-To: <20210527145048.795954-1-colin.king@canonical.com>
Hi Colin,
Colin King <colin.king@canonical.com> wrote on Thu, 27 May 2021
15:50:48 +0100:
> From: Colin Ian King <colin.king@canonical.com>
>
> Currently there are corner cases where spec_times is NULL and
> chip->parameters.onfi or when best_mode is zero where ret is
^
something is missing here, the sentence is not clear
> not assigned a value and an uninitialized return value can be
> returned. Fix this by ensuring ret is initialized to -EINVAL.
I don't see how this situation can happen.
In both cases, no matter the value of best_mode, the for loop will
always execute at least one time (mode 0) so ret will be populated.
Maybe the robot does not know that best_mode cannot be negative and
should be defined unsigned, but the current patch is invalid.
> Addresses-Coverity: ("Uninitialized scalar variable")
> Fixes: 9d3194bf2aef ("mtd: rawnand: Allow SDR timings to be nacked")
> Fixes: a9ecc8c814e9 ("mtd: rawnand: Choose the best timings, NV-DDR included")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
> drivers/mtd/nand/raw/nand_base.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> index 57a583149cc0..18db742f650c 100644
> --- a/drivers/mtd/nand/raw/nand_base.c
> +++ b/drivers/mtd/nand/raw/nand_base.c
> @@ -926,7 +926,7 @@ int nand_choose_best_sdr_timings(struct nand_chip *chip,
> struct nand_sdr_timings *spec_timings)
> {
> const struct nand_controller_ops *ops = chip->controller->ops;
> - int best_mode = 0, mode, ret;
> + int best_mode = 0, mode, ret = -EINVAL;
>
> iface->type = NAND_SDR_IFACE;
>
> @@ -977,7 +977,7 @@ int nand_choose_best_nvddr_timings(struct nand_chip *chip,
> struct nand_nvddr_timings *spec_timings)
> {
> const struct nand_controller_ops *ops = chip->controller->ops;
> - int best_mode = 0, mode, ret;
> + int best_mode = 0, mode, ret = 0;
>
> iface->type = NAND_NVDDR_IFACE;
>
Thanks,
Miquèl
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
next prev parent reply other threads:[~2021-05-27 15:03 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-05-27 14:50 [PATCH][next] mtd: rawnand: ensure return variable is initialized Colin King
2021-05-27 14:50 ` Colin King
2021-05-27 15:03 ` Miquel Raynal [this message]
2021-05-27 15:03 ` Miquel Raynal
2021-05-27 15:22 ` Colin Ian King
2021-05-27 15:22 ` Colin Ian King
2021-06-01 12:14 ` Dan Carpenter
2021-06-01 12:14 ` Dan Carpenter
2021-06-01 12:58 ` Dan Carpenter
2021-06-01 12:58 ` Dan Carpenter
2021-06-07 6:57 ` Miquel Raynal
2021-06-07 6:57 ` Miquel Raynal
2021-06-08 6:10 ` Dan Carpenter
2021-06-08 6:10 ` Dan Carpenter
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=20210527170309.4d99bc31@xps13 \
--to=miquel.raynal@bootlin.com \
--cc=colin.king@canonical.com \
--cc=kernel-janitors@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mtd@lists.infradead.org \
--cc=richard@nod.at \
--cc=vigneshr@ti.com \
/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.