| Message ID | 20201220211037.1354-1-rdunlap@infradead.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RFC,1/2] log2: handle LARGE input to __roundup_pow_of_two() | expand |
On Sun, Dec 20, 2020 at 01:10:37PM -0800, Randy Dunlap wrote: > UBSAN detected a 64-bit shift in log2.h:__roundup_pow_of_two(): > UBSAN: shift-out-of-bounds in ./include/linux/log2.h:57:13 > shift exponent 64 is too large for 64-bit type 'long unsigned int' > > This is during a call from mm/readahead.c:ondemand_readahead(), > get_init_ra_size(), where the 'size' parameter must have been > extremely large (or "negative"). Actually, I think it was zero, which is the real bug that should be fixed.
On 12/20/20 1:23 PM, Matthew Wilcox wrote: > On Sun, Dec 20, 2020 at 01:10:37PM -0800, Randy Dunlap wrote: >> UBSAN detected a 64-bit shift in log2.h:__roundup_pow_of_two(): >> UBSAN: shift-out-of-bounds in ./include/linux/log2.h:57:13 >> shift exponent 64 is too large for 64-bit type 'long unsigned int' >> >> This is during a call from mm/readahead.c:ondemand_readahead(), >> get_init_ra_size(), where the 'size' parameter must have been >> extremely large (or "negative"). > > Actually, I think it was zero, which is the real bug that should be fixed. > Hm, OK, that would make more sense than some Huge value (other than -1). Do you mean something like this? --- --- mm/readahead.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) --- linux-5.10.1.orig/mm/readahead.c +++ linux-5.10.1/mm/readahead.c @@ -310,7 +310,11 @@ void force_page_cache_ra(struct readahea */ static unsigned long get_init_ra_size(unsigned long size, unsigned long max) { - unsigned long newsize = roundup_pow_of_two(size); + unsigned long newsize; + + if (!size) + size = 32; + newsize = roundup_pow_of_two(size); if (newsize <= max / 32) newsize = newsize * 4; Thanks.
--- linux-5.10.1.orig/include/linux/log2.h +++ linux-5.10.1/include/linux/log2.h @@ -54,7 +54,17 @@ bool is_power_of_2(unsigned long n) static inline __attribute__((const)) unsigned long __roundup_pow_of_two(unsigned long n) { - return 1UL << fls_long(n - 1); + unsigned int lastset = fls_long(n - 1); /* this can be 64 or 32 */ + + /* + * for high bit set (64 or 32), we can neither round up nor + * make it a power or 2 + */ + if ((sizeof(n) == 4 && lastset == 32) || + (sizeof(n) == 8 && lastset == 64)) + return n; + + return 1UL << lastset; } /**
UBSAN detected a 64-bit shift in log2.h:__roundup_pow_of_two(): UBSAN: shift-out-of-bounds in ./include/linux/log2.h:57:13 shift exponent 64 is too large for 64-bit type 'long unsigned int' This is during a call from mm/readahead.c:ondemand_readahead(), get_init_ra_size(), where the 'size' parameter must have been extremely large (or "negative"). fls() can legitimately return 32 or 64 when the MSbit is set in a 32-bit or 64-bit unsigned long. For these values, doing "1UL << shiftcout" is invalid or undefined, so catch when this happens. When the MSbit is 32 or 64, we cannot roundup to the next power of 2, so just return n (the input value), which is >= 0x8000...0000 and probably not a power of 2 (unless it is exactly 0x8000...0000). Signed-off-by: Randy Dunlap <rdunlap@infradead.org> Cc: Jens Axboe <axboe@kernel.dk> Cc: Andrew Morton <akpm@linux-foundation.org> Cc: Toralf Förster <toralf.foerster@gmx.de> Cc: linux-mm@kvack.org --- include/linux/log2.h | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-)