Re: [PATCH v3] mtd: spi-nor: Fix shift-out-of-bounds in spi_nor_set_erase_type

[Stein, Alexander]

Hi Tudor,

Am Freitag, dem 03.02.2023 um 09:07 +0200 schrieb Tudor Ambarus:
> From: Louis Rannou <lrannou@xxxxxxxxxxxx>
> 
> 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'
> 
> 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 when masking out an erase type, just set
> the
> erase size to zero. This will fix the shift-out-of-bounds.
> 
> Fixes: 5390a8df769e ("mtd: spi-nor: add support to non-uniform SFDP
> SPI NOR flash memories")
> Cc: stable@xxxxxxxxxxxxxxx
> Reported-by: Alexander Stein <Alexander.Stein@xxxxxxxxxxxx>
> Signed-off-by: Louis Rannou <lrannou@xxxxxxxxxxxx>
> [ta: refine changes, new commit message, fix compilation error]
> Signed-off-by: Tudor Ambarus <tudor.ambarus@xxxxxxxxxx>

Thanks.
Tested-by: Alexander Stein <Alexander.Stein@xxxxxxxxxxxx>

> ---
>  drivers/mtd/spi-nor/core.c | 9 +++++++++
>  drivers/mtd/spi-nor/core.h | 1 +
>  drivers/mtd/spi-nor/sfdp.c | 4 ++--
>  3 files changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index 247d1014879a..22cb18b6c941 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -2025,6 +2025,15 @@ void spi_nor_set_erase_type(struct
> spi_nor_erase_type *erase, u32 size,
>         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 an SPI NOR
> erase type
> + */
> +void spi_nor_mask_erase_type(struct spi_nor_erase_type *erase)
> +{
> +       erase->size = 0;
> +}
> +
>  /**
>   * spi_nor_init_uniform_erase_map() - Initialize uniform erase map
>   * @map:               the erase map of the SPI NOR
> 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]);
>         }
>  
>         /*