Skip to content

Commit 7a16360

Browse files
fdmananakdave
authored andcommitted
btrfs: fix invalid delayed ref after subvolume creation failure
When creating a subvolume, at ioctl.c:create_subvol(), if we fail to insert the new root's root item into the root tree, we are freeing the metadata extent we reserved for the new root to prevent a metadata extent leak, as we don't abort the transaction at that point (since there is nothing at that point that is irreversible). However we allocated the metadata extent for the new root which we are creating for the new subvolume, so its delayed reference refers to the ID of this new root. But when we free the metadata extent we pass the root of the subvolume where the new subvolume is located to btrfs_free_tree_block() - this is incorrect because this will generate a delayed reference that refers to the ID of the parent subvolume's root, and not to ID of the new root. This results in a failure when running delayed references that leads to a transaction abort and a trace like the following: [3868.738042] RIP: 0010:__btrfs_free_extent+0x709/0x950 [btrfs] [3868.739857] Code: 68 0f 85 e6 fb ff (...) [3868.742963] RSP: 0018:ffffb0e9045cf910 EFLAGS: 00010246 [3868.743908] RAX: 00000000fffffffe RBX: 00000000fffffffe RCX: 0000000000000002 [3868.745312] RDX: 00000000fffffffe RSI: 0000000000000002 RDI: ffff90b0cd793b88 [3868.746643] RBP: 000000000e5d8000 R08: 0000000000000000 R09: ffff90b0cd793b88 [3868.747979] R10: 0000000000000002 R11: 00014ded97944d68 R12: 0000000000000000 [3868.749373] R13: ffff90b09afe4a28 R14: 0000000000000000 R15: ffff90b0cd793b88 [3868.750725] FS: 00007f281c4a8b80(0000) GS:ffff90b3ada00000(0000) knlGS:0000000000000000 [3868.752275] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [3868.753515] CR2: 00007f281c6a5000 CR3: 0000000108a42006 CR4: 0000000000370ee0 [3868.754869] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 [3868.756228] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 [3868.757803] Call Trace: [3868.758281] <TASK> [3868.758655] ? btrfs_merge_delayed_refs+0x178/0x1c0 [btrfs] [3868.759827] __btrfs_run_delayed_refs+0x2b1/0x1250 [btrfs] [3868.761047] btrfs_run_delayed_refs+0x86/0x210 [btrfs] [3868.762069] ? lock_acquired+0x19f/0x420 [3868.762829] btrfs_commit_transaction+0x69/0xb20 [btrfs] [3868.763860] ? _raw_spin_unlock+0x29/0x40 [3868.764614] ? btrfs_block_rsv_release+0x1c2/0x1e0 [btrfs] [3868.765870] create_subvol+0x1d8/0x9a0 [btrfs] [3868.766766] btrfs_mksubvol+0x447/0x4c0 [btrfs] [3868.767669] ? preempt_count_add+0x49/0xa0 [3868.768444] __btrfs_ioctl_snap_create+0x123/0x190 [btrfs] [3868.769639] ? _copy_from_user+0x66/0xa0 [3868.770391] btrfs_ioctl_snap_create_v2+0xbb/0x140 [btrfs] [3868.771495] btrfs_ioctl+0xd1e/0x35c0 [btrfs] [3868.772364] ? __slab_free+0x10a/0x360 [3868.773198] ? rcu_read_lock_sched_held+0x12/0x60 [3868.774121] ? lock_release+0x223/0x4a0 [3868.774863] ? lock_acquired+0x19f/0x420 [3868.775634] ? rcu_read_lock_sched_held+0x12/0x60 [3868.776530] ? trace_hardirqs_on+0x1b/0xe0 [3868.777373] ? _raw_spin_unlock_irqrestore+0x3e/0x60 [3868.778280] ? kmem_cache_free+0x321/0x3c0 [3868.779011] ? __x64_sys_ioctl+0x83/0xb0 [3868.779718] __x64_sys_ioctl+0x83/0xb0 [3868.780387] do_syscall_64+0x3b/0xc0 [3868.781059] entry_SYSCALL_64_after_hwframe+0x44/0xae [3868.781953] RIP: 0033:0x7f281c59e957 [3868.782585] Code: 3c 1c 48 f7 d8 4c (...) [3868.785867] RSP: 002b:00007ffe1f83e2b8 EFLAGS: 00000202 ORIG_RAX: 0000000000000010 [3868.787198] RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007f281c59e957 [3868.788450] RDX: 00007ffe1f83e2c0 RSI: 0000000050009418 RDI: 0000000000000003 [3868.789748] RBP: 00007ffe1f83f300 R08: 0000000000000000 R09: 00007ffe1f83fe36 [3868.791214] R10: 0000000000000000 R11: 0000000000000202 R12: 0000000000000003 [3868.792468] R13: 0000000000000003 R14: 00007ffe1f83e2c0 R15: 00000000000003cc [3868.793765] </TASK> [3868.794037] irq event stamp: 0 [3868.794548] hardirqs last enabled at (0): [<0000000000000000>] 0x0 [3868.795670] hardirqs last disabled at (0): [<ffffffff98294214>] copy_process+0x934/0x2040 [3868.797086] softirqs last enabled at (0): [<ffffffff98294214>] copy_process+0x934/0x2040 [3868.798309] softirqs last disabled at (0): [<0000000000000000>] 0x0 [3868.799284] ---[ end trace be24c7002fe27747 ]--- [3868.799928] BTRFS info (device dm-0): leaf 241188864 gen 1268 total ptrs 214 free space 469 owner 2 [3868.801133] BTRFS info (device dm-0): refs 2 lock_owner 225627 current 225627 [3868.802056] item 0 key (237436928 169 0) itemoff 16250 itemsize 33 [3868.802863] extent refs 1 gen 1265 flags 2 [3868.803447] ref#0: tree block backref root 1610 (...) [3869.064354] item 114 key (241008640 169 0) itemoff 12488 itemsize 33 [3869.065421] extent refs 1 gen 1268 flags 2 [3869.066115] ref#0: tree block backref root 1689 (...) [3869.403834] BTRFS error (device dm-0): unable to find ref byte nr 241008640 parent 0 root 1622 owner 0 offset 0 [3869.405641] BTRFS: error (device dm-0) in __btrfs_free_extent:3076: errno=-2 No such entry [3869.407138] BTRFS: error (device dm-0) in btrfs_run_delayed_refs:2159: errno=-2 No such entry Fix this by passing the new subvolume's root ID to btrfs_free_tree_block(). This requires changing the root argument of btrfs_free_tree_block() from struct btrfs_root * to a u64, since at this point during the subvolume creation we have not yet created the struct btrfs_root for the new subvolume, and btrfs_free_tree_block() only needs a root ID and nothing else from a struct btrfs_root. This was triggered by test case generic/475 from fstests. Fixes: 67addf2 ("btrfs: fix metadata extent leak after failure to create subvolume") CC: [email protected] # 4.4+ Reviewed-by: Nikolay Borisov <[email protected]> Signed-off-by: Filipe Manana <[email protected]> Signed-off-by: David Sterba <[email protected]>
1 parent 651740a commit 7a16360

