* [PATCH] mtd: spi-nor: fix shift-out-of-bounds in spi_nor_set_erase_type()
@ 2023-02-21 11:13 Andrea Righi
2023-02-22 14:50 ` Tudor Ambarus
0 siblings, 1 reply; 3+ messages in thread
From: Andrea Righi @ 2023-02-21 11:13 UTC (permalink / raw)
To: Tudor Ambarus, Pratyush Yadav
Cc: Michael Walle, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, Cristian Birsan, Boris Brezillon,
Emil Renner Berthing, linux-mtd, linux-kernel
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;
+ }
}
/**
--
2.38.1
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] mtd: spi-nor: fix shift-out-of-bounds in spi_nor_set_erase_type()
2023-02-21 11:13 [PATCH] mtd: spi-nor: fix shift-out-of-bounds in spi_nor_set_erase_type() Andrea Righi
@ 2023-02-22 14:50 ` Tudor Ambarus
2023-02-22 15:37 ` Andrea Righi
0 siblings, 1 reply; 3+ messages in thread
From: Tudor Ambarus @ 2023-02-22 14:50 UTC (permalink / raw)
To: Andrea Righi, Pratyush Yadav
Cc: Michael Walle, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, Cristian Birsan, Boris Brezillon,
Emil Renner Berthing, linux-mtd, linux-kernel
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;
> + }
> }
>
> /**
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] mtd: spi-nor: fix shift-out-of-bounds in spi_nor_set_erase_type()
2023-02-22 14:50 ` Tudor Ambarus
@ 2023-02-22 15:37 ` Andrea Righi
0 siblings, 0 replies; 3+ messages in thread
From: Andrea Righi @ 2023-02-22 15:37 UTC (permalink / raw)
To: Tudor Ambarus
Cc: Pratyush Yadav, Michael Walle, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, Cristian Birsan, Boris Brezillon,
Emil Renner Berthing, linux-mtd, linux-kernel
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;
> > + }
> > }
> > /**
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-02-22 15:38 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-02-21 11:13 [PATCH] mtd: spi-nor: fix shift-out-of-bounds in spi_nor_set_erase_type() Andrea Righi
2023-02-22 14:50 ` Tudor Ambarus
2023-02-22 15:37 ` Andrea Righi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox