linux-kernel - Re: [f2fs-dev] [PATCH] fs: f2fs: remove WARN_ON in f2fs_is_valid

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <YlPHWaSlk21cc32p@sol.localdomain>
Date:   Sun, 10 Apr 2022 23:14:49 -0700
From:   Eric Biggers <ebiggers@...nel.org>
To:     Dongliang Mu <mudongliangabcd@...il.com>
Cc:     Chao Yu <chao@...nel.org>, Dmitry Vyukov <dvyukov@...gle.com>,
        Jaegeuk Kim <jaegeuk@...nel.org>,
        Dongliang Mu <dzm91@...t.edu.cn>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        syzbot+763ae12a2ede1d99d4dc@...kaller.appspotmail.com,
        linux-f2fs-devel@...ts.sourceforge.net
Subject: Re: [f2fs-dev] [PATCH] fs: f2fs: remove WARN_ON in
 f2fs_is_valid_blkaddr

On Mon, Apr 11, 2022 at 01:06:09PM +0800, Dongliang Mu wrote:
> On Mon, Apr 11, 2022 at 11:10 AM Chao Yu <chao@...nel.org> wrote:
> >
> > On 2022/4/9 14:42, Dongliang Mu wrote:
> > > On Sat, Apr 9, 2022 at 11:46 AM Chao Yu <chao@...nel.org> wrote:
> > >>
> > >> On 2022/4/9 9:34, Dongliang Mu wrote:
> > >>> On Sat, Apr 9, 2022 at 8:27 AM Chao Yu <chao@...nel.org> wrote:
> > >>>>
> > >>>> On 2022/4/8 13:22, Dongliang Mu wrote:
> > >>>>> From: Dongliang Mu <mudongliangabcd@...il.com>
> > >>>>>
> > >>>>> In f2fs_is_valid_blkaddr, if type is DATA_GENERIC_ENHANCE or
> > >>>>> DATA_GENERIC_ENHANCE_READ, it invokes WARN_ON(1) not matter
> > >>>>> blkaddr is in the range or not.
> > >>>>
> > >>>> If we run into the path where we invoke WARN_ON(1) in f2fs_is_valid_blkaddr(),
> > >>>> It means f2fs image may be broken, or there is a bug in f2fs.
> > >>>>
> > >>>> So, do you suffer any related issue in your environment?
> > >>>
> > >>> related issue? Can you explain a little?
> > >>>
> > >>> If you mean if this warning occurs, any other issues or crash
> > >>
> > >> I mean have you seen any warning info printed in the path of
> > >> f2fs_is_valid_blkaddr() before applying this patch, and if so, w/ what
> > >> reproducer? or you just figure out this patch from perspective of code
> > >> review?
> > >
> > > Yes, I have seen both warning information from Syzbot [1] and my local
> > > syzkaller instance.
> > >
> > > In f2fs_is_valid_blkaddr, if the following condition is satisfied,
> > > i.e., blkaddr is not in the right range [2], it will directly invoke
> > > one WARN_ON.
> > >
> > > if (unlikely(blkaddr >= MAX_BLKADDR(sbi) ||
> > >                   blkaddr < MAIN_BLKADDR(sbi))) {
> > >
> > > This is the case on Syzbot.
> > >
> > > Otherwise, it will jump into __is_bitmap_valid. And if the following
> > > condition is satisfied [3], it will trigger another WARN_ON.
> > >
> > > exist = f2fs_test_bit(offset, se->cur_valid_map);
> > > if (!exist && type == DATA_GENERIC_ENHANCE) {
> > >
> > > This appears in my local syzbot instance, but unfortunately it does
> > > not get any reproducer.
> >
> > Oh, it occurs in syzbot test, I guess it is possible that f2fs prints such
> > warning info after blkaddr of node/data block was fuzzed to invalid one.
> >
> > I prefer to keep WARN_ON() to catch more info of bugs found by non-fuzzed
> > type test.
> >
> > Thoughts?
> 
> I am fine with both options. I can remove the WARN_ON in my local
> syzkaller instance and continue fuzzing Linux kernel.
> 
> +Dmitry Vyukov how do you think? If WARN_ON is kept, this crash will
> occur on Syzbot from time to time.

WARN_ON is for kernel bugs; please refer to the documentation in
include/asm-generic/bug.h.  If this is a kernel bug, then the kernel bug needs
to be fixed.  Otherwise, the WARN_ON needs to be removed.

- Eric