stable.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: [PATCH] btrfs: unlock the newly allocated extent buffer in btrfs_alloc_tree_block()
       [not found] <20210914065759.38793-1-wqu@suse.com>
@ 2022-03-06 16:36 ` Denis Efremov
  2022-03-07  0:15   ` Qu Wenruo
  0 siblings, 1 reply; 3+ messages in thread
From: Denis Efremov @ 2022-03-06 16:36 UTC (permalink / raw)
  To: Qu Wenruo, David Sterba; +Cc: Hao Sun, linux-btrfs, stable

Hi,


On 9/14/21 09:57, Qu Wenruo wrote:
> [BUG]
...
> 
>   ================================================
>   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_alloc_tree_block(), after btrfs_init_new_buffer(), the new
> extent buffer @buf is locked, but if later operations like adding
> delayed tree ref fails, we just free @buf without unlocking it,
> resulting above warning.

This patch fixes CVE-2021-4149. Commit 19ea40dddf18
"btrfs: unlock newly allocated extent buffer after error" upstream.
The patch was backported to kernels 5.15, 5.10, 5.4 because it contains
"CC: stable@vger.kernel.org # 5.4+" in the commit message.

However, it looks to me like kernels 4.9, 4.14, 4.19 are also vulnerable.
In v4.9 kernel there is btrfs_init_new_buffer() call:
btrfs_alloc_tree_block(...)
{
	...
	buf = btrfs_init_new_buffer(trans, root, ins.objectid, level);
	...
out_free_buf:                                                                    
        free_extent_buffer(buf);
	...
}

and btrfs_init_new_buffer() contains btrfs_tree_lock(buf) inside it.

The patch can be cherry-picked to v4.9 kernel without a conflict.

Probably, the error was introduced in the commit 67b7859e9bfa
"btrfs: handle ENOMEM in btrfs_alloc_tree_block" It's in the kernel
since v4.1

Can you confirm that kernels v4.9, 4.14, 4.19 are also vulnerable?

Thanks,
Denis

