Re: [PATCH 08/20] btrfs: handle invalid extent item reference found in check_committed_ref()

David Sterba
2024-01-25T16:31:59Z

On Thu, Jan 25, 2024 at 02:40:12PM +1030, Qu Wenruo wrote:
>
>
> On 2024/1/25 07:48, David Sterba wrote:
> > The check_committed_ref() helper looks up an extent item by a key,
> > allowing to do an inexact search when key->offset is -1. It's never
> > expected to find such item, as it would break the allowed range of a
> > extent item offset.
> >
> > Signed-off-by: David Sterba <dsterba@suse.com>
> > ---
> > fs/btrfs/extent-tree.c | 9 ++++++++-
> > 1 file changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> > index 4283e3025ab0..ba47f5996c84 100644
> > --- a/fs/btrfs/extent-tree.c
> > +++ b/fs/btrfs/extent-tree.c
> > @@ -2399,7 +2399,14 @@ static noinline int check_committed_ref(struct btrfs_root *root,
> > ret = btrfs_search_slot(NULL, extent_root, &key, path, 0, 0);
> > if (ret < 0)
> > goto out;
> > - BUG_ON(ret == 0); /* Corruption */
> > + if (ret == 0) {
> > + /*
> > + * Key with offset -1 found, there would have to exist an extent
> > + * item with such offset, but this is out of the valid range.
> > + */
> > + ret = -EUCLEAN;
> > + goto out;
> > + }
>
> It looks like we also need an key offset check for extent item.

We already have it (though I did not remeber that first),
check_extent_item() checks alignment or exact values. Why I don't want
that to be an assert is in my previous reply.