drivers/mtd/nand/bbt.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
When a BBT entry straddles an unsigned long boundary, the GENMASK in
nanddev_bbt_set_block_status() can potentially overflow because
offs + bits_per_block - 1 can theoretically exceed BITS_PER_LONG - 1.
Clamp the high bit so only bits within the current word are masked.
The cross-word portion is already handled by the pos[1] block below.
Discovered by UBSAN: shift-out-of-bounds in
drivers/mtd/nand/bbt.c:116:13
shift exponent 18446744073709551614 is too large for 64-bit type
'long unsigned int'
Fixes: 9c3736a3de21 ("mtd: nand: Add core infrastructure to deal with NAND devices")
Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
drivers/mtd/nand/bbt.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/mtd/nand/bbt.c b/drivers/mtd/nand/bbt.c
index db4f93a903e48..dfe4a6a991c15 100644
--- a/drivers/mtd/nand/bbt.c
+++ b/drivers/mtd/nand/bbt.c
@@ -113,7 +113,8 @@ int nanddev_bbt_set_block_status(struct nand_device *nand, unsigned int entry,
if (entry >= nanddev_neraseblocks(nand))
return -ERANGE;
- pos[0] &= ~GENMASK(offs + bits_per_block - 1, offs);
+ pos[0] &= ~GENMASK(min(offs + bits_per_block - 1,
+ BITS_PER_LONG - 1), offs);
pos[0] |= val << offs;
if (bits_per_block + offs > BITS_PER_LONG) {
--
2.53.0
Hi Daniel, On 12/04/2026 at 01:05:23 +01, Daniel Golle <daniel@makrotopia.org> wrote: > When a BBT entry straddles an unsigned long boundary, the GENMASK in > nanddev_bbt_set_block_status() can potentially overflow because > offs + bits_per_block - 1 can theoretically exceed BITS_PER_LONG - 1. > Clamp the high bit so only bits within the current word are masked. > The cross-word portion is already handled by the pos[1] block below. > > Discovered by UBSAN: shift-out-of-bounds in > drivers/mtd/nand/bbt.c:116:13 > shift exponent 18446744073709551614 is too large for 64-bit type > 'long unsigned int' How likely is that? It doesn't matter how many bits you use per blocks (today is 2), it would require a NAND chip that covers an entire country to reach that number of blocks. If an attacker plays with that value, does it really matter? Apart from writing out of bounds -which is physically impossible, we are not talking about virtual memory here- and get an error later on, I do not see a good reason for this. Honestly, I find the final result much less readable than before for no obvious added value IMO. But maybe I am looking at this the wrong way? Thanks, Miquèl
On Mon, Apr 13, 2026 at 10:12:10AM +0200, Miquel Raynal wrote: > Hi Daniel, > > On 12/04/2026 at 01:05:23 +01, Daniel Golle <daniel@makrotopia.org> wrote: > > > When a BBT entry straddles an unsigned long boundary, the GENMASK in > > nanddev_bbt_set_block_status() can potentially overflow because > > offs + bits_per_block - 1 can theoretically exceed BITS_PER_LONG - 1. > > Clamp the high bit so only bits within the current word are masked. > > The cross-word portion is already handled by the pos[1] block below. > > > > Discovered by UBSAN: shift-out-of-bounds in > > drivers/mtd/nand/bbt.c:116:13 > > shift exponent 18446744073709551614 is too large for 64-bit type > > 'long unsigned int' > > How likely is that? It doesn't matter how many bits you use per blocks > (today is 2), it would require a NAND chip that covers an entire country > to reach that number of blocks. If an attacker plays with that value, > does it really matter? Apart from writing out of bounds -which is > physically impossible, we are not talking about virtual memory here- and > get an error later on, I do not see a good reason for this. > > Honestly, I find the final result much less readable than before for no > obvious added value IMO. But maybe I am looking at this the wrong way? It's just the only UBSAN warning I get to see on a recent kernel and my primary goal here was to make the warning go away. Adding an assertion to ensure 'offs' is clamped to will likely also make the warning go away.
Hi Daniel, >> > When a BBT entry straddles an unsigned long boundary, the GENMASK in >> > nanddev_bbt_set_block_status() can potentially overflow because >> > offs + bits_per_block - 1 can theoretically exceed BITS_PER_LONG - 1. >> > Clamp the high bit so only bits within the current word are masked. >> > The cross-word portion is already handled by the pos[1] block below. >> > >> > Discovered by UBSAN: shift-out-of-bounds in >> > drivers/mtd/nand/bbt.c:116:13 >> > shift exponent 18446744073709551614 is too large for 64-bit type >> > 'long unsigned int' >> >> How likely is that? It doesn't matter how many bits you use per blocks >> (today is 2), it would require a NAND chip that covers an entire country >> to reach that number of blocks. If an attacker plays with that value, >> does it really matter? Apart from writing out of bounds -which is >> physically impossible, we are not talking about virtual memory here- and >> get an error later on, I do not see a good reason for this. >> >> Honestly, I find the final result much less readable than before for no >> obvious added value IMO. But maybe I am looking at this the wrong way? > > It's just the only UBSAN warning I get to see on a recent kernel and my > primary goal here was to make the warning go away. Adding an assertion > to ensure 'offs' is clamped to will likely also make the warning go > away. I believe that's a more appropriate approach, if you don't mind. Thanks, Miquèl
© 2016 - 2026 Red Hat, Inc.