Messages in this thread | | | Subject | Re: [PATCH] mtd: spi-nor: Check for spi_nor_hwcaps_read2cmd() return value | From | Cyrille Pitchen <> | Date | Tue, 19 Sep 2017 19:33:15 +0200 |
| |
Hi Richard,
Le 17/09/2017 à 11:57, Richard Weinberger a écrit : > The function can return a negativ value in case of errors, > don't use it blindly as array index. > > Detected by CoverityScan CID#1418067 ("Memory - illegal accesses") > Fixes: f384b352cbf0 ("mtd: spi-nor: parse Serial Flash Discoverable > Parameters (SFDP) tables") > Signed-off-by: Richard Weinberger <richard@nod.at> > --- > drivers/mtd/spi-nor/spi-nor.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c > index cf1d4a15e10a..d71765739a93 100644 > --- a/drivers/mtd/spi-nor/spi-nor.c > +++ b/drivers/mtd/spi-nor/spi-nor.c > @@ -2145,6 +2145,9 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor, > > params->hwcaps.mask |= rd->hwcaps; > cmd = spi_nor_hwcaps_read2cmd(rd->hwcaps); > + if (cmd < 0) > + return -EINVAL; > + > read = ¶ms->reads[cmd]; > half = bfpt.dwords[rd->settings_dword] >> rd->settings_shift; > spi_nor_set_read_settings_from_bfpt(read, half, rd->proto); >
This one is a false positive:
static const struct sfdp_bfpt_read sfdp_bfpt_reads[] = { /* Fast Read 1-1-2 */ { SNOR_HWCAPS_READ_1_1_2, BFPT_DWORD(1), BIT(16), /* Supported bit */ BFPT_DWORD(4), 0, /* Settings */ SNOR_PROTO_1_1_2, }, [...] };
static int spi_nor_parse_bfpt(...) { [...] for (i = 0; i < ARRAY_SIZE(sfdp_bfpt_reads); i++) { const struct sfdp_bfpt_read *rd = &sfdp_bfpt_reads[i]; [...] cmd = spi_nor_hwcaps_read2cmd(rd->hwcaps); read = ¶ms->reads[cmd]; [...] } [...] }
sfdp_bfpt_reads[] is a static const array where each element has a valid .hwcaps value so spi_nor_hwcaps_read2cmd() can actually never return an out of range 'cmd' result here.
However in spi_nor_select_read(), spi_nor_hwcaps_read2cmd() is called too but this time its input argument now comes from the spi_nor_scan() caller so in this later case the code already checks the output of spi_nor_hwcaps_read2cmd() as it could be -EINVAL.
Do you want us to take this patch anyway to please the code checker ? I don't think it is mandatory as a fix for 4.14-rc2.
Best regards,
Cyrille
| |