| Message ID | 20230202191451.35142-1-tudor.ambarus@linaro.org |
|---|---|
| State | Superseded |
| Delegated to: | Ambarus Tudor |
| Headers | show |
| Series | [v2] mtd: spi-nor: Fix shift-out-of-bounds in spi_nor_set_erase_type | expand |
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c > index 247d1014879a..9b90d941d87a 100644 > --- a/drivers/mtd/spi-nor/core.c > +++ b/drivers/mtd/spi-nor/core.c > @@ -2019,10 +2019,22 @@ void spi_nor_set_erase_type(struct spi_nor_erase_type *erase, u32 size, > u8 opcode) > { > erase->size = size; > - erase->opcode = opcode; > - /* JEDEC JESD216B Standard imposes erase sizes to be power of 2. */ > - erase->size_shift = ffs(erase->size) - 1; > - erase->size_mask = (1 << erase->size_shift) - 1; > + > + if (size) { actually this is not needed now that spi_nor_mask_erase_type() is introduced. All the callers of spi_nor_set_erase_type() guarantee that erase_size is not zero. > + erase->opcode = opcode; > + /* JEDEC JESD216B imposes erase sizes to be power of 2. */ > + erase->size_shift = ffs(size) - 1; > + erase->size_mask = (1 << erase->size_shift) - 1; > + } > +} > + So the fix should just contain the introduction of spi_nor_mask_erase_type(). Louis, do you want to authorship v3? > +/** > + * spi_nor_mask_erase_type() - mask out an SPI NOR erase type > + * @erase: pointer to a structure that describes a SPI NOR erase type an SPI > + */ > +void spi_nor_mask_erase_type(struct spi_nor_erase_type *erase) > +{ > + erase->size = 0; > } > > /** > diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h > index f6d012e1f681..25423225c29d 100644 > --- a/drivers/mtd/spi-nor/core.h > +++ b/drivers/mtd/spi-nor/core.h > @@ -681,6 +681,7 @@ void spi_nor_set_pp_settings(struct spi_nor_pp_command *pp, u8 opcode, > > void spi_nor_set_erase_type(struct spi_nor_erase_type *erase, u32 size, > u8 opcode); > +void spi_nor_mask_erase_type(struct spi_nor_erase_type *erase); > struct spi_nor_erase_region * > spi_nor_region_next(struct spi_nor_erase_region *region); > void spi_nor_init_uniform_erase_map(struct spi_nor_erase_map *map, > diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c > index fd4daf8fa5df..298ab5e53a8c 100644 > --- a/drivers/mtd/spi-nor/sfdp.c > +++ b/drivers/mtd/spi-nor/sfdp.c > @@ -875,7 +875,7 @@ static int spi_nor_init_non_uniform_erase_map(struct spi_nor *nor, > */ > for (i = 0; i < SNOR_ERASE_TYPE_MAX; i++) > if (!(regions_erase_type & BIT(erase[i].idx))) > - spi_nor_set_erase_type(&erase[i], 0, 0xFF); > + spi_nor_mask_erase_type(&erase[i]); > > return 0; > } > @@ -1089,7 +1089,7 @@ static int spi_nor_parse_4bait(struct spi_nor *nor, > erase_type[i].opcode = (dwords[SFDP_DWORD(2)] >> > erase_type[i].idx * 8) & 0xFF; > else > - spi_nor_set_erase_type(&erase_type[i], 0u, 0xFF); > + spi_nor_mask_erase_type(&erase_type[i]); > } > > /*
diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c index 247d1014879a..9b90d941d87a 100644 --- a/drivers/mtd/spi-nor/core.c +++ b/drivers/mtd/spi-nor/core.c @@ -2019,10 +2019,22 @@ void spi_nor_set_erase_type(struct spi_nor_erase_type *erase, u32 size, u8 opcode) { erase->size = size; - erase->opcode = opcode; - /* JEDEC JESD216B Standard imposes erase sizes to be power of 2. */ - erase->size_shift = ffs(erase->size) - 1; - erase->size_mask = (1 << erase->size_shift) - 1; + + if (size) { + erase->opcode = opcode; + /* JEDEC JESD216B imposes erase sizes to be power of 2. */ + erase->size_shift = ffs(size) - 1; + erase->size_mask = (1 << erase->size_shift) - 1; + } +} + +/** + * spi_nor_mask_erase_type() - mask out an SPI NOR erase type + * @erase: pointer to a structure that describes a SPI NOR erase type + */ +void spi_nor_mask_erase_type(struct spi_nor_erase_type *erase) +{ + erase->size = 0; } /** diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h index f6d012e1f681..25423225c29d 100644 --- a/drivers/mtd/spi-nor/core.h +++ b/drivers/mtd/spi-nor/core.h @@ -681,6 +681,7 @@ void spi_nor_set_pp_settings(struct spi_nor_pp_command *pp, u8 opcode, void spi_nor_set_erase_type(struct spi_nor_erase_type *erase, u32 size, u8 opcode); +void spi_nor_mask_erase_type(struct spi_nor_erase_type *erase); struct spi_nor_erase_region * spi_nor_region_next(struct spi_nor_erase_region *region); void spi_nor_init_uniform_erase_map(struct spi_nor_erase_map *map, diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c index fd4daf8fa5df..298ab5e53a8c 100644 --- a/drivers/mtd/spi-nor/sfdp.c +++ b/drivers/mtd/spi-nor/sfdp.c @@ -875,7 +875,7 @@ static int spi_nor_init_non_uniform_erase_map(struct spi_nor *nor, */ for (i = 0; i < SNOR_ERASE_TYPE_MAX; i++) if (!(regions_erase_type & BIT(erase[i].idx))) - spi_nor_set_erase_type(&erase[i], 0, 0xFF); + spi_nor_mask_erase_type(&erase[i]); return 0; } @@ -1089,7 +1089,7 @@ static int spi_nor_parse_4bait(struct spi_nor *nor, erase_type[i].opcode = (dwords[SFDP_DWORD(2)] >> erase_type[i].idx * 8) & 0xFF; else - spi_nor_set_erase_type(&erase_type[i], 0u, 0xFF); + spi_nor_mask_erase_type(&erase_type[i]); } /*
spi_nor_set_erase_type() was used either to set or to mask out an erase type. When we used it to mask out an erase type a shift-out-of-bounds was hit: UBSAN: shift-out-of-bounds in drivers/mtd/spi-nor/core.c:2237:24 shift exponent 4294967295 is too large for 32-bit type 'int' size_shift and size_mask were not used anyway when erase size was set to zero, so the functionality was not affected. The setting of the size_{shift, mask} and of the opcode are unnecessary when the erase size is zero, as throughout the code just the erase size is considered to determine whether an erase type is supported or not. Setting the opcode to 0xFF was wrong too as nobody guarantees that 0xFF is an unused opcode. Thus condition the setting of these params by the erase size. This will fix the shift-out-of-bounds too. While here avoid a superfluous dereference and use 'size' directly. Fixes: 5390a8df769e ("mtd: spi-nor: add support to non-uniform SFDP SPI NOR flash memories") Cc: stable@vger.kernel.org Reported-by: Alexander Stein <Alexander.Stein@tq-group.com> Suggested-by: Louis Rannou <lrannou@baylibre.com> Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org> --- v1 at https://lore.kernel.org/r/20211106075616.95401-1-tudor.ambarus@microchip.com/ drivers/mtd/spi-nor/core.c | 20 ++++++++++++++++---- drivers/mtd/spi-nor/core.h | 1 + drivers/mtd/spi-nor/sfdp.c | 4 ++-- 3 files changed, 19 insertions(+), 6 deletions(-)