Re: [PATCH v2] btrfs: fix invalid pointer dereference in __btrfs_run_delayed_refs()

[Qu Wenruo <wqu@suse.com>]

在 2026/5/22 00:37, fdmanana@kernel.org 写道:
> From: Filipe Manana <fdmanana@suse.com>
> 
> In the beginning of the loop, we try to obtain a locked delayed ref head,
> if 'locked_ref' is currently NULL, by calling btrfs_select_ref_head(),
> which can return an error pointer. If the error pointer is -EAGAIN we do
> a continue and go back to the beginning of the loop, which will not try
> again to call btrfs_select_ref_head() since 'locked_ref' is no longer
> NULL but it's ERR_PTR(-EAGAIN), and then we do:
> 
>     spin_lock(&locked_ref->lock);
> 
> against a ERR_PTR(-EAGAIN) value, generating an invalid pointer
> dereference.
> 
> Fix this by ensuring that 'locked_ref' is set to NULL when
> btrfs_select_ref_head() returns ERR_PTR(-EAGAIN) and incrementing 'count'
> as well, to prevent infinite looping. We do this by doing a goto to the
> bottom of the loop that already sets 'locked_ref' to NULL and does a
> cond_resched(), with an increment to 'count' right before the goto.
> These measures were in place before the refactoring in commit 0110a4c43451
> ("btrfs: refactor __btrfs_run_delayed_refs loop") but were unintentionally
> lost afterwards.
> 
> Reported-by: Dan Carpenter <error27@gmail.com>
> Link: https://lore.kernel.org/linux-btrfs/ag8ARRwykv8bpJ87@stanley.mountain/
> Fixes: 0110a4c43451 ("btrfs: refactor __btrfs_run_delayed_refs loop")
> Signed-off-by: Filipe Manana <fdmanana@suse.com>

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

Thanks,
Qu

> ---
> 
> V2: Use a goto approach to jump to the bottom of the loop, that already
>      sets locked_ref to NULL and does a cond_resched() too.
> 
>   fs/btrfs/extent-tree.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index ecc1acb1e340..6030cdbdb742 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -2108,7 +2108,8 @@ static noinline int __btrfs_run_delayed_refs(struct btrfs_trans_handle *trans,
>   			locked_ref = btrfs_select_ref_head(fs_info, delayed_refs);
>   			if (IS_ERR_OR_NULL(locked_ref)) {
>   				if (PTR_ERR(locked_ref) == -EAGAIN) {
> -					continue;
> +					count++;
> +					goto again;
>   				} else {
>   					break;
>   				}
> @@ -2156,7 +2157,7 @@ static noinline int __btrfs_run_delayed_refs(struct btrfs_trans_handle *trans,
>   		 * Either success case or btrfs_run_delayed_refs_for_head
>   		 * returned -EAGAIN, meaning we need to select another head
>   		 */
> -
> +again:
>   		locked_ref = NULL;
>   		cond_resched();
>   	} while ((min_bytes != U64_MAX && bytes_processed < min_bytes) ||