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: linkBe 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.