diff mbox series

mtd: spi-nor: fix shift-out-of-bounds in spi_nor_set_erase_type()

Message ID 20230221111346.34268-1-andrea.righi@canonical.com
State New
Headers
Series mtd: spi-nor: fix shift-out-of-bounds in spi_nor_set_erase_type() |

Commit Message

Andrea Righi Feb. 21, 2023, 11:13 a.m. UTC 
  It seems that according to JEDEC JESD216B Standard erase size needs to
be a power of 2, but sometimes we set the size to 0 (e.g., in
spi_nor_parse_4bait()) causing UBSAN warnings like the following:

  UBSAN: shift-out-of-bounds in drivers/mtd/spi-nor/core.c:2026:24
  shift exponent 4294967295 is too large for 32-bit type 'int'
  Hardware name: Dell Inc. XPS 13 9300/077Y9N, BIOS 1.11.0 03/22/2022
  Call Trace:
   <TASK>
   show_stack+0x4e/0x61
   dump_stack_lvl+0x4a/0x6f
   dump_stack+0x10/0x18
   ubsan_epilogue+0x9/0x3a
   __ubsan_handle_shift_out_of_bounds.cold+0x61/0xef
   spi_nor_set_erase_type.cold+0x16/0x1e [spi_nor]
   spi_nor_parse_4bait+0x270/0x380 [spi_nor]
   spi_nor_parse_sfdp+0x47f/0x610 [spi_nor]

Fix by checking if size is a power when setting struct
spi_nor_erase_type, otherwise consider size, mask and shift as invalid.

Fixes: 5390a8df769e ("mtd: spi-nor: add support to non-uniform SFDP SPI NOR flash memories")
Reported-by: Emil Renner Berthing <emil.renner.berthing@canonical.com>
Signed-off-by: Andrea Righi <andrea.righi@canonical.com>
---
 drivers/mtd/spi-nor/core.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

Comments

Tudor Ambarus Feb. 22, 2023, 2:50 p.m. UTC | #1 
Hi!

This should be fixed by:
https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git/commit/?h=spi-nor/next&id=f0f0cfdc3a024e21161714f2e05f0df3b84d42ad

Which base did you use?

Cheers,
ta

