Re: [PATCH] md: do overflow check for sb->bblog_shift in super_1_load()
"yu kuai" <[email protected]> Fri, 31 Jul 2026 16:00:02 +0800
| Newsgroups | org.kernel.vger.linux-raid,org.kernel.vger.linux-block,org.kernel.vger.stable |
|---|---|
| Message-ID | <[email protected]> |
=E5=9C=A8 2026/7/20 19:14, [email protected] =E5=86=99=E9=81=93: > From: Coly Li<[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 =3D sb->bblog_shift; > 1931 for (i =3D 0 ; i < (sectors << (9-3)) ; i++, bbp++) { > 1932 u64 bb =3D le64_to_cpu(*bbp); > 1933 int count =3D bb & (0x3ff); > 1934 u64 sector =3D bb >> 10; > 1935 sector <<=3D sb->bblog_shift; > 1936 count <<=3D sb->bblog_shift; > 1937 if (bb + 1 =3D=3D 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 <<=3D 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 ra= nges 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(+) Applied to md-7.3 --=20 Thanks, Kuai