Re: [PATCH v4] btrfs: trim: fix underflow in trim length to prevent access beyond device boundary

[David Sterba <dsterba@suse.cz>]

On Fri, Jul 31, 2020 at 07:29:11PM +0800, Qu Wenruo wrote:
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -4720,6 +4720,18 @@ int btrfs_shrink_device(struct btrfs_device *device, u64 new_size)
>  	}
>  
>  	mutex_lock(&fs_info->chunk_mutex);
> +	/*
> +	 * Also clear any CHUNK_TRIMMED and CHUNK_ALLOCATED bits beyond the
> +	 * current device boundary.
> +	 * This shouldn't fail, as alloc_state should only utilize those two
> +	 * bits, thus we shouldn't alloc new memory for clearing the status.

If this fails or not depends on implementation details of
clear_extent_bits and this comment will get out of sync eventually, so I
don't think it should be that specific.

If the new_size is somewhere in the middle of an existing state, it'll
need to be split anyway, no?

alloc_state |-----+++++|
clear             |------------------------- ... (u64)-1|

So we'd need to keep the state "-" and unset bits only from "+", and
this will require a split.

But I still have doubts about just clearing the range, why are there any
device->alloc_state entries at all after device is shrunk? Using
clear_extent_bits here is not wrong if we look at the end result of
clearing the range, but otherwise it leaves some state information
and allocated memory behind.