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

[Anand Jain <anand.jain@oracle.com>]

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.

btrfs_read_roots()
::
  btrfs_read_tree_root() | btrfs_get_fs_root_commit_root() | 
load_global_roots_objectid()
  read_tree_root_path()
   btrfs_find_root()


Thanks, Anand