Re: [PATCH 1/2] btrfs: fix BUG_ON with proper error handle in find_next_devid

[Qu Wenruo <quwenruo.btrfs@gmx.com>]

[-- Attachment #1.1: Type: text/plain, Size: 1366 bytes --]



On 2019/8/27 下午3:40, Anand Jain wrote:
> In a corrupted tree if search for next devid finds the device with
> devid = -1, then report the error -EUCLEAN back to the parent
> function to fail gracefully.
> 
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
>  fs/btrfs/volumes.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index 4db4a100c05b..36aa5f79fb6c 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -1849,7 +1849,12 @@ static noinline int find_next_devid(struct btrfs_fs_info *fs_info,
>  	if (ret < 0)
>  		goto error;
>  
> -	BUG_ON(ret == 0); /* Corruption */
> +	if (ret == 0) {
> +		/* Corruption */
> +		btrfs_err(fs_info, "corrupted chunk tree devid -1 matched");

It will never hit this branch.

As in tree checker, we have checked if the devid is so large that a
chunk item or system chunk array can't contain one.

That limit is way smaller than (u64)-1.
Thus if we really have a key (DEV_ITEMS DEV_ITEM -1), it will be
rejected by tree-checker in the first place, thus you will get a ret ==
-EUCLEAN from previous btrfs_search_slot() call.

Thanks,
Qu
> +		ret = -EUCLEAN;
> +		goto error;
> +	}
>  
>  	ret = btrfs_previous_item(fs_info->chunk_root, path,
>  				  BTRFS_DEV_ITEMS_OBJECTID,
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]