* [PATCH v2 1/3] btrfs: protect dirty_cowonly_roots, switch_commits and root->dirty_list
2026-10-08 6:26 [PATCH v2 0/3] btrfs: fix a UAF where btrfs_root::dirty_list is freed but still referred Qu Wenruo
@ 2026-10-08 6:26 ` Qu Wenruo
2026-10-08 6:26 ` [PATCH v2 2/3] btrfs: prevent use-after-free in btrfs_transaction::switch_commits Qu Wenruo
2026-10-08 6:26 ` [PATCH v2 3/3] btrfs: unlink btrfs_root::dirty_list during btrfs_put_root() Qu Wenruo
2 siblings, 0 replies; 4+ messages in thread
From: Qu Wenruo @ 2026-10-08 6:26 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] 4+ messages in thread* [PATCH v2 3/3] btrfs: unlink btrfs_root::dirty_list during btrfs_put_root()
2026-10-08 6:26 [PATCH v2 0/3] btrfs: fix a UAF where btrfs_root::dirty_list is freed but still referred Qu Wenruo
2026-10-08 6:26 ` [PATCH v2 1/3] btrfs: protect dirty_cowonly_roots, switch_commits and root->dirty_list Qu Wenruo
2026-10-08 6:26 ` [PATCH v2 2/3] btrfs: prevent use-after-free in btrfs_transaction::switch_commits Qu Wenruo
@ 2026-10-08 6:26 ` Qu Wenruo
2 siblings, 0 replies; 4+ messages in thread
From: Qu Wenruo @ 2026-10-08 6:26 UTC (permalink / raw)
To: linux-btrfs; +Cc: syzbot+947286c775f432b073a8, Yalagada Pavan Kumar
[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) with proper locking
inside btrfs_put_root().
Reported-by: syzbot+947286c775f432b073a8@syzkaller.appspotmail.com
Link: https://lore.kernel.org/linux-btrfs/6ac5a5e1.9ad129c8.3ce5b3.0010.GAE@google.com/
Suggested-by: Yalagada Pavan Kumar <pavankumaryalagada@gmail.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
fs/btrfs/disk-io.c | 10 ++++++++--
fs/btrfs/free-space-tree.c | 4 ----
fs/btrfs/qgroup.c | 4 ----
3 files changed, 8 insertions(+), 10 deletions(-)
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 757db0c363b0..7531ebd61e8d 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -1840,6 +1840,12 @@ void btrfs_put_root(struct btrfs_root *root)
return;
if (refcount_dec_and_test(&root->refs)) {
+ struct btrfs_fs_info *fs_info = root->fs_info;
+
+ spin_lock(&fs_info->trans_lock);
+ list_del_init(&root->dirty_list);
+ spin_unlock(&fs_info->trans_lock);
+
if (WARN_ON(!xa_empty(&root->inodes)))
xa_destroy(&root->inodes);
if (WARN_ON(!xa_empty(&root->delayed_nodes)))
@@ -1849,9 +1855,9 @@ void btrfs_put_root(struct btrfs_root *root)
free_anon_bdev(root->anon_dev);
free_root_extent_buffers(root);
#ifdef CONFIG_BTRFS_DEBUG
- spin_lock(&root->fs_info->fs_roots_radix_lock);
+ spin_lock(&fs_info->fs_roots_radix_lock);
list_del_init(&root->leak_list);
- spin_unlock(&root->fs_info->fs_roots_radix_lock);
+ spin_unlock(&fs_info->fs_roots_radix_lock);
#endif
kfree(root);
}
diff --git a/fs/btrfs/free-space-tree.c b/fs/btrfs/free-space-tree.c
index b7a4a6ade30f..3660577c286e 100644
--- a/fs/btrfs/free-space-tree.c
+++ b/fs/btrfs/free-space-tree.c
@@ -1311,10 +1311,6 @@ int btrfs_delete_free_space_tree(struct btrfs_fs_info *fs_info)
btrfs_global_root_delete(free_space_root);
- spin_lock(&fs_info->trans_lock);
- list_del(&free_space_root->dirty_list);
- spin_unlock(&fs_info->trans_lock);
-
btrfs_tree_lock(free_space_root->node);
btrfs_clear_buffer_dirty(trans, free_space_root->node);
btrfs_tree_unlock(free_space_root->node);
diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
index cf8dfd5c692b..38a268b63991 100644
--- a/fs/btrfs/qgroup.c
+++ b/fs/btrfs/qgroup.c
@@ -1433,10 +1433,6 @@ int btrfs_quota_disable(struct btrfs_fs_info *fs_info)
goto out;
}
- spin_lock(&fs_info->trans_lock);
- list_del("a_root->dirty_list);
- spin_unlock(&fs_info->trans_lock);
-
btrfs_tree_lock(quota_root->node);
btrfs_clear_buffer_dirty(trans, quota_root->node);
btrfs_tree_unlock(quota_root->node);
--
2.55.0
^ permalink raw reply related [flat|nested] 4+ messages in thread