Re: [PATCH] mtd: nand: bbt: clamp GENMASK high bit to word boundary

[Miquel Raynal]

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