| Message ID | ZsPrbmbnAgGRgqLw@hostname (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] ocfs2: Fix shift-out-of-bounds UBSAN bug in ocfs2_verify_volume() | expand |
On 8/20/24 9:03 AM, qasdev wrote: > On Mon, Aug 19, 2024 at 10:52:29AM +0800, Joseph Qi wrote: >> >> >> On 8/18/24 7:43 PM, Heming Zhao wrote: >>> On 8/16/24 21:41, qasdev wrote: >>>> From ad1ca2fd2ecf4eb7ec2c76fcbbf34639f0ad87ca Mon Sep 17 00:00:00 2001 >>>> From: Qasim Ijaz <qasdev00@gmail.com> >>>> Date: Fri, 16 Aug 2024 02:30:25 +0100 >>>> Subject: [PATCH] ocfs2: Fix shift-out-of-bounds UBSAN bug in >>>> ocfs2_verify_volume() >>>> >> >> The above should be eliminated from patch body. >> >>>> This patch addresses a shift-out-of-bounds error in the >>>> ocfs2_verify_volume() function, identified by UBSAN. The bug was triggered >>>> by an invalid s_clustersize_bits value (e.g., 1548), which caused the >>>> expression "1 << le32_to_cpu(di->id2.i_super.s_clustersize_bits)" >>>> to exceed the limits of a 32-bit integer, >>>> leading to an out-of-bounds shift. >>>> >>>> Reported-by: syzbot <syzbot+f3fff775402751ebb471@syzkaller.appspotmail.com> >>>> Closes: https://syzkaller.appspot.com/bug?extid=f3fff775402751ebb471 >>>> Tested-by: syzbot <syzbot+f3fff775402751ebb471@syzkaller.appspotmail.com> >>>> Signed-off-by: Qasim Ijaz <qasdev00@gmail.com> >>>> --- >>>> fs/ocfs2/super.c | 8 ++++++-- >>>> 1 file changed, 6 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/fs/ocfs2/super.c b/fs/ocfs2/super.c >>>> index afee70125ae3..1e43cdca7f40 100644 >>>> --- a/fs/ocfs2/super.c >>>> +++ b/fs/ocfs2/super.c >>>> @@ -2357,8 +2357,12 @@ static int ocfs2_verify_volume(struct ocfs2_dinode *di, >>>> (unsigned long long)bh->b_blocknr); >>>> } else if (le32_to_cpu(di->id2.i_super.s_clustersize_bits) < 12 || >>>> le32_to_cpu(di->id2.i_super.s_clustersize_bits) > 20) { >>>> - mlog(ML_ERROR, "bad cluster size found: %u\n", >>>> - 1 << le32_to_cpu(di->id2.i_super.s_clustersize_bits)); >>>> + if (le32_to_cpu(di->id2.i_super.s_clustersize_bits) < 32) >>>> + mlog(ML_ERROR, "bad cluster size found: %u\n", >>>> + 1 << le32_to_cpu(di->id2.i_super.s_clustersize_bits)); >>>> + else >>>> + mlog(ML_ERROR, "invalid cluster size bit value: %u\n", >>>> + le32_to_cpu(di->id2.i_super.s_clustersize_bits)); >>> >>> I prefer to use concise code to fix the error. >>> Do you like below code? >>> - mlog(ML_ERROR, "bad cluster size found: %u\n", >>> - 1 << le32_to_cpu(di->id2.i_super.s_clustersize_bits)); >>> + mlog(ML_ERROR, "bad cluster size bit found: %u\n", >>> + le32_to_cpu(di->id2.i_super.s_clustersize_bits)); >>> >> >> Agree. qasdev, Could you please update and send v2? >> >> Thanks, >> Joseph > > Thanks for the feedback. After considering the input, I've refined the patch > to make it more concise. I've updated the patch and included it below: > Hi, please send v2 as a standalone thread. Thanks, Joseph
diff --git a/fs/ocfs2/super.c b/fs/ocfs2/super.c index afee70125ae3..b704983b2112 100644 --- a/fs/ocfs2/super.c +++ b/fs/ocfs2/super.c @@ -2357,8 +2357,8 @@ static int ocfs2_verify_volume(struct ocfs2_dinode *di, (unsigned long long)bh->b_blocknr); } else if (le32_to_cpu(di->id2.i_super.s_clustersize_bits) < 12 || le32_to_cpu(di->id2.i_super.s_clustersize_bits) > 20) { - mlog(ML_ERROR, "bad cluster size found: %u\n", - 1 << le32_to_cpu(di->id2.i_super.s_clustersize_bits)); + mlog(ML_ERROR, "bad cluster size bit found: %u\n", + le32_to_cpu(di->id2.i_super.s_clustersize_bits)); } else if (!le64_to_cpu(di->id2.i_super.s_root_blkno)) { mlog(ML_ERROR, "bad root_blkno: 0\n"); } else if (!le64_to_cpu(di->id2.i_super.s_system_dir_blkno)) {