Linux Btrfs filesystem development
 help / color / mirror / Atom feed
* [PATCH v4 0/3] btrfs: fix a UAF where btrfs_root::dirty_list is freed but still referred
@ 2026-10-08 22:13 Qu Wenruo
  2026-10-08 22:13 ` [PATCH v4 1/3] btrfs: protect dirty_cowonly_roots, switch_commits and root->dirty_list Qu Wenruo
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Qu Wenruo @ 2026-10-08 22:13 UTC (permalink / raw)
  To: linux-btrfs

[CHANGELOG]
v4:
- Also cleanup qgroup swapped blocks and dirty_log_pages
  Which are only released during switch_commit_roots().

v3:
- Use Yalagada's v2 fix as the final UAF fix
  The delayed list_del_init() call inside btrfs_put_root() is not
  safe as another racing ioctl can grab the quota root, extending
  its lifespan.

v2:
- Add extra patches to address Sashiko's comment
  * Make all root->dirty_list users to hold trans_lock
  * Release root->dirty_list from cur_trans->switch_commits during
    transaction cleanup

The first patch is to make lock consistent when accessing
btrfs_root::dirty_list, btrfs_fs_info::dirty_cowonly_roots and
btrfs_transaction::switch_commits.

Normally it's not a big deal as the existing lock-holding callers are
also holding a trans handle, thus they can not race with transaction
committing.
But the last commit will change the cleanup timing, and Sashiko is not
happy with that, so make it more consistent and shut Sashiko up.

The second patch is an existing bug exposed by Sashiko, which also
affects the last UAF fix.

The last one is the final UAF fix for the bug reported by syzbot.

Qu Wenruo (2):
  btrfs: protect dirty_cowonly_roots, switch_commits and
    root->dirty_list
  btrfs: prevent use-after-free in btrfs_transaction::switch_commits

Yalagada Pavan Kumar (1):
  btrfs: fix use-after-free on quota enable allocation failure

 fs/btrfs/disk-io.c     | 14 ++++++++++++++
 fs/btrfs/qgroup.c      |  7 +++++++
 fs/btrfs/transaction.c | 30 ++++++++++++++++++++++++++----
 3 files changed, 47 insertions(+), 4 deletions(-)

-- 
2.55.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v4 1/3] btrfs: protect dirty_cowonly_roots, switch_commits and root->dirty_list
  2026-10-08 22:13 [PATCH v4 0/3] btrfs: fix a UAF where btrfs_root::dirty_list is freed but still referred Qu Wenruo
@ 2026-10-08 22:13 ` Qu Wenruo
  2026-10-08 22:13 ` [PATCH v4 2/3] btrfs: prevent use-after-free in btrfs_transaction::switch_commits Qu Wenruo
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 6+ messages in thread
From: Qu Wenruo @ 2026-10-08 22:13 UTC (permalink / raw)
  To: linux-btrfs

Currently root->dirty_list is accessed with inconsistent locking.

Several call sites hold fs_info->trans_lock when modifying
root->dirty_list, including:

- add_root_to_dirty_list()
- btrfs_delete_free_space_tree()
- btrfs_quota_disable()

On the other hand there are also several call sites not holding that lock:

- switch_commit_roots()
- commit_cowonly_roots()
- btrfs_commit_transaction()

Those are also fine, because in their context they are the only
process committing the transaction.
The lock-holding callers are all holding a transaction handle, thus they
cannot race with transaction commit.

But for the sake of consistency, still hold the trans_lock for the call
sites that read and modify btrfs_root::dirty_list,
btrfs_fs_info::dirty_cowonly_roots and
btrfs_transaction::switch_commits.

And for several list_for_each_entry_safe() call sites, since we need to
unlock the trans_lock for functions that can sleep, convert them to use
"while (!list_empty()) { }" loop instead to be extra safe.

This is a preparation for an incoming UAF fix, which will move the
btrfs_root::dirty_list cleanup into btrfs_put_root().

That incoming fix will change the timing of cleanup, thus a consistent
lock scheme will help to shut Sashiko up.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 fs/btrfs/transaction.c | 30 ++++++++++++++++++++++++++----
 1 file changed, 26 insertions(+), 4 deletions(-)

diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
index 8df7b47b09e0..03c52566d65e 100644
--- a/fs/btrfs/transaction.c
+++ b/fs/btrfs/transaction.c
@@ -213,7 +213,6 @@ static noinline void switch_commit_roots(struct btrfs_trans_handle *trans)
 {
 	struct btrfs_transaction *cur_trans = trans->transaction;
 	struct btrfs_fs_info *fs_info = trans->fs_info;
-	struct btrfs_root *root, *tmp;
 
 	/*
 	 * At this point no one can be using this transaction to modify any tree
@@ -227,18 +226,27 @@ static noinline void switch_commit_roots(struct btrfs_trans_handle *trans)
 	if (test_bit(BTRFS_FS_RELOC_RUNNING, &fs_info->flags))
 		fs_info->last_reloc_trans = trans->transid;
 
-	list_for_each_entry_safe(root, tmp, &cur_trans->switch_commits,
-				 dirty_list) {
+	spin_lock(&fs_info->trans_lock);
+	while (!list_empty(&cur_trans->switch_commits)) {
+		struct btrfs_root *root;
+
+		root = list_first_entry(&cur_trans->switch_commits,
+					struct btrfs_root, dirty_list);
 		list_del_init(&root->dirty_list);
+		spin_unlock(&fs_info->trans_lock);
 		free_extent_buffer(root->commit_root);
 		root->commit_root = btrfs_root_node(root);
 		btrfs_extent_io_tree_release(&root->dirty_log_pages);
 		btrfs_qgroup_clean_swapped_blocks(root);
+		spin_lock(&fs_info->trans_lock);
 	}
+	spin_unlock(&fs_info->trans_lock);
 
 	/* We can free old roots now. */
 	spin_lock(&cur_trans->dropped_roots_lock);
 	while (!list_empty(&cur_trans->dropped_roots)) {
+		struct btrfs_root *root;
+
 		root = list_first_entry(&cur_trans->dropped_roots,
 					struct btrfs_root, root_list);
 		list_del_init(&root->root_list);
@@ -1439,6 +1447,7 @@ static noinline int commit_cowonly_roots(struct btrfs_trans_handle *trans)
 		return ret;
 
 again:
+	spin_lock(&fs_info->trans_lock);
 	while (!list_empty(&fs_info->dirty_cowonly_roots)) {
 		struct btrfs_root *root;
 
@@ -1447,11 +1456,14 @@ static noinline int commit_cowonly_roots(struct btrfs_trans_handle *trans)
 		clear_bit(BTRFS_ROOT_DIRTY, &root->state);
 		list_move_tail(&root->dirty_list,
 			       &trans->transaction->switch_commits);
+		spin_unlock(&fs_info->trans_lock);
 
 		ret = update_cowonly_root(trans, root);
 		if (ret)
 			return ret;
+		spin_lock(&fs_info->trans_lock);
 	}
+	spin_unlock(&fs_info->trans_lock);
 
 	/* Now flush any delayed refs generated by updating all of the roots */
 	ret = btrfs_run_delayed_refs(trans, U64_MAX);
@@ -1474,8 +1486,12 @@ static noinline int commit_cowonly_roots(struct btrfs_trans_handle *trans)
 			return ret;
 	}
 
-	if (!list_empty(&fs_info->dirty_cowonly_roots))
+	spin_lock(&fs_info->trans_lock);
+	if (!list_empty(&fs_info->dirty_cowonly_roots)) {
+		spin_unlock(&fs_info->trans_lock);
 		goto again;
+	}
+	spin_unlock(&fs_info->trans_lock);
 
 	/* Update dev-replace pointer once everything is committed */
 	fs_info->dev_replace.committed_cursor_left =
@@ -1588,8 +1604,10 @@ static noinline int commit_fs_roots(struct btrfs_trans_handle *trans)
 			smp_mb__after_atomic();
 
 			if (root->commit_root != root->node) {
+				spin_lock(&fs_info->trans_lock);
 				list_add_tail(&root->dirty_list,
 					&trans->transaction->switch_commits);
+				spin_unlock(&fs_info->trans_lock);
 				btrfs_set_root_node(&root->root_item,
 						    root->node);
 			}
@@ -2578,13 +2596,17 @@ int btrfs_commit_transaction(struct btrfs_trans_handle *trans)
 
 	btrfs_set_root_node(&fs_info->tree_root->root_item,
 			    fs_info->tree_root->node);
+	spin_lock(&fs_info->trans_lock);
 	list_add_tail(&fs_info->tree_root->dirty_list,
 		      &cur_trans->switch_commits);
+	spin_unlock(&fs_info->trans_lock);
 
 	btrfs_set_root_node(&fs_info->chunk_root->root_item,
 			    fs_info->chunk_root->node);
+	spin_lock(&fs_info->trans_lock);
 	list_add_tail(&fs_info->chunk_root->dirty_list,
 		      &cur_trans->switch_commits);
+	spin_unlock(&fs_info->trans_lock);
 
 	switch_commit_roots(trans);
 
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v4 2/3] btrfs: prevent use-after-free in btrfs_transaction::switch_commits
  2026-10-08 22:13 [PATCH v4 0/3] btrfs: fix a UAF where btrfs_root::dirty_list is freed but still referred Qu Wenruo
  2026-10-08 22:13 ` [PATCH v4 1/3] btrfs: protect dirty_cowonly_roots, switch_commits and root->dirty_list Qu Wenruo
