Re: [PATCH 05/20] btrfs: handle invalid root reference found in btrfs_find_root()

David Sterba
2024-01-29T15:57:22Z

On Fri, Jan 26, 2024 at 08:06:06PM +0800, Anand Jain wrote:
> On 1/25/24 05:18, David Sterba wrote:
> > The btrfs_find_root() looks up a root 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 allowed the range of a root id.
> >
> > Signed-off-by: David Sterba <dsterba@suse.com>
> > ---
> > fs/btrfs/root-tree.c | 9 ++++++++-
> > 1 file changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/fs/btrfs/root-tree.c b/fs/btrfs/root-tree.c
> > index ba7e2181ff4e..326cd0d03287 100644
> > --- a/fs/btrfs/root-tree.c
> > +++ b/fs/btrfs/root-tree.c
> > @@ -82,7 +82,14 @@ int btrfs_find_root(struct btrfs_root *root, const struct btrfs_key *search_key,
> > if (ret > 0)
> > goto out;
> > } else {
> > - BUG_ON(ret == 0); /* Logical error */
> > + /*
> > + * Key with offset -1 found, there would have to exist a root
> > + * with such id, but this is out of the valid range.
> > + */
> > + if (ret == 0) {
> > + ret = -EUCLEAN;
> > + goto out;
> > + }
> > if (path->slots[0] == 0)
> > goto out;
> > path->slots[0]--;
>
>
> While here, why not also add an error message, especially for calls
> from btrfs_read_roots() when the IGNOREBADROOTS is set, we ignore
> the error and continue without the abort(). Including an error
> message will provide more information about the bad root.

Yeah, after some thoughs i agree that the messages would be good, self
documenting instead of the comments. I'll do that in another series as
we first need some common way of handling it and wrappers.