Skip to content

Commit 24e9e6b

Browse files
fdmananagregkh
authored andcommitted
Btrfs: fix race between adding and putting tree mod seq elements and nodes
[ Upstream commit 7227ff4 ] There is a race between adding and removing elements to the tree mod log list and rbtree that can lead to use-after-free problems. Consider the following example that explains how/why the problems happens: 1) Task A has mod log element with sequence number 200. It currently is the only element in the mod log list; 2) Task A calls btrfs_put_tree_mod_seq() because it no longer needs to access the tree mod log. When it enters the function, it initializes 'min_seq' to (u64)-1. Then it acquires the lock 'tree_mod_seq_lock' before checking if there are other elements in the mod seq list. Since the list it empty, 'min_seq' remains set to (u64)-1. Then it unlocks the lock 'tree_mod_seq_lock'; 3) Before task A acquires the lock 'tree_mod_log_lock', task B adds itself to the mod seq list through btrfs_get_tree_mod_seq() and gets a sequence number of 201; 4) Some other task, name it task C, modifies a btree and because there elements in the mod seq list, it adds a tree mod elem to the tree mod log rbtree. That node added to the mod log rbtree is assigned a sequence number of 202; 5) Task B, which is doing fiemap and resolving indirect back references, calls btrfs get_old_root(), with 'time_seq' == 201, which in turn calls tree_mod_log_search() - the search returns the mod log node from the rbtree with sequence number 202, created by task C; 6) Task A now acquires the lock 'tree_mod_log_lock', starts iterating the mod log rbtree and finds the node with sequence number 202. Since 202 is less than the previously computed 'min_seq', (u64)-1, it removes the node and frees it; 7) Task B still has a pointer to the node with sequence number 202, and it dereferences the pointer itself and through the call to __tree_mod_log_rewind(), resulting in a use-after-free problem. This issue can be triggered sporadically with the test case generic/561 from fstests, and it happens more frequently with a higher number of duperemove processes. When it happens to me, it either freezes the VM or it produces a trace like the following before crashing: [ 1245.321140] general protection fault: 0000 [#1] PREEMPT SMP DEBUG_PAGEALLOC PTI [ 1245.321200] CPU: 1 PID: 26997 Comm: pool Not tainted 5.5.0-rc6-btrfs-next-52 #1 [ 1245.321235] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.12.0-0-ga698c8995f-prebuilt.qemu.org 04/01/2014 [ 1245.321287] RIP: 0010:rb_next+0x16/0x50 [ 1245.321307] Code: .... [ 1245.321372] RSP: 0018:ffffa151c4d039b0 EFLAGS: 00010202 [ 1245.321388] RAX: 6b6b6b6b6b6b6b6b RBX: ffff8ae221363c80 RCX: 6b6b6b6b6b6b6b6b [ 1245.321409] RDX: 0000000000000001 RSI: 0000000000000000 RDI: ffff8ae221363c80 [ 1245.321439] RBP: ffff8ae20fcc4688 R08: 0000000000000002 R09: 0000000000000000 [ 1245.321475] R10: ffff8ae20b120910 R11: 00000000243f8bb1 R12: 0000000000000038 [ 1245.321506] R13: ffff8ae221363c80 R14: 000000000000075f R15: ffff8ae223f762b8 [ 1245.321539] FS: 00007fdee1ec7700(0000) GS:ffff8ae236c80000(0000) knlGS:0000000000000000 [ 1245.321591] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 1245.321614] CR2: 00007fded4030c48 CR3: 000000021da16003 CR4: 00000000003606e0 [ 1245.321642] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 [ 1245.321668] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 [ 1245.321706] Call Trace: [ 1245.321798] __tree_mod_log_rewind+0xbf/0x280 [btrfs] [ 1245.321841] btrfs_search_old_slot+0x105/0xd00 [btrfs] [ 1245.321877] resolve_indirect_refs+0x1eb/0xc60 [btrfs] [ 1245.321912] find_parent_nodes+0x3dc/0x11b0 [btrfs] [ 1245.321947] btrfs_check_shared+0x115/0x1c0 [btrfs] [ 1245.321980] ? extent_fiemap+0x59d/0x6d0 [btrfs] [ 1245.322029] extent_fiemap+0x59d/0x6d0 [btrfs] [ 1245.322066] do_vfs_ioctl+0x45a/0x750 [ 1245.322081] ksys_ioctl+0x70/0x80 [ 1245.322092] ? trace_hardirqs_off_thunk+0x1a/0x1c [ 1245.322113] __x64_sys_ioctl+0x16/0x20 [ 1245.322126] do_syscall_64+0x5c/0x280 [ 1245.322139] entry_SYSCALL_64_after_hwframe+0x49/0xbe [ 1245.322155] RIP: 0033:0x7fdee3942dd7 [ 1245.322177] Code: .... [ 1245.322258] RSP: 002b:00007fdee1ec6c88 EFLAGS: 00000246 ORIG_RAX: 0000000000000010 [ 1245.322294] RAX: ffffffffffffffda RBX: 00007fded40210d8 RCX: 00007fdee3942dd7 [ 1245.322314] RDX: 00007fded40210d8 RSI: 00000000c020660b RDI: 0000000000000004 [ 1245.322337] RBP: 0000562aa89e7510 R08: 0000000000000000 R09: 00007fdee1ec6d44 [ 1245.322369] R10: 0000000000000073 R11: 0000000000000246 R12: 00007fdee1ec6d48 [ 1245.322390] R13: 00007fdee1ec6d40 R14: 00007fded40210d0 R15: 00007fdee1ec6d50 [ 1245.322423] Modules linked in: .... [ 1245.323443] ---[ end trace 01de1e9ec5dff3cd ]--- Fix this by ensuring that btrfs_put_tree_mod_seq() computes the minimum sequence number and iterates the rbtree while holding the lock 'tree_mod_log_lock' in write mode. Also get rid of the 'tree_mod_seq_lock' lock, since it is now redundant. Fixes: bd989ba ("Btrfs: add tree modification log functions") Fixes: 097b8a7 ("Btrfs: join tree mod log code with the code holding back delayed refs") CC: [email protected] # 4.4+ Reviewed-by: Josef Bacik <[email protected]> Reviewed-by: Nikolay Borisov <[email protected]> Signed-off-by: Filipe Manana <[email protected]> Signed-off-by: David Sterba <[email protected]> Signed-off-by: Sasha Levin <[email protected]>
1 parent a71561b commit 24e9e6b

File tree

5 files changed

+8
-16
lines changed

5 files changed

+8
-16
lines changed

fs/btrfs/ctree.c

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -351,12 +351,10 @@ u64 btrfs_get_tree_mod_seq(struct btrfs_fs_info *fs_info,
351351
struct seq_list *elem)
352352
{
353353
write_lock(&fs_info->tree_mod_log_lock);
354-
spin_lock(&fs_info->tree_mod_seq_lock);
355354
if (!elem->seq) {
356355
elem->seq = btrfs_inc_tree_mod_seq(fs_info);
357356
list_add_tail(&elem->list, &fs_info->tree_mod_seq_list);
358357
}
359-
spin_unlock(&fs_info->tree_mod_seq_lock);
360358
write_unlock(&fs_info->tree_mod_log_lock);
361359

362360
return elem->seq;
@@ -376,7 +374,7 @@ void btrfs_put_tree_mod_seq(struct btrfs_fs_info *fs_info,
376374
if (!seq_putting)
377375
return;
378376

379-
spin_lock(&fs_info->tree_mod_seq_lock);
377+
write_lock(&fs_info->tree_mod_log_lock);
380378
list_del(&elem->list);
381379
elem->seq = 0;
382380

@@ -387,19 +385,17 @@ void btrfs_put_tree_mod_seq(struct btrfs_fs_info *fs_info,
387385
* blocker with lower sequence number exists, we
388386
* cannot remove anything from the log
389387
*/
390-
spin_unlock(&fs_info->tree_mod_seq_lock);
388+
write_unlock(&fs_info->tree_mod_log_lock);
391389
return;
392390
}
393391
min_seq = cur_elem->seq;
394392
}
395393
}
396-
spin_unlock(&fs_info->tree_mod_seq_lock);
397394

398395
/*
399396
* anything that's lower than the lowest existing (read: blocked)
400397
* sequence number can be removed from the tree.
401398
*/
402-
write_lock(&fs_info->tree_mod_log_lock);
403399
tm_root = &fs_info->tree_mod_log;
404400
for (node = rb_first(tm_root); node; node = next) {
405401
next = rb_next(node);

fs/btrfs/ctree.h

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -851,14 +851,12 @@ struct btrfs_fs_info {
851851
struct list_head delayed_iputs;
852852
struct mutex cleaner_delayed_iput_mutex;
853853

854-
/* this protects tree_mod_seq_list */
855-
spinlock_t tree_mod_seq_lock;
856854
atomic64_t tree_mod_seq;
857-
struct list_head tree_mod_seq_list;
858855

859-
/* this protects tree_mod_log */
856+
/* this protects tree_mod_log and tree_mod_seq_list */
860857
rwlock_t tree_mod_log_lock;
861858
struct rb_root tree_mod_log;
859+
struct list_head tree_mod_seq_list;
862860

863861
atomic_t nr_async_submits;
864862
atomic_t async_submit_draining;

fs/btrfs/delayed-ref.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -279,15 +279,15 @@ void btrfs_merge_delayed_refs(struct btrfs_trans_handle *trans,
279279
if (head->is_data)
280280
return;
281281

282-
spin_lock(&fs_info->tree_mod_seq_lock);
282+
read_lock(&fs_info->tree_mod_log_lock);
283283
if (!list_empty(&fs_info->tree_mod_seq_list)) {
284284
struct seq_list *elem;
285285

286286
elem = list_first_entry(&fs_info->tree_mod_seq_list,
287287
struct seq_list, list);
288288
seq = elem->seq;
289289
}
290-
spin_unlock(&fs_info->tree_mod_seq_lock);
290+
read_unlock(&fs_info->tree_mod_log_lock);
291291

292292
ref = list_first_entry(&head->ref_list, struct btrfs_delayed_ref_node,
293293
list);
@@ -315,7 +315,7 @@ int btrfs_check_delayed_seq(struct btrfs_fs_info *fs_info,
315315
struct seq_list *elem;
316316
int ret = 0;
317317

318-
spin_lock(&fs_info->tree_mod_seq_lock);
318+
read_lock(&fs_info->tree_mod_log_lock);
319319
if (!list_empty(&fs_info->tree_mod_seq_list)) {
320320
elem = list_first_entry(&fs_info->tree_mod_seq_list,
321321
struct seq_list, list);
@@ -329,7 +329,7 @@ int btrfs_check_delayed_seq(struct btrfs_fs_info *fs_info,
329329
}
330330
}
331331

332-
spin_unlock(&fs_info->tree_mod_seq_lock);
332+
read_unlock(&fs_info->tree_mod_log_lock);
333333
return ret;
334334
}
335335

fs/btrfs/disk-io.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2519,7 +2519,6 @@ int open_ctree(struct super_block *sb,
25192519
spin_lock_init(&fs_info->delayed_iput_lock);
25202520
spin_lock_init(&fs_info->defrag_inodes_lock);
25212521
spin_lock_init(&fs_info->free_chunk_lock);
2522-
spin_lock_init(&fs_info->tree_mod_seq_lock);
25232522
spin_lock_init(&fs_info->super_lock);
25242523
spin_lock_init(&fs_info->qgroup_op_lock);
25252524
spin_lock_init(&fs_info->buffer_lock);

fs/btrfs/tests/btrfs-tests.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,6 @@ struct btrfs_fs_info *btrfs_alloc_dummy_fs_info(void)
112112
spin_lock_init(&fs_info->qgroup_op_lock);
113113
spin_lock_init(&fs_info->super_lock);
114114
spin_lock_init(&fs_info->fs_roots_radix_lock);
115-
spin_lock_init(&fs_info->tree_mod_seq_lock);
116115
mutex_init(&fs_info->qgroup_ioctl_lock);
117116
mutex_init(&fs_info->qgroup_rescan_lock);
118117
rwlock_init(&fs_info->tree_mod_log_lock);

0 commit comments

Comments
 (0)