* [PATCH] btrfs: unlock the original extent buffer when error happens in __btrfs_cow_block()
@ 2021-09-14 5:55 Qu Wenruo
2021-09-14 6:24 ` Qu Wenruo
0 siblings, 1 reply; 2+ messages in thread
From: Qu Wenruo @ 2021-09-14 5:55 UTC (permalink / raw)
To: linux-btrfs; +Cc: Hao Sun
[BUG]
There is a very detailed bug report that injected ENOMEM error could
leave a tree block locked while we return to user-space:
BTRFS info (device loop0): enabling ssd optimizations
FAULT_INJECTION: forcing a failure.
name failslab, interval 1, probability 0, space 0, times 0
CPU: 0 PID: 7579 Comm: syz-executor Not tainted 5.15.0-rc1 #16
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
rel-1.12.0-59-gc9ba5276e321-prebuilt.qemu.org 04/01/2014
Call Trace:
__dump_stack lib/dump_stack.c:88 [inline]
dump_stack_lvl+0x8d/0xcf lib/dump_stack.c:106
fail_dump lib/fault-inject.c:52 [inline]
should_fail+0x13c/0x160 lib/fault-inject.c:146
should_failslab+0x5/0x10 mm/slab_common.c:1328
slab_pre_alloc_hook.constprop.99+0x4e/0xc0 mm/slab.h:494
slab_alloc_node mm/slub.c:3120 [inline]
slab_alloc mm/slub.c:3214 [inline]
kmem_cache_alloc+0x44/0x280 mm/slub.c:3219
btrfs_alloc_delayed_extent_op fs/btrfs/delayed-ref.h:299 [inline]
btrfs_alloc_tree_block+0x38c/0x670 fs/btrfs/extent-tree.c:4833
__btrfs_cow_block+0x16f/0x7d0 fs/btrfs/ctree.c:415
btrfs_cow_block+0x12a/0x300 fs/btrfs/ctree.c:570
btrfs_search_slot+0x6b0/0xee0 fs/btrfs/ctree.c:1768
btrfs_insert_empty_items+0x80/0xf0 fs/btrfs/ctree.c:3905
btrfs_new_inode+0x311/0xa60 fs/btrfs/inode.c:6530
btrfs_create+0x12b/0x270 fs/btrfs/inode.c:6783
lookup_open+0x660/0x780 fs/namei.c:3282
open_last_lookups fs/namei.c:3352 [inline]
path_openat+0x465/0xe20 fs/namei.c:3557
do_filp_open+0xe3/0x170 fs/namei.c:3588
do_sys_openat2+0x357/0x4a0 fs/open.c:1200
do_sys_open+0x87/0xd0 fs/open.c:1216
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x34/0xb0 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x44/0xae
RIP: 0033:0x46ae99
Code: f7 d8 64 89 02 b8 ff ff ff ff c3 66 0f 1f 44 00 00 48 89 f8 48
89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d
01 f0 ff ff 73 01 c3 48 c7 c1 bc ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007f46711b9c48 EFLAGS: 00000246 ORIG_RAX: 0000000000000055
RAX: ffffffffffffffda RBX: 000000000078c0a0 RCX: 000000000046ae99
RDX: 0000000000000000 RSI: 00000000000000a1 RDI: 0000000020005800
RBP: 00007f46711b9c80 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000017
R13: 0000000000000000 R14: 000000000078c0a0 R15: 00007ffc129da6e0
================================================
WARNING: lock held when returning to user space!
5.15.0-rc1 #16 Not tainted
------------------------------------------------
syz-executor/7579 is leaving the kernel with locks still held!
1 lock held by syz-executor/7579:
#0: ffff888104b73da8 (btrfs-tree-01/1){+.+.}-{3:3}, at:
__btrfs_tree_lock+0x2e/0x1a0 fs/btrfs/locking.c:112
[CAUSE]
In __btrfs_cow_block() we could have a case where buf == *cow_ret, this
is the common call pattern in btrfs_search_slow().
In that case, before we return we should unlock the original buffer.
As in the btrfs_search_slot() call site:
if (last_level)
err = btrfs_cow_block(trans, root, b, NULL, 0,
&b,
BTRFS_NESTING_COW);
else
err = btrfs_cow_block(trans, root, b,
p->nodes[level + 1],
p->slots[level + 1], &b,
BTRFS_NESTING_COW);
btrfs_search_slot() expects btrfs_cow_block() to unlock the original
extent buffer @b.
As btrfs_search_slot() only puts the cowed tree block into path @p, thus
if btrfs_cow_block() fails, there will be no one to unlock extent buffer
@b.
[FIX]
Add unlock_orig check for all error paths in __btrfs_cow_block().
Reported-by: Hao Sun <sunhao.th@gmail.com>
Link: https://lore.kernel.org/linux-btrfs/CACkBjsZ9O6Zr0KK1yGn=1rQi6Crh1yeCRdTSBxx9R99L4xdn-Q@mail.gmail.com/
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
fs/btrfs/ctree.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
index 84627cbd5b5b..5cbbeb8384c7 100644
--- a/fs/btrfs/ctree.c
+++ b/fs/btrfs/ctree.c
@@ -415,8 +415,11 @@ static noinline int __btrfs_cow_block(struct btrfs_trans_handle *trans,
cow = btrfs_alloc_tree_block(trans, root, parent_start,
root->root_key.objectid, &disk_key, level,
search_start, empty_size, nest);
- if (IS_ERR(cow))
+ if (IS_ERR(cow)) {
+ if (unlock_orig)
+ btrfs_tree_unlock(buf);
return PTR_ERR(cow);
+ }
/* cow is set to blocking by btrfs_init_new_buffer */
@@ -436,6 +439,8 @@ static noinline int __btrfs_cow_block(struct btrfs_trans_handle *trans,
ret = update_ref_for_cow(trans, root, buf, cow, &last_ref);
if (ret) {
btrfs_tree_unlock(cow);
+ if (unlock_orig)
+ btrfs_tree_unlock(buf);
free_extent_buffer(cow);
btrfs_abort_transaction(trans, ret);
return ret;
@@ -445,6 +450,8 @@ static noinline int __btrfs_cow_block(struct btrfs_trans_handle *trans,
ret = btrfs_reloc_cow_block(trans, root, buf, cow);
if (ret) {
btrfs_tree_unlock(cow);
+ if (unlock_orig)
+ btrfs_tree_unlock(buf);
free_extent_buffer(cow);
btrfs_abort_transaction(trans, ret);
return ret;
@@ -479,6 +486,8 @@ static noinline int __btrfs_cow_block(struct btrfs_trans_handle *trans,
ret = btrfs_tree_mod_log_free_eb(buf);
if (ret) {
btrfs_tree_unlock(cow);
+ if (unlock_orig)
+ btrfs_tree_unlock(buf);
free_extent_buffer(cow);
btrfs_abort_transaction(trans, ret);
return ret;
--
2.33.0
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] btrfs: unlock the original extent buffer when error happens in __btrfs_cow_block()
2021-09-14 5:55 [PATCH] btrfs: unlock the original extent buffer when error happens in __btrfs_cow_block() Qu Wenruo
@ 2021-09-14 6:24 ` Qu Wenruo
0 siblings, 0 replies; 2+ messages in thread
From: Qu Wenruo @ 2021-09-14 6:24 UTC (permalink / raw)
To: linux-btrfs; +Cc: Hao Sun
On 2021/9/14 下午1:55, Qu Wenruo wrote:
> [BUG]
> There is a very detailed bug report that injected ENOMEM error could
> leave a tree block locked while we return to user-space:
>
> BTRFS info (device loop0): enabling ssd optimizations
> FAULT_INJECTION: forcing a failure.
> name failslab, interval 1, probability 0, space 0, times 0
> CPU: 0 PID: 7579 Comm: syz-executor Not tainted 5.15.0-rc1 #16
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
> rel-1.12.0-59-gc9ba5276e321-prebuilt.qemu.org 04/01/2014
> Call Trace:
> __dump_stack lib/dump_stack.c:88 [inline]
> dump_stack_lvl+0x8d/0xcf lib/dump_stack.c:106
> fail_dump lib/fault-inject.c:52 [inline]
> should_fail+0x13c/0x160 lib/fault-inject.c:146
> should_failslab+0x5/0x10 mm/slab_common.c:1328
> slab_pre_alloc_hook.constprop.99+0x4e/0xc0 mm/slab.h:494
> slab_alloc_node mm/slub.c:3120 [inline]
> slab_alloc mm/slub.c:3214 [inline]
> kmem_cache_alloc+0x44/0x280 mm/slub.c:3219
> btrfs_alloc_delayed_extent_op fs/btrfs/delayed-ref.h:299 [inline]
> btrfs_alloc_tree_block+0x38c/0x670 fs/btrfs/extent-tree.c:4833
> __btrfs_cow_block+0x16f/0x7d0 fs/btrfs/ctree.c:415
> btrfs_cow_block+0x12a/0x300 fs/btrfs/ctree.c:570
> btrfs_search_slot+0x6b0/0xee0 fs/btrfs/ctree.c:1768
> btrfs_insert_empty_items+0x80/0xf0 fs/btrfs/ctree.c:3905
> btrfs_new_inode+0x311/0xa60 fs/btrfs/inode.c:6530
> btrfs_create+0x12b/0x270 fs/btrfs/inode.c:6783
> lookup_open+0x660/0x780 fs/namei.c:3282
> open_last_lookups fs/namei.c:3352 [inline]
> path_openat+0x465/0xe20 fs/namei.c:3557
> do_filp_open+0xe3/0x170 fs/namei.c:3588
> do_sys_openat2+0x357/0x4a0 fs/open.c:1200
> do_sys_open+0x87/0xd0 fs/open.c:1216
> do_syscall_x64 arch/x86/entry/common.c:50 [inline]
> do_syscall_64+0x34/0xb0 arch/x86/entry/common.c:80
> entry_SYSCALL_64_after_hwframe+0x44/0xae
> RIP: 0033:0x46ae99
> Code: f7 d8 64 89 02 b8 ff ff ff ff c3 66 0f 1f 44 00 00 48 89 f8 48
> 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d
> 01 f0 ff ff 73 01 c3 48 c7 c1 bc ff ff ff f7 d8 64 89 01 48
> RSP: 002b:00007f46711b9c48 EFLAGS: 00000246 ORIG_RAX: 0000000000000055
> RAX: ffffffffffffffda RBX: 000000000078c0a0 RCX: 000000000046ae99
> RDX: 0000000000000000 RSI: 00000000000000a1 RDI: 0000000020005800
> RBP: 00007f46711b9c80 R08: 0000000000000000 R09: 0000000000000000
> R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000017
> R13: 0000000000000000 R14: 000000000078c0a0 R15: 00007ffc129da6e0
>
> ================================================
> WARNING: lock held when returning to user space!
> 5.15.0-rc1 #16 Not tainted
> ------------------------------------------------
> syz-executor/7579 is leaving the kernel with locks still held!
> 1 lock held by syz-executor/7579:
> #0: ffff888104b73da8 (btrfs-tree-01/1){+.+.}-{3:3}, at:
> __btrfs_tree_lock+0x2e/0x1a0 fs/btrfs/locking.c:112
>
> [CAUSE]
> In __btrfs_cow_block() we could have a case where buf == *cow_ret, this
> is the common call pattern in btrfs_search_slow().
>
> In that case, before we return we should unlock the original buffer.
>
> As in the btrfs_search_slot() call site:
>
> if (last_level)
> err = btrfs_cow_block(trans, root, b, NULL, 0,
> &b,
> BTRFS_NESTING_COW);
> else
> err = btrfs_cow_block(trans, root, b,
> p->nodes[level + 1],
> p->slots[level + 1], &b,
> BTRFS_NESTING_COW);
>
> btrfs_search_slot() expects btrfs_cow_block() to unlock the original
> extent buffer @b.
>
> As btrfs_search_slot() only puts the cowed tree block into path @p, thus
> if btrfs_cow_block() fails, there will be no one to unlock extent buffer
> @b.
>
> [FIX]
> Add unlock_orig check for all error paths in __btrfs_cow_block().
The patch is causing btrfs/010 to hang, it looks like there are some
non-error path that we shouldn't unlock the original buf.
Will update the fix soon.
Thanks,
Qu
>
> Reported-by: Hao Sun <sunhao.th@gmail.com>
> Link: https://lore.kernel.org/linux-btrfs/CACkBjsZ9O6Zr0KK1yGn=1rQi6Crh1yeCRdTSBxx9R99L4xdn-Q@mail.gmail.com/
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
> fs/btrfs/ctree.c | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
> index 84627cbd5b5b..5cbbeb8384c7 100644
> --- a/fs/btrfs/ctree.c
> +++ b/fs/btrfs/ctree.c
> @@ -415,8 +415,11 @@ static noinline int __btrfs_cow_block(struct btrfs_trans_handle *trans,
> cow = btrfs_alloc_tree_block(trans, root, parent_start,
> root->root_key.objectid, &disk_key, level,
> search_start, empty_size, nest);
> - if (IS_ERR(cow))
> + if (IS_ERR(cow)) {
> + if (unlock_orig)
> + btrfs_tree_unlock(buf);
> return PTR_ERR(cow);
> + }
>
> /* cow is set to blocking by btrfs_init_new_buffer */
>
> @@ -436,6 +439,8 @@ static noinline int __btrfs_cow_block(struct btrfs_trans_handle *trans,
> ret = update_ref_for_cow(trans, root, buf, cow, &last_ref);
> if (ret) {
> btrfs_tree_unlock(cow);
> + if (unlock_orig)
> + btrfs_tree_unlock(buf);
> free_extent_buffer(cow);
> btrfs_abort_transaction(trans, ret);
> return ret;
> @@ -445,6 +450,8 @@ static noinline int __btrfs_cow_block(struct btrfs_trans_handle *trans,
> ret = btrfs_reloc_cow_block(trans, root, buf, cow);
> if (ret) {
> btrfs_tree_unlock(cow);
> + if (unlock_orig)
> + btrfs_tree_unlock(buf);
> free_extent_buffer(cow);
> btrfs_abort_transaction(trans, ret);
> return ret;
> @@ -479,6 +486,8 @@ static noinline int __btrfs_cow_block(struct btrfs_trans_handle *trans,
> ret = btrfs_tree_mod_log_free_eb(buf);
> if (ret) {
> btrfs_tree_unlock(cow);
> + if (unlock_orig)
> + btrfs_tree_unlock(buf);
> free_extent_buffer(cow);
> btrfs_abort_transaction(trans, ret);
> return ret;
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2021-09-14 6:24 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-14 5:55 [PATCH] btrfs: unlock the original extent buffer when error happens in __btrfs_cow_block() Qu Wenruo
2021-09-14 6:24 ` Qu Wenruo
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.