@ 2026-10-08 22:13 ` Qu Wenruo
  2026-10-08 22:13 ` [PATCH v4 3/3] btrfs: fix use-after-free on quota enable allocation failure Qu Wenruo
  2026-10-09 16:30 ` [PATCH v4 0/3] btrfs: fix a UAF where btrfs_root::dirty_list is freed but still referred Boris Burkov
  3 siblings, 0 replies; 6+ messages in thread
From: Qu Wenruo @ 2026-10-08 22:13 UTC (permalink / raw)
  To: linux-btrfs

If a transaction is aborted during committing,
btrfs_transaction::switch_commits can be non-empty, but the
btrfs_transaction structure can be freed at the last
btrfs_put_transaction() inside cleanup_transaction().

The btrfs_root::dirty_list entry still linked into switch_commits will
point to already released memory, causing use-after-free.

Address this problem by manually deleting all entries inside
btrfs_transaction::switch_commits.

And since we're here, also release the dirty_log_pages and qgroup
swapped blocks for that root, which are only released during a
successful switch_commit_roots() call.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 fs/btrfs/disk-io.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 66b054eb98c6..3f8ed5732c11 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -4966,6 +4966,20 @@ void btrfs_cleanup_one_transaction(struct btrfs_transaction *cur_trans)
 		list_del_init(&dev->post_commit_list);
 	}
 
+	spin_lock(&fs_info->trans_lock);
+	while (!list_empty(&cur_trans->switch_commits)) {
+		struct btrfs_root *root;
+
+		root = list_first_entry(&cur_trans->switch_commits,
+					struct btrfs_root, dirty_list);
+		list_del_init(&root->dirty_list);
+		spin_unlock(&fs_info->trans_lock);
+		btrfs_extent_io_tree_release(&root->dirty_log_pages);
+		btrfs_qgroup_clean_swapped_blocks(root);
+		spin_lock(&fs_info->trans_lock);
+	}
+	spin_unlock(&fs_info->trans_lock);
+
 	btrfs_destroy_delayed_refs(cur_trans);
 
 	cur_trans->state = TRANS_STATE_COMMIT_START;
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v4 3/3] btrfs: fix use-after-free on quota enable allocation failure
  2026-10-08 22:13 [PATCH v4 0/3] btrfs: fix a UAF where btrfs_root::dirty_list is freed but still referred Qu Wenruo
  2026-10-08 22:13 ` [PATCH v4 1/3] btrfs: protect dirty_cowonly_roots, switch_commits and root->dirty_list Qu Wenruo
  2026-10-08 22:13 ` [PATCH v4 2/3] btrfs: prevent use-after-free in btrfs_transaction::switch_commits Qu Wenruo
@ 2026-10-08 22:13 ` Qu Wenruo
  2026-10-09 16:30 ` [PATCH v4 0/3] btrfs: fix a UAF where btrfs_root::dirty_list is freed but still referred Boris Burkov
  3 siblings, 0 replies; 6+ messages in thread
From: Qu Wenruo @ 2026-10-08 22:13 UTC (permalink / raw)
  To: linux-btrfs; +Cc: Yalagada Pavan Kumar, syzbot+947286c775f432b073a8

From: Yalagada Pavan Kumar <pavankumaryalagada@gmail.com>

[BUG]
There is a syzbot report that a use-after-free bug is triggered after a
failed btrfs_quota_enable():

 loop0: detected capacity change from 32768 to 0
 ==================================================================
 BUG: KASAN: slab-use-after-free in __list_add_valid_or_report+0x4e/0x130 lib/list_debug.c:29
 Read of size 8 at addr ffff888040e0a528 by task syz.0.0/5324

 CPU: 0 UID: 0 PID: 5324 Comm: syz.0.0 Not tainted syzkaller #0 PREEMPT(full)
 Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
 Call Trace:
  <TASK>
  dump_stack_lvl+0xe8/0x150 lib/dump_stack.c:120
  print_address_description+0x55/0x1e0 mm/kasan/report.c:378
  print_report+0x58/0x70 mm/kasan/report.c:482
  kasan_report+0x117/0x150 mm/kasan/report.c:595
  __list_add_valid_or_report+0x4e/0x130 lib/list_debug.c:29
  __list_add_valid include/linux/list.h:104 [inline]
  __list_add include/linux/list.h:169 [inline]
  list_add include/linux/list.h:192 [inline]
  list_move include/linux/list.h:345 [inline]
  add_root_to_dirty_list+0x38b/0x470 fs/btrfs/ctree.c:232
  btrfs_force_cow_block+0xf09/0x1bf0 fs/btrfs/ctree.c:553
  btrfs_cow_block+0x3f1/0xaf0 fs/btrfs/ctree.c:696
  btrfs_search_slot+0xd70/0x2d20 fs/btrfs/ctree.c:-1
  btrfs_search_prev_slot fs/btrfs/free-space-tree.c:134 [inline]
  remove_free_space_extent fs/btrfs/free-space-tree.c:732 [inline]
  __btrfs_remove_from_free_space_tree fs/btrfs/free-space-tree.c:837 [inline]
  btrfs_remove_from_free_space_tree+0x4d6/0xda0 fs/btrfs/free-space-tree.c:866
  alloc_reserved_extent+0x4a/0x2b0 fs/btrfs/extent-tree.c:4976
  alloc_reserved_tree_block fs/btrfs/extent-tree.c:5155 [inline]
  run_delayed_tree_ref fs/btrfs/extent-tree.c:1815 [inline]
  run_one_delayed_ref fs/btrfs/extent-tree.c:1851 [inline]
  btrfs_run_delayed_refs_for_head fs/btrfs/extent-tree.c:2058 [inline]
  __btrfs_run_delayed_refs+0x18b5/0x43b0 fs/btrfs/extent-tree.c:2134
  btrfs_run_delayed_refs+0xdc/0x2a0 fs/btrfs/extent-tree.c:2246
  btrfs_commit_transaction+0x28a/0x3170 fs/btrfs/transaction.c:2267
  sync_filesystem+0x1d2/0x240 fs/sync.c:66
  btrfs_reconfigure+0x2dc/0x2100 fs/btrfs/super.c:1511
  reconfigure_super+0x232/0x8f0 fs/super.c:1128
  do_remount fs/namespace.c:3410 [inline]
  path_mount+0xd4e/0x1050 fs/namespace.c:4160
  do_mount fs/namespace.c:4181 [inline]
  __do_sys_mount fs/namespace.c:4397 [inline]
  __se_sys_mount+0x31d/0x420 fs/namespace.c:4374
  do_syscall_x64 arch/x86/entry/syscall_64.c:61 [inline]
  do_syscall_64+0x166/0x520 arch/x86/entry/syscall_64.c:84
  entry_SYSCALL_64_after_hwframe+0x77/0x7f
 RIP: 0033:0x7f9c8dd9f3ca
  </TASK>

 Allocated by task 5323:
  kasan_save_stack mm/kasan/common.c:57 [inline]
  kasan_save_track+0x3e/0x80 mm/kasan/common.c:78
  poison_kmalloc_redzone mm/kasan/common.c:409 [inline]
  __kasan_kmalloc+0x93/0xb0 mm/kasan/common.c:426
  kasan_kmalloc include/linux/kasan.h:263 [inline]
  __kmalloc_cache_noprof+0x321/0x600 mm/slub.c:5563
  _kmalloc_noprof include/linux/slab.h:991 [inline]
  _kzalloc_noprof include/linux/slab.h:1312 [inline]
  btrfs_alloc_root+0x75/0x840 fs/btrfs/disk-io.c:638
  btrfs_create_tree+0xa8/0x5c0 fs/btrfs/disk-io.c:832
  btrfs_quota_enable+0x3d6/0x1e20 fs/btrfs/qgroup.c:1074
  btrfs_ioctl_quota_ctl+0x186/0x1f0 fs/btrfs/ioctl.c:3581
  vfs_ioctl fs/ioctl.c:51 [inline]
  __do_sys_ioctl fs/ioctl.c:597 [inline]
  __se_sys_ioctl+0xfc/0x170 fs/ioctl.c:583
  do_syscall_x64 arch/x86/entry/syscall_64.c:61 [inline]
  do_syscall_64+0x166/0x520 arch/x86/entry/syscall_64.c:84
  entry_SYSCALL_64_after_hwframe+0x77/0x7f

 Freed by task 5323:
  kasan_save_stack mm/kasan/common.c:57 [inline]
  kasan_save_track+0x3e/0x80 mm/kasan/common.c:78
  kasan_save_free_info+0x40/0x50 mm/kasan/generic.c:584
  poison_slab_object mm/kasan/common.c:264 [inline]
  __kasan_slab_free+0x5c/0x80 mm/kasan/common.c:296
  kasan_slab_free include/linux/kasan.h:235 [inline]
  slab_free_hook mm/slub.c:2748 [inline]
  slab_free mm/slub.c:6508 [inline]
  kfree+0x1c5/0x650 mm/slub.c:6801
  btrfs_quota_enable+0x1208/0x1e20 fs/btrfs/qgroup.c:1292
  btrfs_ioctl_quota_ctl+0x186/0x1f0 fs/btrfs/ioctl.c:3581
  vfs_ioctl fs/ioctl.c:51 [inline]
  __do_sys_ioctl fs/ioctl.c:597 [inline]
  __se_sys_ioctl+0xfc/0x170 fs/ioctl.c:583
  do_syscall_x64 arch/x86/entry/syscall_64.c:61 [inline]
  do_syscall_64+0x166/0x520 arch/x86/entry/syscall_64.c:84
  entry_SYSCALL_64_after_hwframe+0x77/0x7f

[CAUSE]
During btrfs_quota_enable(), a temporary quota_root is created, then
qgroup items are created for each subvolume and inserted into that root.

If there are enough subvolumes, the level of quota_root will be bumped,
and add_root_to_dirty_list() will be called, adding
quota_root->dirty_list into fs_info->dirty_cowonly_roots.

But if a problem is hit during btrfs_quota_enable() later, that
quota_root will be released through btrfs_put_root(), causing the whole
quota_root structure to be released, while leaving
fs_info->dirty_cowonly_roots still pointing to the released
quota_root->dirty_list.

Later access through fs_info->dirty_cowonly_roots will trigger a
use-after-free bug.

[FIX]
With previous patches doing the preparation work:

- Always hold the trans_lock when modifying btrfs_root::dirty_list
- Unlink all btrfs_root::dirty_list entries from
  btrfs_transaction::switch_commits when a transaction is cleaned up

Now we can just call list_del_init(&root->dirty_list) when
btrfs_quota_enable() failed.

Also to follow the existing error handling behavior, abort the
transaction when the memory preallocation failed.

Reported-by: syzbot+947286c775f432b073a8@syzkaller.appspotmail.com
Link: https://lore.kernel.org/linux-btrfs/6ac5a5e1.9ad129c8.3ce5b3.0010.GAE@google.com/
Closes: https://syzkaller.appspot.com/bug?extid=947286c775f432b073a8
Signed-off-by: Yalagada Pavan Kumar <pavankumaryalagada@gmail.com>
Reviewed-by: Qu Wenruo <wqu@suse.com>
[ Change the commit message to include a proper analysis. ]
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 fs/btrfs/qgroup.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
index cf8dfd5c692b..a0651fbfe778 100644
--- a/fs/btrfs/qgroup.c
+++ b/fs/btrfs/qgroup.c
@@ -1207,6 +1207,7 @@ int btrfs_quota_enable(struct btrfs_fs_info *fs_info,
 	prealloc = kzalloc_obj(*prealloc, GFP_NOFS);
 	if (!prealloc) {
 		ret = -ENOMEM;
+		btrfs_abort_transaction(trans, ret);
 		goto out;
 	}
 	qgroup = add_qgroup_rb(fs_info, prealloc, BTRFS_FS_TREE_OBJECTID);
@@ -1296,6 +1297,12 @@ int btrfs_quota_enable(struct btrfs_fs_info *fs_info,
 		 * sysfs entries.
 		 */
 		btrfs_free_qgroup_config(fs_info);
+
+		if (quota_root) {
+			spin_lock(&fs_info->trans_lock);
+			list_del_init(&quota_root->dirty_list);
+			spin_unlock(&fs_info->trans_lock);
+		}
 		btrfs_put_root(quota_root);
 	}
 	mutex_unlock(&fs_info->qgroup_ioctl_lock);
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v4 0/3] btrfs: fix a UAF where btrfs_root::dirty_list is freed but still referred
  2026-10-08 22:13 [PATCH v4 0/3] btrfs: fix a UAF where btrfs_root::dirty_list is freed but still referred Qu Wenruo
                   ` (2 preceding siblings ...)
  2026-10-08 22:13 ` [PATCH v4 3/3] btrfs: fix use-after-free on quota enable allocation failure Qu Wenruo
@ 2026-10-09 16:30 ` Boris Burkov
  2026-10-09 21:00   ` Qu Wenruo
  3 siblings, 1 reply; 6+ messages in thread
From: Boris Burkov @ 2026-10-09 16:30 UTC (permalink / raw)
  To: Qu Wenruo; +Cc: linux-btrfs

On Fri, Oct 09, 2026 at 08:43:37AM +1030, Qu Wenruo wrote:
> [CHANGELOG]
> v4:
> - Also cleanup qgroup swapped blocks and dirty_log_pages
>   Which are only released during switch_commit_roots().
> 
> v3:
> - Use Yalagada's v2 fix as the final UAF fix
>   The delayed list_del_init() call inside btrfs_put_root() is not
>   safe as another racing ioctl can grab the quota root, extending
>   its lifespan.
> 
> v2:
> - Add extra patches to address Sashiko's comment
>   * Make all root->dirty_list users to hold trans_lock
>   * Release root->dirty_list from cur_trans->switch_commits during
>     transaction cleanup
> 
> The first patch is to make lock consistent when accessing
> btrfs_root::dirty_list, btrfs_fs_info::dirty_cowonly_roots and
> btrfs_transaction::switch_commits.
> 
> Normally it's not a big deal as the existing lock-holding callers are
> also holding a trans handle, thus they can not race with transaction
> committing.
> But the last commit will change the cleanup timing, and Sashiko is not
> happy with that, so make it more consistent and shut Sashiko up.

As far as I can tell, it is possible but unlikely (rescan + squota?) for
the new callsite to actually do the removal outside a trans handle. Is
that your understanding?

I would prefer to have the bar for "shutting sashiko up" to be at real
bugs, even if they are sort of unlikely. If I misunderstood and it's
fully a false alarm, I would sort of rather not make changes to satisfy
its misconceptions.

Fixes look good overall, thanks.
Reviewed-by: Boris Burkov <boris@bur.io>

> 
> The second patch is an existing bug exposed by Sashiko, which also
> affects the last UAF fix.
> 
> The last one is the final UAF fix for the bug reported by syzbot.
> 
> Qu Wenruo (2):
>   btrfs: protect dirty_cowonly_roots, switch_commits and
>     root->dirty_list
>   btrfs: prevent use-after-free in btrfs_transaction::switch_commits
> 
> Yalagada Pavan Kumar (1):
>   btrfs: fix use-after-free on quota enable allocation failure
> 
>  fs/btrfs/disk-io.c     | 14 ++++++++++++++
>  fs/btrfs/qgroup.c      |  7 +++++++
>  fs/btrfs/transaction.c | 30 ++++++++++++++++++++++++++----
>  3 files changed, 47 insertions(+), 4 deletions(-)
> 
> -- 
> 2.55.0
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v4 0/3] btrfs: fix a UAF where btrfs_root::dirty_list is freed but still referred
  2026-10-09 16:30 ` [PATCH v4 0/3] btrfs: fix a UAF where btrfs_root::dirty_list is freed but still referred Boris Burkov