On 2/21/23 11:13, Andrea Righi wrote:
> It seems that according to JEDEC JESD216B Standard erase size needs to
> be a power of 2, but sometimes we set the size to 0 (e.g., in
> spi_nor_parse_4bait()) causing UBSAN warnings like the following:
> 
>    UBSAN: shift-out-of-bounds in drivers/mtd/spi-nor/core.c:2026:24
>    shift exponent 4294967295 is too large for 32-bit type 'int'
>    Hardware name: Dell Inc. XPS 13 9300/077Y9N, BIOS 1.11.0 03/22/2022
>    Call Trace:
>     <TASK>
>     show_stack+0x4e/0x61
>     dump_stack_lvl+0x4a/0x6f
>     dump_stack+0x10/0x18
>     ubsan_epilogue+0x9/0x3a
>     __ubsan_handle_shift_out_of_bounds.cold+0x61/0xef
>     spi_nor_set_erase_type.cold+0x16/0x1e [spi_nor]
>     spi_nor_parse_4bait+0x270/0x380 [spi_nor]
>     spi_nor_parse_sfdp+0x47f/0x610 [spi_nor]
> 
> Fix by checking if size is a power when setting struct
> spi_nor_erase_type, otherwise consider size, mask and shift as invalid.
> 
> Fixes: 5390a8df769e ("mtd: spi-nor: add support to non-uniform SFDP SPI NOR flash memories")
> Reported-by: Emil Renner Berthing <emil.renner.berthing@canonical.com>
> Signed-off-by: Andrea Righi <andrea.righi@canonical.com>
> ---
>   drivers/mtd/spi-nor/core.c | 12 +++++++++---
>   1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index d67c926bca8b..3c5b5bf9cbd1 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -2019,11 +2019,17 @@ spi_nor_spimem_adjust_hwcaps(struct spi_nor *nor, u32 *hwcaps)
>   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 (likely(is_power_of_2(size))) {
> +		erase->size = size;
> +		erase->size_shift = ffs(erase->size) - 1;
> +		erase->size_mask = (1 << erase->size_shift) - 1;
> +	} else {
> +		erase->size = 0u;
> +		erase->size_shift = ~0u;
> +		erase->size_mask = ~0u;
> +	}
>   }
>   
>   /**
Andrea Righi Feb. 22, 2023, 3:37 p.m. UTC | #2 
On Wed, Feb 22, 2023 at 02:50:42PM +0000, Tudor Ambarus wrote:
> Hi!
> 
> This should be fixed by:
> https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git/commit/?h=spi-nor/next&id=f0f0cfdc3a024e21161714f2e05f0df3b84d42ad

Ah yes, this should definitely fix it.

It'd still like to do a check like is_power_of_2(size) in
spi_nor_set_erase_type(), to make sure we don't silently set size to
something non-standard, but I can send another patch for that, so for
now you can ignore this patch.

> 
> Which base did you use?

I used the latest git from Linus' repository that doesn't have this
commit yet.

Thanks,
-Andrea

> 
> Cheers,
> ta
> 
> On 2/21/23 11:13, Andrea Righi wrote:
> > It seems that according to JEDEC JESD216B Standard erase size needs to
> > be a power of 2, but sometimes we set the size to 0 (e.g., in
> > spi_nor_parse_4bait()) causing UBSAN warnings like the following:
> > 
> >    UBSAN: shift-out-of-bounds in drivers/mtd/spi-nor/core.c:2026:24
> >    shift exponent 4294967295 is too large for 32-bit type 'int'
> >    Hardware name: Dell Inc. XPS 13 9300/077Y9N, BIOS 1.11.0 03/22/2022
> >    Call Trace:
> >     <TASK>
> >     show_stack+0x4e/0x61
> >     dump_stack_lvl+0x4a/0x6f
> >     dump_stack+0x10/0x18
> >     ubsan_epilogue+0x9/0x3a
> >     __ubsan_handle_shift_out_of_bounds.cold+0x61/0xef
> >     spi_nor_set_erase_type.cold+0x16/0x1e [spi_nor]
> >     spi_nor_parse_4bait+0x270/0x380 [spi_nor]
> >     spi_nor_parse_sfdp+0x47f/0x610 [spi_nor]
> > 
> > Fix by checking if size is a power when setting struct
> > spi_nor_erase_type, otherwise consider size, mask and shift as invalid.
> > 
> > Fixes: 5390a8df769e ("mtd: spi-nor: add support to non-uniform SFDP SPI NOR flash memories")
> > Reported-by: Emil Renner Berthing <emil.renner.berthing@canonical.com>
> > Signed-off-by: Andrea Righi <andrea.righi@canonical.com>
> > ---
> >   drivers/mtd/spi-nor/core.c | 12 +++++++++---
> >   1 file changed, 9 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> > index d67c926bca8b..3c5b5bf9cbd1 100644
> > --- a/drivers/mtd/spi-nor/core.c
> > +++ b/drivers/mtd/spi-nor/core.c
> > @@ -2019,11 +2019,17 @@ spi_nor_spimem_adjust_hwcaps(struct spi_nor *nor, u32 *hwcaps)
> >   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 (likely(is_power_of_2(size))) {
> > +		erase->size = size;
> > +		erase->size_shift = ffs(erase->size) - 1;
> > +		erase->size_mask = (1 << erase->size_shift) - 1;
> > +	} else {
> > +		erase->size = 0u;
> > +		erase->size_shift = ~0u;
> > +		erase->size_mask = ~0u;
> > +	}
> >   }
> >   /**
diff mbox series

Patch

diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index d67c926bca8b..3c5b5bf9cbd1 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -2019,11 +2019,17 @@  spi_nor_spimem_adjust_hwcaps(struct spi_nor *nor, u32 *hwcaps)
 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 (likely(is_power_of_2(size))) {
+		erase->size = size;
+		erase->size_shift = ffs(erase->size) - 1;
+		erase->size_mask = (1 << erase->size_shift) - 1;
+	} else {
+		erase->size = 0u;
+		erase->size_shift = ~0u;
+		erase->size_mask = ~0u;
+	}
 }
 
 /**