| Message ID | Zr9XJJlZ+RzkLK/M@hostname (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | ocfs2: Fix shift-out-of-bounds UBSAN bug in ocfs2_verify_volume() | expand |
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() > > 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)); Thanks, Heming > } 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)) {
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
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)); } 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)) {