* [PATCH] ocfs2: Fix shift-out-of-bounds UBSAN bug in ocfs2_verify_volume() @ 2024-08-16 13:41 qasdev 2024-08-18 11:43 ` Heming Zhao 0 siblings, 1 reply; 5+ messages in thread From: qasdev @ 2024-08-16 13:41 UTC (permalink / raw) To: mark, jlbec, joseph.qi; +Cc: ocfs2-devel, linux-kernel 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)); } 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)) { -- 2.30.2 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ocfs2: Fix shift-out-of-bounds UBSAN bug in ocfs2_verify_volume() 2024-08-16 13:41 [PATCH] ocfs2: Fix shift-out-of-bounds UBSAN bug in ocfs2_verify_volume() qasdev @ 2024-08-18 11:43 ` Heming Zhao 2024-08-19 2:52 ` Joseph Qi 0 siblings, 1 reply; 5+ messages in thread From: Heming Zhao @ 2024-08-18 11:43 UTC (permalink / raw) To: qasdev, mark, jlbec, joseph.qi; +Cc: ocfs2-devel, linux-kernel 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)) { ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ocfs2: Fix shift-out-of-bounds UBSAN bug in ocfs2_verify_volume() 2024-08-18 11:43 ` Heming Zhao @ 2024-08-19 2:52 ` Joseph Qi 2024-08-20 1:03 ` [PATCH v2] " qasdev 0 siblings, 1 reply; 5+ messages in thread From: Joseph Qi @ 2024-08-19 2:52 UTC (permalink / raw) To: Heming Zhao, qasdev, mark, jlbec; +Cc: ocfs2-devel, linux-kernel 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2] ocfs2: Fix shift-out-of-bounds UBSAN bug in ocfs2_verify_volume() 2024-08-19 2:52 ` Joseph Qi @ 2024-08-20 1:03 ` qasdev 2024-08-20 1:12 ` Joseph Qi 0 siblings, 1 reply; 5+ messages in thread From: qasdev @ 2024-08-20 1:03 UTC (permalink / raw) To: Joseph Qi, Heming Zhao, mark, jlbec; +Cc: ocfs2-devel, linux-kernel 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: 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 | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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)) { -- 2.30.2 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] ocfs2: Fix shift-out-of-bounds UBSAN bug in ocfs2_verify_volume() 2024-08-20 1:03 ` [PATCH v2] " qasdev @ 2024-08-20 1:12 ` Joseph Qi 0 siblings, 0 replies; 5+ messages in thread From: Joseph Qi @ 2024-08-20 1:12 UTC (permalink / raw) To: qasdev, Heming Zhao, mark, jlbec; +Cc: ocfs2-devel, linux-kernel 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-08-20 1:12 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2024-08-16 13:41 [PATCH] ocfs2: Fix shift-out-of-bounds UBSAN bug in ocfs2_verify_volume() qasdev 2024-08-18 11:43 ` Heming Zhao 2024-08-19 2:52 ` Joseph Qi 2024-08-20 1:03 ` [PATCH v2] " qasdev 2024-08-20 1:12 ` Joseph Qi
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
Powered by JetHome