Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20201003063143.13093-1-foxhlchen@gmail.com> <ca694fc8-5f1b-ebf0-b891-c9a20225704f@redhat.com>
In-Reply-To: <ca694fc8-5f1b-ebf0-b891-c9a20225704f@redhat.com>
From:   Fox Chen <foxhlchen@gmail.com>
Date:   Mon, 5 Oct 2020 22:08:04 +0800
Message-ID: <CAC2o3D+X48sR4kFi9nS_atpTuSRWDnX92RD3uCUeh9AHXGnaNw@mail.gmail.com>
Subject: Re: [Cluster-devel] [PATCH] gfs2: gfs2_read_sb: put gfs2_assert
 inside the loop
To:     Andrew Price <anprice@redhat.com>
Cc:     rpeterso@redhat.com, agruenba@redhat.com, cluster-devel@redhat.com,
        Greg KH <gregkh@linuxfoundation.org>,
        linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Mon, Oct 5, 2020 at 9:23 PM Andrew Price <anprice@redhat.com> wrote:
>
> On 03/10/2020 07:31, Fox Chen wrote:
> > for (x = 2;; x++) {
> >          ...
> >          gfs2_assert(sdp, x <= GFS2_MAX_META_HEIGHT);  <--- after
> >          ...
> >          if (d != sdp->sd_heightsize[x - 1] || m)
> >                  break;
> >          sdp->sd_heightsize[x] = space;
> > }
> >
> > sdp->sd_max_height = x
> > gfs2_assert(sdp, sdp->sd_max_height <= GFS2_MAX_META_HEIGHT) <--- before
> >
> > Before this patch, gfs2_assert is put outside of the loop of
> > sdp->sd_heightsize[x] calculation. When something goes wrong,
>
> So this looks related to one of the recent syzbot reports, where the
> "something goes wrong" is the block size in the on-disk superblock was
> zeroed and that leads eventually to this out-of-bounds write. The
> correct fix in that case would be to add a validity check for the block
> size in gfs2_check_sb().
>

Yes, I saw this bug from the syzbot report and I though instead of
KASAN gfs2_assert should be able to catch it so I proposed this patch.
:)

thank you both for your comments.


fox