Hi-- On 4/29/22 10:39, Matthew Wilcox wrote:
On Fri, Apr 29, 2022 at 10:27:11AM -0700, Randy Dunlap wrote:When the NTFS BOOT sectors_per_clusters field is > 0x80, it represents a shift value. First change its sign to positive and then make sure that the shift count is not too large. This prevents negative shift values and shift values that are larger than the field size. Prevents this UBSAN error: UBSAN: shift-out-of-bounds in ../fs/ntfs3/super.c:673:16 shift exponent -192 is negative Fixes: 82cae269cfa9 ("fs/ntfs3: Add initialization of super block") Signed-off-by: Randy Dunlap<rdunlap@xxxxxxxxxxxxx> Reported-by:syzbot+1631f09646bc214d2e76@xxxxxxxxxxxxxxxxxxxxxxxxx Cc: Konstantin Komarov<almaz.alexandrovich@xxxxxxxxxxxxxxxxxxxx> Cc:ntfs3@xxxxxxxxxxxxxxx Cc: Alexander Viro<viro@xxxxxxxxxxxxxxxxxx> Cc: Andrew Morton<akpm@xxxxxxxxxxxxxxxxxxxx> Cc: Kari Argillander<kari.argillander@xxxxxxxxxxxxxxxxxxxx> Cc: Namjae Jeon<linkinjeon@xxxxxxxxxx> --- fs/ntfs3/super.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) --- linux-next-20220428.orig/fs/ntfs3/super.c +++ linux-next-20220428/fs/ntfs3/super.c @@ -670,7 +670,8 @@ static u32 true_sectors_per_clst(const s { return boot->sectors_per_clusters <= 0x80 ? boot->sectors_per_clusters - : (1u << (0 - boot->sectors_per_clusters)); + : -(s8)boot->sectors_per_clusters > 31 ? -1 + : (1u << -(s8)boot->sectors_per_clusters); }This hurts my brain. Can we do instead:
It's just C. Lot clearer than some of our macro magic.
if (boot->sectors_per_clusters <= 0x80) return boot->sectors_per_clusters; if (boot->sectors_per_clusters < 0xA0)
The 0xA0 does not take into account the '-' negating of sectors_per_clusters in the shift.
Looks like it should be if (boot->sectors_per_clusters >= 0xe1) return 1U << -boot->sectors_per_clusters;
return 1U << (boot->sectors_per_clusters - 0x80); return 0xffffffff;
Sorry about your head.
