Re: [PATCH] btrfs: fix double free of anon_dev after failure to create subvolume

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

On 2021/12/11 03:02, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
>
> When creating a subvolume, at create_subvol(), we allocate an anonymous
> device and later call btrfs_get_new_fs_root(), which in turn just calls
> btrfs_get_root_ref(). There we call btrfs_init_fs_root() which assigns
> the anonymous device to the root, but if after that call there's an error,
> when we jump to 'fail' label, we call btrfs_put_root(), which frees the
> anonymous device and then returns an error that is propagated back to
> create_subvol(). Than create_subvol() frees the anonymous device again.
>
> When this happens, if the anonymous device was not reallocated after
> the first time it was freed with btrfs_put_root(), we get a kernel
> message like the following:
>
>    (...)
>    [13950.282466] BTRFS: error (device dm-0) in create_subvol:663: errno=-5 IO failure
>    [13950.283027] ida_free called for id=65 which is not allocated.
>    [13950.285974] BTRFS info (device dm-0): forced readonly
>    (...)
>
> If the anonymous device gets reallocated by another btrfs filesystem
> or any other kernel subsystem, then bad things can happen.
>
> So fix this by setting the root's anonymous device to 0 at
> btrfs_get_root_ref(), before we call btrfs_put_root(), if an error
> happened.
>
> Fixes: 2dfb1e43f57dd3 ("btrfs: preallocate anon block device at first phase of snapshot creation")
> Signed-off-by: Filipe Manana <fdmanana@suse.com>

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

Thanks,
Qu

> ---
>   fs/btrfs/disk-io.c | 8 ++++++++
>   1 file changed, 8 insertions(+)
>
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index 5c598e124c25..fc7dd5109806 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -1826,6 +1826,14 @@ static struct btrfs_root *btrfs_get_root_ref(struct btrfs_fs_info *fs_info,
>   	}
>   	return root;
>   fail:
> +	/*
> +	 * If our caller provided us an anonymous device, then it's his
> +	 * responsability to free it in case we fail. So we have to set our
> +	 * root's anon_dev to 0 to avoid a double free, once by btrfs_put_root()
> +	 * and once again by our caller.
> +	 */
> +	if (anon_dev)
> +		root->anon_dev = 0;
>   	btrfs_put_root(root);
>   	return ERR_PTR(ret);
>   }