| Message ID | 20230221111346.34268-1-andrea.righi@canonical.com |
|---|---|
| State | Not Applicable |
| Delegated to: | Ambarus Tudor |
| Headers | show |
| Series | mtd: spi-nor: fix shift-out-of-bounds in spi_nor_set_erase_type() | expand |
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; > + } > } > > /**
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 --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; + } } /**
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(-)