> 
> [FIX]
> Unlock @buf in out_free_buf: tag.
> 
> 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/extent-tree.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index c88e7727a31a..8aa981ffe7b7 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -4898,6 +4898,7 @@ struct extent_buffer *btrfs_alloc_tree_block(struct btrfs_trans_handle *trans,
>  out_free_delayed:
>  	btrfs_free_delayed_extent_op(extent_op);
>  out_free_buf:
> +	btrfs_tree_unlock(buf);
>  	free_extent_buffer(buf);
>  out_free_reserved:
>  	btrfs_free_reserved_extent(fs_info, ins.objectid, ins.offset, 0);

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

* Re: [PATCH] btrfs: unlock the newly allocated extent buffer in btrfs_alloc_tree_block()
  2022-03-06 16:36 ` [PATCH] btrfs: unlock the newly allocated extent buffer in btrfs_alloc_tree_block() Denis Efremov
@ 2022-03-07  0:15   ` Qu Wenruo
  2022-03-09  6:47     ` [PATCH] btrfs: unlock newly allocated extent buffer after error Denis Efremov
  0 siblings, 1 reply; 3+ messages in thread
From: Qu Wenruo @ 2022-03-07  0:15 UTC (permalink / raw)
  To: Denis Efremov, Qu Wenruo, David Sterba; +Cc: Hao Sun, linux-btrfs, stable



On 2022/3/7 00:36, Denis Efremov wrote:
> Hi,
>
>
> On 9/14/21 09:57, Qu Wenruo wrote:
>> [BUG]
> ...
>>
>>    ================================================
>>    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_alloc_tree_block(), after btrfs_init_new_buffer(), the new
>> extent buffer @buf is locked, but if later operations like adding
>> delayed tree ref fails, we just free @buf without unlocking it,
>> resulting above warning.
>
> This patch fixes CVE-2021-4149. Commit 19ea40dddf18
> "btrfs: unlock newly allocated extent buffer after error" upstream.
> The patch was backported to kernels 5.15, 5.10, 5.4 because it contains
> "CC: stable@vger.kernel.org # 5.4+" in the commit message.
>
> However, it looks to me like kernels 4.9, 4.14, 4.19 are also vulnerable.
> In v4.9 kernel there is btrfs_init_new_buffer() call:
> btrfs_alloc_tree_block(...)
> {
> 	...
> 	buf = btrfs_init_new_buffer(trans, root, ins.objectid, level);
> 	...
> out_free_buf:
>          free_extent_buffer(buf);
> 	...
> }
>
> and btrfs_init_new_buffer() contains btrfs_tree_lock(buf) inside it.
>
> The patch can be cherry-picked to v4.9 kernel without a conflict.
>
> Probably, the error was introduced in the commit 67b7859e9bfa
> "btrfs: handle ENOMEM in btrfs_alloc_tree_block" It's in the kernel
> since v4.1
>
> Can you confirm that kernels v4.9, 4.14, 4.19 are also vulnerable?

Oh, thanks for catching this, I'm never good at taking care of older
kernels.

But since those three are TLS kernels, they deserve the fix.

And yes, in those three versions, they have btrfs_tree_lock() called in
btrfs_init_new_buffer(), so they are also affected.

For the cause, your commit is completely correct.

So feel free to backport those patches to stable, with your new fixed-by
tag.

Thanks,
Qu
>
> Thanks,
> Denis
>
>>
>> [FIX]
>> Unlock @buf in out_free_buf: tag.
>>
>> 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/extent-tree.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
>> index c88e7727a31a..8aa981ffe7b7 100644
>> --- a/fs/btrfs/extent-tree.c
>> +++ b/fs/btrfs/extent-tree.c
>> @@ -4898,6 +4898,7 @@ struct extent_buffer *btrfs_alloc_tree_block(struct btrfs_trans_handle *trans,
>>   out_free_delayed:
>>   	btrfs_free_delayed_extent_op(extent_op);
>>   out_free_buf:
>> +	btrfs_tree_unlock(buf);
>>   	free_extent_buffer(buf);
>>   out_free_reserved:
>>   	btrfs_free_reserved_extent(fs_info, ins.objectid, ins.offset, 0);

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

* [PATCH] btrfs: unlock newly allocated extent buffer after error
  2022-03-07  0:15   ` Qu Wenruo
@ 2022-03-09  6:47     ` Denis Efremov
  0 siblings, 0 replies; 3+ messages in thread
From: Denis Efremov @ 2022-03-09  6:47 UTC (permalink / raw)
  Cc: Qu Wenruo, Hao Sun, stable, David Sterba, Denis Efremov

From: Qu Wenruo <wqu@suse.com>

commit 19ea40dddf1833db868533958ca066f368862211 upstream.

[BUG]
There is a 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_alloc_tree_block(), after btrfs_init_new_buffer(), the new
extent buffer @buf is locked, but if later operations like adding
delayed tree ref fail, we just free @buf without unlocking it,
resulting above warning.

[FIX]
Unlock @buf in out_free_buf: label.

Reported-by: Hao Sun <sunhao.th@gmail.com>
Link: https://lore.kernel.org/linux-btrfs/CACkBjsZ9O6Zr0KK1yGn=1rQi6Crh1yeCRdTSBxx9R99L4xdn-Q@mail.gmail.com/
CC: stable@vger.kernel.org # 4.1+
Fixes: 67b7859e9bfa ("btrfs: handle ENOMEM in btrfs_alloc_tree_block")
Signed-off-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Denis Efremov <denis.e.efremov@oracle.com>
---
I added Fixes tag and changed kernel version in "CC: stable@..." line.

 fs/btrfs/extent-tree.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index bf46ed74eae6..d71f800e8bf6 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -8327,6 +8327,7 @@ struct extent_buffer *btrfs_alloc_tree_block(struct btrfs_trans_handle *trans,
 out_free_delayed:
 	btrfs_free_delayed_extent_op(extent_op);
 out_free_buf:
+	btrfs_tree_unlock(buf);
 	free_extent_buffer(buf);
 out_free_reserved:
 	btrfs_free_reserved_extent(fs_info, ins.objectid, ins.offset, 0);
-- 
2.35.1


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

end of thread, other threads:[~2022-03-09  6:48 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20210914065759.38793-1-wqu@suse.com>
2022-03-06 16:36 ` [PATCH] btrfs: unlock the newly allocated extent buffer in btrfs_alloc_tree_block() Denis Efremov
2022-03-07  0:15   ` Qu Wenruo
2022-03-09  6:47     ` [PATCH] btrfs: unlock newly allocated extent buffer after error Denis Efremov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).