Messages in this thread | | | Date | Mon, 11 Apr 2022 17:53:33 +0800 | Subject | Re: [f2fs-dev] [PATCH] fs: f2fs: remove WARN_ON in f2fs_is_valid_blkaddr | From | Chao Yu <> |
| |
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
| |