Re: [PATCH] btrfs: unlock the newly allocated extent buffer in btrfs_alloc_tree_block()

[Denis Efremov <denis.e.efremov@xxxxxxxxxx>]

Hi,


On 9/14/21 09:57, Qu Wenruo wrote:
> [BUG]
...
> 
>   ================================================
>   WARNING: lock held when returning to user space!
>   5.15.0-rc1 #16 Not tainted
>   ------------------------------------------------
>   syz-executor/7579 is leaving the kernel with locks still held!
>   1 lock held by syz-executor/7579:
>    #0: ffff888104b73da8 (btrfs-tree-01/1){+.+.}-{3:3}, at:
>   __btrfs_tree_lock+0x2e/0x1a0 fs/btrfs/locking.c:112
> 
> [CAUSE]
> In btrfs_alloc_tree_block(), after btrfs_init_new_buffer(), the new
> extent buffer @buf is locked, but if later operations like adding
> delayed tree ref fails, we just free @buf without unlocking it,
> resulting above warning.

This patch fixes CVE-2021-4149. Commit 19ea40dddf18
"btrfs: unlock newly allocated extent buffer after error" upstream.
The patch was backported to kernels 5.15, 5.10, 5.4 because it contains
"CC: stable@xxxxxxxxxxxxxxx # 5.4+" in the commit message.

However, it looks to me like kernels 4.9, 4.14, 4.19 are also vulnerable.
In v4.9 kernel there is btrfs_init_new_buffer() call:
btrfs_alloc_tree_block(...)
{
	...
	buf = btrfs_init_new_buffer(trans, root, ins.objectid, level);
	...
out_free_buf:                                                                    
        free_extent_buffer(buf);
	...
}

and btrfs_init_new_buffer() contains btrfs_tree_lock(buf) inside it.

The patch can be cherry-picked to v4.9 kernel without a conflict.

Probably, the error was introduced in the commit 67b7859e9bfa
"btrfs: handle ENOMEM in btrfs_alloc_tree_block" It's in the kernel
since v4.1

Can you confirm that kernels v4.9, 4.14, 4.19 are also vulnerable?

Thanks,
Denis

> 
> [FIX]
> Unlock @buf in out_free_buf: tag.
> 
> Reported-by: Hao Sun <sunhao.th@xxxxxxxxx>
> Link: https://lore.kernel.org/linux-btrfs/CACkBjsZ9O6Zr0KK1yGn=1rQi6Crh1yeCRdTSBxx9R99L4xdn-Q@xxxxxxxxxxxxxx/
> Signed-off-by: Qu Wenruo <wqu@xxxxxxxx>
> ---
>  fs/btrfs/extent-tree.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index c88e7727a31a..8aa981ffe7b7 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -4898,6 +4898,7 @@ struct extent_buffer *btrfs_alloc_tree_block(struct btrfs_trans_handle *trans,
>  out_free_delayed:
>  	btrfs_free_delayed_extent_op(extent_op);
>  out_free_buf:
> +	btrfs_tree_unlock(buf);
>  	free_extent_buffer(buf);
>  out_free_reserved:
>  	btrfs_free_reserved_extent(fs_info, ins.objectid, ins.offset, 0);