From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 477E2C4332F for ; Tue, 12 Apr 2022 10:23:46 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1353127AbiDLKXT (ORCPT ); Tue, 12 Apr 2022 06:23:19 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:60036 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1380630AbiDLKUw (ORCPT ); Tue, 12 Apr 2022 06:20:52 -0400 Received: from mail-ej1-x630.google.com (mail-ej1-x630.google.com [IPv6:2a00:1450:4864:20::630]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 1550D4C43E for ; Tue, 12 Apr 2022 02:25:40 -0700 (PDT) Received: by mail-ej1-x630.google.com with SMTP id bh17so36067632ejb.8 for ; Tue, 12 Apr 2022 02:25:40 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=X3YNHPaCJ0j6J7AgyeqSNpvtgtUHrIrd+cUv59gmDdw=; b=VDmPWsFhmRDehPDyMetWshMjV0aZT1SEh3ZvGVJlflnm8d84DzneF+d9o2XKkbbkOJ /lvK8KKFLduChzyolt0Ln3fsQHpjlFOlwNPW0h945g1CRhI7iiDEtkLU/nsZKJUC9KiR 8DQncwuLk6UdZA/HGp0Ve/2q+uovgoLrD4RB9EbHiVswxU1MN7un0J7cbvnfcFAzTaxS MpKfhWbdjYjS1ufX/R4/k7ZB1xwawDdO21viIotcT4DWbpzR6zukClEn+PkmYCZwynKw DTArIWvs1+fSxPGUpZki8xbuQrNRtW3vhoUti9/7UqU88slSL1ifx3ThSeBAD05ZcEzW MBdQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=X3YNHPaCJ0j6J7AgyeqSNpvtgtUHrIrd+cUv59gmDdw=; b=2bC2UUjXdohMcrPfKLGDRooTbtDzw8I8qFzz41CivSe1vcpmVvQ4r+2EJ3GOtzmsp+ W0+tyll2u+vL7EYSS68trple9pX0dDwQgwE3uR5eN7eD/RwPRXq1YYJ5emHsR6sT+yI0 U4XI+n0ECDrkjy6QcE5BZ1mxjrF+/vqPDzzqeaHvjQ73YZK1qEw11sDnZlKnMtYzBZl4 XQ52/NvvpAPE4qLLzSE84a20xoWj0rk6+8qvLgytYywdczcGj5X5BIVh5JwYoaKpeDlM iJ0V6W0aFKIfcINbmNuYIFUMUqjtSEJmX/89u6kbwtF5AgXrWMJmAuNOQzYYPSzkZMOT lzPA== X-Gm-Message-State: AOAM532Y5E7/03h2vfpvAeSduyhNPUqb2uYOGD/i7QNGN4eE4oyWUQPe lbr4LnwpkqU6pT88O3xpLMKJQmEX/YhBqTsXP9Q= X-Google-Smtp-Source: ABdhPJyOaUXGrfelcS+wdxAvMUvh4iGL4B43HTVL/dE8NDiNrkZdvrJKneNEM5Q6Hsisd4g2s0OHs2NgcPmFTA8u37Q= X-Received: by 2002:a17:906:1be1:b0:6ce:b0a8:17d with SMTP id t1-20020a1709061be100b006ceb0a8017dmr33419444ejg.413.1649755538517; Tue, 12 Apr 2022 02:25:38 -0700 (PDT) MIME-Version: 1.0 References: <20220408052207.209856-1-dzm91@hust.edu.cn> <7e896205-027d-32e2-32b5-8951231132ac@kernel.org> <2f5b7291-da06-0895-8e75-c7a37251b47e@kernel.org> In-Reply-To: From: Dongliang Mu Date: Tue, 12 Apr 2022 17:25:12 +0800 Message-ID: Subject: Re: [f2fs-dev] [PATCH] fs: f2fs: remove WARN_ON in f2fs_is_valid_blkaddr To: Chao Yu Cc: Eric Biggers , Dmitry Vyukov , Jaegeuk Kim , Dongliang Mu , linux-kernel , syzbot+763ae12a2ede1d99d4dc@syzkaller.appspotmail.com, linux-f2fs-devel@lists.sourceforge.net Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Apr 11, 2022 at 5:53 PM Chao Yu wrote: > > 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 wrote: > >>> > >>> On 2022/4/9 14:42, Dongliang Mu wrote: > >>>> On Sat, Apr 9, 2022 at 11:46 AM Chao Yu wrote: > >>>>> > >>>>> On 2022/4/9 9:34, Dongliang Mu wrote: > >>>>>> On Sat, Apr 9, 2022 at 8:27 AM Chao Yu wrote: > >>>>>>> > >>>>>>> On 2022/4/8 13:22, Dongliang Mu wrote: > >>>>>>>> From: Dongliang Mu > >>>>>>>> > >>>>>>>> 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? I agree. Let's change WARN_ON to dump_stack(). > > Thanks, > > > > > - Eric