Re: [PATCH] btrfs: fix leak of qgroup extent records after transaction abort

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

在 2024/6/3 21:36, fdmanana@kernel.org 写道:
> From: Filipe Manana <fdmanana@suse.com>
>
> Qgroup extent records are created when delayed ref heads are created and
> then released after accounting extents at btrfs_qgroup_account_extents(),
> called during the transaction commit path.
>
> If a transaction is aborted we free the qgroup records by calling
> btrfs_qgroup_destroy_extent_records() at btrfs_destroy_delayed_refs(),
> unless we don't have delayed references. We are incorrectly assuming
> that no delayed references means we don't have qgroup extents records.
>
> We can currently have no delayed references because we ran them all
> during a transaction commit and the transaction was aborted after that
> due to some error in the commit path.
>
> So fix this by ensuring we btrfs_qgroup_destroy_extent_records() at
> btrfs_destroy_delayed_refs() even if we don't have any delayed references.

Will it cause some underflow for delayed_refs->num_entries?

As in the rb tree iteration code, we would try to decrease
delayed_refs->num_entries again.

Thanks,
Qu
>
> Reported-by: syzbot+0fecc032fa134afd49df@syzkaller.appspotmail.com
> Link: https://lore.kernel.org/linux-btrfs/0000000000004e7f980619f91835@google.com/
> Signed-off-by: Filipe Manana <fdmanana@suse.com>
> ---
>   fs/btrfs/disk-io.c | 10 +---------
>   1 file changed, 1 insertion(+), 9 deletions(-)
>
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index 8693893744a0..b1daaaec0614 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -4522,18 +4522,10 @@ static void btrfs_destroy_delayed_refs(struct btrfs_transaction *trans,
>   				       struct btrfs_fs_info *fs_info)
>   {
>   	struct rb_node *node;
> -	struct btrfs_delayed_ref_root *delayed_refs;
> +	struct btrfs_delayed_ref_root *delayed_refs = &trans->delayed_refs;
>   	struct btrfs_delayed_ref_node *ref;
>
> -	delayed_refs = &trans->delayed_refs;
> -
>   	spin_lock(&delayed_refs->lock);
> -	if (atomic_read(&delayed_refs->num_entries) == 0) {
> -		spin_unlock(&delayed_refs->lock);
> -		btrfs_debug(fs_info, "delayed_refs has NO entry");
> -		return;
> -	}
> -
>   	while ((node = rb_first_cached(&delayed_refs->href_root)) != NULL) {
>   		struct btrfs_delayed_ref_head *head;
>   		struct rb_node *n;