diff mbox series

[v2] btrfs: fix use-after-free when COWing tree bock and tracing is enabled

Message ID 1b96eb7eb9ce220acc21b2ac2057a5a3eab1fb3b.1733953828.git.fdmanana@suse.com (mailing list archive)
State New
Headers show
Series [v2] btrfs: fix use-after-free when COWing tree bock and tracing is enabled | expand

Commit Message

Filipe Manana Dec. 11, 2024, 9:53 p.m. UTC
From: Filipe Manana <fdmanana@suse.com>

When a COWing a tree block, at btrfs_cow_block(), and we have the
tracepoint trace_btrfs_cow_block() enabled and preemption is also enabled
(CONFIG_PREEMPT=y), we can trigger a use-after-free in the COWed extent
buffer while inside the tracepoint code. This is because in some paths
that call btrfs_cow_block(), such as btrfs_search_slot(), we are holding
the last reference on the extent buffer @buf so btrfs_force_cow_block()
drops the last reference on the @buf extent buffer when it calls
free_extent_buffer_stale(buf), which schedules the release of the extent
buffer with RCU. This means that if we are on a kernel with preemption,
the current task may be preempted before calling trace_btrfs_cow_block()
and the extent buffer already released by the time trace_btrfs_cow_block()
is called, resulting in a use-after-free.

Fix this by moving the trace_btrfs_cow_block() from btrfs_cow_block() to
btrfs_force_cow_block() before the COWed extent buffer is freed.
This also has a side effect of invoking the tracepoint in the tree defrag
code, at defrag.c:btrfs_realloc_node(), since btrfs_force_cow_block() is
called there, but this is fine and it was actually missing there.

Reported-by: syzbot+8517da8635307182c8a5@syzkaller.appspotmail.com
Link: https://lore.kernel.org/linux-btrfs/6759a9b9.050a0220.1ac542.000d.GAE@google.com/
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---

V2: Instead of temporarily bumping the extent buffer's reference count
    to safely call the tracepoint, move the tracepoint call to
    btrfs_force_cow_block().

 fs/btrfs/ctree.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

Comments

Qu Wenruo Dec. 11, 2024, 11:05 p.m. UTC | #1
在 2024/12/12 08:23, fdmanana@kernel.org 写道:
> From: Filipe Manana <fdmanana@suse.com>
> 
> When a COWing a tree block, at btrfs_cow_block(), and we have the
> tracepoint trace_btrfs_cow_block() enabled and preemption is also enabled
> (CONFIG_PREEMPT=y), we can trigger a use-after-free in the COWed extent
> buffer while inside the tracepoint code. This is because in some paths
> that call btrfs_cow_block(), such as btrfs_search_slot(), we are holding
> the last reference on the extent buffer @buf so btrfs_force_cow_block()
> drops the last reference on the @buf extent buffer when it calls
> free_extent_buffer_stale(buf), which schedules the release of the extent
> buffer with RCU. This means that if we are on a kernel with preemption,
> the current task may be preempted before calling trace_btrfs_cow_block()
> and the extent buffer already released by the time trace_btrfs_cow_block()
> is called, resulting in a use-after-free.
> 
> Fix this by moving the trace_btrfs_cow_block() from btrfs_cow_block() to
> btrfs_force_cow_block() before the COWed extent buffer is freed.
> This also has a side effect of invoking the tracepoint in the tree defrag
> code, at defrag.c:btrfs_realloc_node(), since btrfs_force_cow_block() is
> called there, but this is fine and it was actually missing there.
> 
> Reported-by: syzbot+8517da8635307182c8a5@syzkaller.appspotmail.com
> Link: https://lore.kernel.org/linux-btrfs/6759a9b9.050a0220.1ac542.000d.GAE@google.com/
> Signed-off-by: Filipe Manana <fdmanana@suse.com>

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

Thanks for pinning down and fix the bug,
Qu

> ---
> 
> V2: Instead of temporarily bumping the extent buffer's reference count
>      to safely call the tracepoint, move the tracepoint call to
>      btrfs_force_cow_block().
> 
>   fs/btrfs/ctree.c | 11 ++++-------
>   1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
> index 693dc27ffb89..185985a337b3 100644
> --- a/fs/btrfs/ctree.c
> +++ b/fs/btrfs/ctree.c
> @@ -654,6 +654,8 @@ int btrfs_force_cow_block(struct btrfs_trans_handle *trans,
>   			goto error_unlock_cow;
>   		}
>   	}
> +
> +	trace_btrfs_cow_block(root, buf, cow);
>   	if (unlock_orig)
>   		btrfs_tree_unlock(buf);
>   	free_extent_buffer_stale(buf);
> @@ -710,7 +712,6 @@ int btrfs_cow_block(struct btrfs_trans_handle *trans,
>   {
>   	struct btrfs_fs_info *fs_info = root->fs_info;
>   	u64 search_start;
> -	int ret;
>   
>   	if (unlikely(test_bit(BTRFS_ROOT_DELETING, &root->state))) {
>   		btrfs_abort_transaction(trans, -EUCLEAN);
> @@ -751,12 +752,8 @@ int btrfs_cow_block(struct btrfs_trans_handle *trans,
>   	 * Also We don't care about the error, as it's handled internally.
>   	 */
>   	btrfs_qgroup_trace_subtree_after_cow(trans, root, buf);
> -	ret = btrfs_force_cow_block(trans, root, buf, parent, parent_slot,
> -				    cow_ret, search_start, 0, nest);
> -
> -	trace_btrfs_cow_block(root, buf, *cow_ret);
> -
> -	return ret;
> +	return btrfs_force_cow_block(trans, root, buf, parent, parent_slot,
> +				     cow_ret, search_start, 0, nest);
>   }
>   ALLOW_ERROR_INJECTION(btrfs_cow_block, ERRNO);
>
diff mbox series

Patch

diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
index 693dc27ffb89..185985a337b3 100644
--- a/fs/btrfs/ctree.c
+++ b/fs/btrfs/ctree.c
@@ -654,6 +654,8 @@  int btrfs_force_cow_block(struct btrfs_trans_handle *trans,
 			goto error_unlock_cow;
 		}
 	}
+
+	trace_btrfs_cow_block(root, buf, cow);
 	if (unlock_orig)
 		btrfs_tree_unlock(buf);
 	free_extent_buffer_stale(buf);
@@ -710,7 +712,6 @@  int btrfs_cow_block(struct btrfs_trans_handle *trans,
 {
 	struct btrfs_fs_info *fs_info = root->fs_info;
 	u64 search_start;
-	int ret;
 
 	if (unlikely(test_bit(BTRFS_ROOT_DELETING, &root->state))) {
 		btrfs_abort_transaction(trans, -EUCLEAN);
@@ -751,12 +752,8 @@  int btrfs_cow_block(struct btrfs_trans_handle *trans,
 	 * Also We don't care about the error, as it's handled internally.
 	 */
 	btrfs_qgroup_trace_subtree_after_cow(trans, root, buf);
-	ret = btrfs_force_cow_block(trans, root, buf, parent, parent_slot,
-				    cow_ret, search_start, 0, nest);
-
-	trace_btrfs_cow_block(root, buf, *cow_ret);
-
-	return ret;
+	return btrfs_force_cow_block(trans, root, buf, parent, parent_slot,
+				     cow_ret, search_start, 0, nest);
 }
 ALLOW_ERROR_INJECTION(btrfs_cow_block, ERRNO);