From: Chao Yu <chao@kernel.org>
To: Eric Biggers <ebiggers@kernel.org>,
Dongliang Mu <mudongliangabcd@gmail.com>
Cc: Dmitry Vyukov <dvyukov@google.com>,
Jaegeuk Kim <jaegeuk@kernel.org>,
Dongliang Mu <dzm91@hust.edu.cn>,
linux-kernel <linux-kernel@vger.kernel.org>,
syzbot+763ae12a2ede1d99d4dc@syzkaller.appspotmail.com,
linux-f2fs-devel@lists.sourceforge.net
Subject: Re: [f2fs-dev] [PATCH] fs: f2fs: remove WARN_ON in f2fs_is_valid_blkaddr
Date: Mon, 11 Apr 2022 17:53:33 +0800 [thread overview]
Message-ID: <a25bac9c-188b-4ab4-21e2-d34e20c92524@kernel.org> (raw)
In-Reply-To: <YlPHWaSlk21cc32p@sol.localdomain>
On 2022/4/11 14:14, Eric Biggers wrote:
> 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@kernel.org> wrote:
>>>
>>> On 2022/4/9 14:42, Dongliang Mu wrote:
>>>> On Sat, Apr 9, 2022 at 11:46 AM Chao Yu <chao@kernel.org> wrote:
>>>>>
>>>>> On 2022/4/9 9:34, Dongliang Mu wrote:
>>>>>> On Sat, Apr 9, 2022 at 8:27 AM Chao Yu <chao@kernel.org> wrote:
>>>>>>>
>>>>>>> On 2022/4/8 13:22, Dongliang Mu wrote:
>>>>>>>> From: Dongliang Mu <mudongliangabcd@gmail.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.
Alright, so how about using dump_stack() instead as suggested in doc?
Thanks,
>
> - Eric
next prev parent reply other threads:[~2022-04-11 9:54 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-04-08 5:22 [PATCH] fs: f2fs: remove WARN_ON in f2fs_is_valid_blkaddr Dongliang Mu
2022-04-09 0:27 ` Chao Yu
2022-04-09 1:34 ` Dongliang Mu
2022-04-09 3:46 ` Chao Yu
2022-04-09 6:42 ` Dongliang Mu
2022-04-11 3:09 ` [f2fs-dev] " Chao Yu
2022-04-11 5:06 ` Dongliang Mu
2022-04-11 6:14 ` Eric Biggers
2022-04-11 9:53 ` Chao Yu [this message]
2022-04-12 9:25 ` Dongliang Mu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=a25bac9c-188b-4ab4-21e2-d34e20c92524@kernel.org \
--to=chao@kernel.org \
--cc=dvyukov@google.com \
--cc=dzm91@hust.edu.cn \
--cc=ebiggers@kernel.org \
--cc=jaegeuk@kernel.org \
--cc=linux-f2fs-devel@lists.sourceforge.net \
--cc=linux-kernel@vger.kernel.org \
--cc=mudongliangabcd@gmail.com \
--cc=syzbot+763ae12a2ede1d99d4dc@syzkaller.appspotmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).