File tree

6 files changed

+31
-22
lines changed

6 files changed

+31
-22
lines changed

fs/btrfs/ctree.c

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -462,8 +462,8 @@ static noinline int __btrfs_cow_block(struct btrfs_trans_handle *trans,
462462
BUG_ON(ret < 0);
463463
rcu_assign_pointer(root->node, cow);
464464

465-
btrfs_free_tree_block(trans, root, buf, parent_start,
466-
last_ref);
465+
btrfs_free_tree_block(trans, btrfs_root_id(root), buf,
466+
parent_start, last_ref);
467467
free_extent_buffer(buf);
468468
add_root_to_dirty_list(root);
469469
} else {
@@ -484,8 +484,8 @@ static noinline int __btrfs_cow_block(struct btrfs_trans_handle *trans,
484484
return ret;
485485
}
486486
}
487-
btrfs_free_tree_block(trans, root, buf, parent_start,
488-
last_ref);
487+
btrfs_free_tree_block(trans, btrfs_root_id(root), buf,
488+
parent_start, last_ref);
489489
}
490490
if (unlock_orig)
491491
btrfs_tree_unlock(buf);
@@ -926,7 +926,7 @@ static noinline int balance_level(struct btrfs_trans_handle *trans,
926926
free_extent_buffer(mid);
927927

928928
root_sub_used(root, mid->len);
929-
btrfs_free_tree_block(trans, root, mid, 0, 1);
929+
btrfs_free_tree_block(trans, btrfs_root_id(root), mid, 0, 1);
930930
/* once for the root ptr */
931931
free_extent_buffer_stale(mid);
932932
return 0;
@@ -985,7 +985,8 @@ static noinline int balance_level(struct btrfs_trans_handle *trans,
985985
btrfs_tree_unlock(right);
986986
del_ptr(root, path, level + 1, pslot + 1);
987987
root_sub_used(root, right->len);
988-
btrfs_free_tree_block(trans, root, right, 0, 1);
988+
btrfs_free_tree_block(trans, btrfs_root_id(root), right,
989+
0, 1);
989990
free_extent_buffer_stale(right);
990991
right = NULL;
991992
} else {
@@ -1030,7 +1031,7 @@ static noinline int balance_level(struct btrfs_trans_handle *trans,
10301031
btrfs_tree_unlock(mid);
10311032
del_ptr(root, path, level + 1, pslot);
10321033
root_sub_used(root, mid->len);
1033-
btrfs_free_tree_block(trans, root, mid, 0, 1);
1034+
btrfs_free_tree_block(trans, btrfs_root_id(root), mid, 0, 1);
10341035
free_extent_buffer_stale(mid);
10351036
mid = NULL;
10361037
} else {
@@ -4031,7 +4032,7 @@ static noinline void btrfs_del_leaf(struct btrfs_trans_handle *trans,
40314032
root_sub_used(root, leaf->len);
40324033

40334034
atomic_inc(&leaf->refs);
4034-
btrfs_free_tree_block(trans, root, leaf, 0, 1);
4035+
btrfs_free_tree_block(trans, btrfs_root_id(root), leaf, 0, 1);
40354036
free_extent_buffer_stale(leaf);
40364037
}
40374038
/*

fs/btrfs/ctree.h

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2257,6 +2257,11 @@ static inline bool btrfs_root_dead(const struct btrfs_root *root)
22572257
return (root->root_item.flags & cpu_to_le64(BTRFS_ROOT_SUBVOL_DEAD)) != 0;
22582258
}
22592259

2260+
static inline u64 btrfs_root_id(const struct btrfs_root *root)
2261+
{
2262+
return root->root_key.objectid;
2263+
}
2264+
22602265
/* struct btrfs_root_backup */
22612266
BTRFS_SETGET_STACK_FUNCS(backup_tree_root, struct btrfs_root_backup,
22622267
tree_root, 64);
@@ -2719,7 +2724,7 @@ struct extent_buffer *btrfs_alloc_tree_block(struct btrfs_trans_handle *trans,
27192724
u64 empty_size,
27202725
enum btrfs_lock_nesting nest);
27212726
void btrfs_free_tree_block(struct btrfs_trans_handle *trans,
2722-
struct btrfs_root *root,
2727+
u64 root_id,
27232728
struct extent_buffer *buf,
27242729
u64 parent, int last_ref);
27252730
int btrfs_alloc_reserved_file_extent(struct btrfs_trans_handle *trans,

fs/btrfs/extent-tree.c

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3275,20 +3275,20 @@ static noinline int check_ref_cleanup(struct btrfs_trans_handle *trans,
32753275
}
32763276

32773277
void btrfs_free_tree_block(struct btrfs_trans_handle *trans,
3278-
struct btrfs_root *root,
3278+
u64 root_id,
32793279
struct extent_buffer *buf,
32803280
u64 parent, int last_ref)
32813281
{
3282-
struct btrfs_fs_info *fs_info = root->fs_info;
3282+
struct btrfs_fs_info *fs_info = trans->fs_info;
32833283
struct btrfs_ref generic_ref = { 0 };
32843284
int ret;
32853285

32863286
btrfs_init_generic_ref(&generic_ref, BTRFS_DROP_DELAYED_REF,
32873287
buf->start, buf->len, parent);
32883288
btrfs_init_tree_ref(&generic_ref, btrfs_header_level(buf),
3289-
root->root_key.objectid, 0, false);
3289+
root_id, 0, false);
32903290

3291-
if (root->root_key.objectid != BTRFS_TREE_LOG_OBJECTID) {
3291+
if (root_id != BTRFS_TREE_LOG_OBJECTID) {
32923292
btrfs_ref_tree_mod(fs_info, &generic_ref);
32933293
ret = btrfs_add_delayed_tree_ref(trans, &generic_ref, NULL);
32943294
BUG_ON(ret); /* -ENOMEM */
@@ -3298,7 +3298,7 @@ void btrfs_free_tree_block(struct btrfs_trans_handle *trans,
32983298
struct btrfs_block_group *cache;
32993299
bool must_pin = false;
33003300

3301-
if (root->root_key.objectid != BTRFS_TREE_LOG_OBJECTID) {
3301+
if (root_id != BTRFS_TREE_LOG_OBJECTID) {
33023302
ret = check_ref_cleanup(trans, buf->start);
33033303
if (!ret) {
33043304
btrfs_redirty_list_add(trans->transaction, buf);
@@ -5472,7 +5472,8 @@ static noinline int walk_up_proc(struct btrfs_trans_handle *trans,
54725472
goto owner_mismatch;
54735473
}
54745474

5475-
btrfs_free_tree_block(trans, root, eb, parent, wc->refs[level] == 1);
5475+
btrfs_free_tree_block(trans, btrfs_root_id(root), eb, parent,
5476+
wc->refs[level] == 1);
54765477
out:
54775478
wc->refs[level] = 0;
54785479
wc->flags[level] = 0;

fs/btrfs/free-space-tree.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1256,8 +1256,8 @@ int btrfs_clear_free_space_tree(struct btrfs_fs_info *fs_info)
12561256
btrfs_tree_lock(free_space_root->node);
12571257
btrfs_clean_tree_block(free_space_root->node);
12581258
btrfs_tree_unlock(free_space_root->node);
1259-
btrfs_free_tree_block(trans, free_space_root, free_space_root->node,
1260-
0, 1);
1259+
btrfs_free_tree_block(trans, btrfs_root_id(free_space_root),
1260+
free_space_root->node, 0, 1);
12611261

12621262
btrfs_put_root(free_space_root);
12631263

fs/btrfs/ioctl.c

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -617,11 +617,12 @@ static noinline int create_subvol(struct user_namespace *mnt_userns,
617617
* Since we don't abort the transaction in this case, free the
618618
* tree block so that we don't leak space and leave the
619619
* filesystem in an inconsistent state (an extent item in the
620-
* extent tree without backreferences). Also no need to have
621-
* the tree block locked since it is not in any tree at this
622-
* point, so no other task can find it and use it.
620+
* extent tree with a backreference for a root that does not
621+
* exists). Also no need to have the tree block locked since it
622+
* is not in any tree at this point, so no other task can find
623+
* it and use it.
623624
*/
624-
btrfs_free_tree_block(trans, root, leaf, 0, 1);
625+
btrfs_free_tree_block(trans, objectid, leaf, 0, 1);
625626
free_extent_buffer(leaf);
626627
goto fail;
627628
}

fs/btrfs/qgroup.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1219,7 +1219,8 @@ int btrfs_quota_disable(struct btrfs_fs_info *fs_info)
12191219
btrfs_tree_lock(quota_root->node);
12201220
btrfs_clean_tree_block(quota_root->node);
12211221
btrfs_tree_unlock(quota_root->node);
1222-
btrfs_free_tree_block(trans, quota_root, quota_root->node, 0, 1);
1222+
btrfs_free_tree_block(trans, btrfs_root_id(quota_root),
1223+
quota_root->node, 0, 1);
12231224

12241225
btrfs_put_root(quota_root);
12251226

0 commit comments

Comments
 (0)