Re: [PATCH] md: do overflow check for sb->bblog_shift in super_1_load()

在 2026/7/20 19:14, [email protected] 写道:

> In super_1_load(), sb->bblog_shift is an __u8 type value loaded from on-
> disk superblock. It is used for badblocks API badblocks_set() by the
> following sequence,
>
>   1930   rdev->badblocks.shift = sb->bblog_shift;
>   1931   for (i = 0 ; i < (sectors << (9-3)) ; i++, bbp++) {
>   1932           u64 bb = le64_to_cpu(*bbp);
>   1933           int count = bb & (0x3ff);
>   1934           u64 sector = bb >> 10;
>   1935           sector <<= sb->bblog_shift;
>   1936           count <<= sb->bblog_shift;
>   1937           if (bb + 1 == 0)
>   1938                   break;
>   1939           if (!badblocks_set(&rdev->badblocks, sector, count, 1))
>   1940                   return -EINVAL;
>   1941   }
>
> bb->bblog_shit is in range of 0-255, variable sector is 64bit width, for
> an invalid bb->bblog_shit, it is possible to make sector be overflowed
> by the following calculation,
>   1935           sector <<= sb->bblog_shift;
> Then in turn when call badblocks_set() at line 1939 with the invalid
> rdev->badblocks.shift set at line 1930, may result an overflow inside
> _badblocks_clear() in block/badblocks.c.
>
> Although there are many places to call badblocks APIs, the non-zero
> shift value is only used in super_1_load(), other places always use 0 as
> the shift value. Therefore it is unnecessary to do a general shift value
> overflow check inside badblock API, and just check here as the caller.
>
> This may avoid unnecessary check, make the badblocks API code more simple
> and elegant.
>
> Fixes: 2699b67223aca ("md: load/store badblock list from v1.x metadata")
> Fixes: 1726c77467833 ("badblocks: improve badblocks_set() for multiple ranges handling")
> Cc:[email protected]
> Cc: Ramesh Adhikari<[email protected]>
> Signed-off-by: Coly Li<[email protected]>
> ---
>   drivers/md/md.c | 7 +++++++
>   1 file changed, 7 insertions(+)
Reviewed-by: Yu Kuai <[email protected]>

-- 
Thanks,
Kuai