linux-kernel - Re: [PATCH] btrfs: block-group: Fix free-space bitmap threshould

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <9953a211-5068-c31d-3cf7-fe8441f3c38c@gmx.com>
Date:   Fri, 21 Aug 2020 11:34:30 +0800
From:   Qu Wenruo <quwenruo.btrfs@....com>
To:     Marcos Paulo de Souza <marcos@...esouza.com>,
        linux-kernel@...r.kernel.org
Cc:     dsterba@...e.com, wqu@...e.com, linux-btrfs@...r.kernel.org,
        Marcos Paulo de Souza <mpdesouza@...e.com>,
        stable@...r.kernel.org
Subject: Re: [PATCH] btrfs: block-group: Fix free-space bitmap threshould



On 2020/8/21 上午10:42, Marcos Paulo de Souza wrote:
> From: Marcos Paulo de Souza <mpdesouza@...e.com>
> 
> [BUG]
> After commit 9afc66498a0b ("btrfs: block-group: refactor how we read one
> block group item"), cache->length is being assigned after calling
> btrfs_create_block_group_cache. This causes a problem since
> set_free_space_tree_thresholds is calculate the free-space threshould to
> decide is the free-space tree should convert from extents to bitmaps.
> 
> The current code calls set_free_space_tree_thresholds with cache->length
> being 0, which then makes cache->bitmap_high_thresh being zero. This
> implies the system will always use bitmap instead of extents, which is
> not desired if the block group is not fragmented.
> 
> This behavior can be seen by a test that expects to repair systems
> with FREE_SPACE_EXTENT and FREE_SPACE_BITMAP, but the current code only
> created FREE_SPACE_BITMAP.
> 
> [FIX]
> Call set_free_space_tree_thresholds after setting cache->length.
> 
> Link: https://github.com/kdave/btrfs-progs/issues/251
> Fixes: 9afc66498a0b ("btrfs: block-group: refactor how we read one block group item")
> CC: stable@...r.kernel.org # 5.8+
> Signed-off-by: Marcos Paulo de Souza <mpdesouza@...e.com>

Reviewed-by: Qu Wenruo <wqu@...e.com>

It would be even nicer if you could add some warning or self-test on
cache->length to prevent such problem from happening again.

Thanks,
Qu
> ---
>  fs/btrfs/block-group.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c
> index 44fdfa2eeb2e..01e8ba1da1d3 100644
> --- a/fs/btrfs/block-group.c
> +++ b/fs/btrfs/block-group.c
> @@ -1798,7 +1798,6 @@ static struct btrfs_block_group *btrfs_create_block_group_cache(
>  
>  	cache->fs_info = fs_info;
>  	cache->full_stripe_len = btrfs_full_stripe_len(fs_info, start);
> -	set_free_space_tree_thresholds(cache);
>  
>  	cache->discard_index = BTRFS_DISCARD_INDEX_UNUSED;
>  
> @@ -1908,6 +1907,8 @@ static int read_one_block_group(struct btrfs_fs_info *info,
>  
>  	read_block_group_item(cache, path, key);
>  
> +	set_free_space_tree_thresholds(cache);
> +
>  	if (need_clear) {
>  		/*
>  		 * When we mount with old space cache, we need to
> @@ -2128,6 +2129,7 @@ int btrfs_make_block_group(struct btrfs_trans_handle *trans, u64 bytes_used,
>  		return -ENOMEM;
>  
>  	cache->length = size;
> +	set_free_space_tree_thresholds(cache);
>  	cache->used = bytes_used;
>  	cache->flags = type;
>  	cache->last_byte_to_unpin = (u64)-1;
> 



Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)