@ 2026-10-09 21:00   ` Qu Wenruo
  0 siblings, 0 replies; 6+ messages in thread
From: Qu Wenruo @ 2026-10-09 21:00 UTC (permalink / raw)
  To: Boris Burkov, Qu Wenruo; +Cc: linux-btrfs



在 2026/10/10 03:00, Boris Burkov 写道:
> On Fri, Oct 09, 2026 at 08:43:37AM +1030, Qu Wenruo wrote:
>> [CHANGELOG]
>> v4:
>> - Also cleanup qgroup swapped blocks and dirty_log_pages
>>    Which are only released during switch_commit_roots().
>>
>> v3:
>> - Use Yalagada's v2 fix as the final UAF fix
>>    The delayed list_del_init() call inside btrfs_put_root() is not
>>    safe as another racing ioctl can grab the quota root, extending
>>    its lifespan.
>>
>> v2:
>> - Add extra patches to address Sashiko's comment
>>    * Make all root->dirty_list users to hold trans_lock
>>    * Release root->dirty_list from cur_trans->switch_commits during
>>      transaction cleanup
>>
>> The first patch is to make lock consistent when accessing
>> btrfs_root::dirty_list, btrfs_fs_info::dirty_cowonly_roots and
>> btrfs_transaction::switch_commits.
>>
>> Normally it's not a big deal as the existing lock-holding callers are
>> also holding a trans handle, thus they can not race with transaction
>> committing.
>> But the last commit will change the cleanup timing, and Sashiko is not
>> happy with that, so make it more consistent and shut Sashiko up.
> 
> As far as I can tell, it is possible but unlikely (rescan + squota?) for
> the new callsite to actually do the removal outside a trans handle. Is
> that your understanding?

That part (the first patch) is a little overkilled.

The main reason that patch is introduced is to prepare for calling 
"list_del_init(&root->dirty_list);" during btrfs_put_root().

As we have some call sites that doesn't hold a trans handler.

But later I switched back to the v2 fix from Yalagada, which does the 
manual list_del_init() call instead of relying on btrfs_put_root().

> 
> I would prefer to have the bar for "shutting sashiko up" to be at real
> bugs, even if they are sort of unlikely. If I misunderstood and it's
> fully a false alarm, I would sort of rather not make changes to satisfy
> its misconceptions.

I'll remove the first patch from the series.

Thanks for the review,
Qu

> 
> Fixes look good overall, thanks.
> Reviewed-by: Boris Burkov <boris@bur.io>
> 
>>
>> The second patch is an existing bug exposed by Sashiko, which also
>> affects the last UAF fix.
>>
>> The last one is the final UAF fix for the bug reported by syzbot.
>>
>> Qu Wenruo (2):
>>    btrfs: protect dirty_cowonly_roots, switch_commits and
>>      root->dirty_list
>>    btrfs: prevent use-after-free in btrfs_transaction::switch_commits
>>
>> Yalagada Pavan Kumar (1):
>>    btrfs: fix use-after-free on quota enable allocation failure
>>
>>   fs/btrfs/disk-io.c     | 14 ++++++++++++++
>>   fs/btrfs/qgroup.c      |  7 +++++++
>>   fs/btrfs/transaction.c | 30 ++++++++++++++++++++++++++----
>>   3 files changed, 47 insertions(+), 4 deletions(-)
>>
>> -- 
>> 2.55.0
>>
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-10-09 21:00 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-08 22:13 [PATCH v4 0/3] btrfs: fix a UAF where btrfs_root::dirty_list is freed but still referred Qu Wenruo
2026-10-08 22:13 ` [PATCH v4 1/3] btrfs: protect dirty_cowonly_roots, switch_commits and root->dirty_list Qu Wenruo
2026-10-08 22:13 ` [PATCH v4 2/3] btrfs: prevent use-after-free in btrfs_transaction::switch_commits Qu Wenruo
2026-10-08 22:13 ` [PATCH v4 3/3] btrfs: fix use-after-free on quota enable allocation failure Qu Wenruo
2026-10-09 16:30 ` [PATCH v4 0/3] btrfs: fix a UAF where btrfs_root::dirty_list is freed but still referred Boris Burkov
2026-10-09 21:00   ` Qu Wenruo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox