public inbox for linux-btrfs@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] btrfs: fix a use-after-free race if btrfs_open_devices() failed
@ 2025-06-27  0:22 Qu Wenruo
  2025-06-27  1:09 ` David Sterba
  0 siblings, 1 reply; 3+ messages in thread
From: Qu Wenruo @ 2025-06-27  0:22 UTC (permalink / raw)
  To: linux-btrfs; +Cc: syzbot+772bdfe41846e057fa83

[BUG]
With the latest v5 version patchset "btrfs: use fs_holder_ops for btrfs"
merged into linux-next, syzbot reported an use-after-free:

==================================================================
BUG: KASAN: slab-use-after-free in close_fs_devices+0x81f/0x870 fs/btrfs/volumes.c:1182
Read of size 4 at addr ffff88802fe14930 by task syz.4.616/8589

CPU: 0 UID: 0 PID: 8589 Comm: syz.4.616 Not tainted 6.16.0-rc3-next-20250626-syzkaller #0 PREEMPT(full)
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 05/07/2025
Call Trace:
 <TASK>
 dump_stack_lvl+0x189/0x250 lib/dump_stack.c:120
 print_address_description mm/kasan/report.c:408 [inline]
 print_report+0xd2/0x2b0 mm/kasan/report.c:521
 kasan_report+0x118/0x150 mm/kasan/report.c:634
 close_fs_devices+0x81f/0x870 fs/btrfs/volumes.c:1182
 btrfs_close_devices+0xc5/0x560 fs/btrfs/volumes.c:1201
 btrfs_free_fs_info+0x4f/0x3c0 fs/btrfs/disk-io.c:1250
 deactivate_locked_super+0xbc/0x130 fs/super.c:474
 btrfs_get_tree_super fs/btrfs/super.c:-1 [inline]
 btrfs_get_tree_subvol fs/btrfs/super.c:2073 [inline]
 btrfs_get_tree+0xd1e/0x17f0 fs/btrfs/super.c:2107
 vfs_get_tree+0x92/0x2b0 fs/super.c:1804
 do_new_mount+0x24a/0xa40 fs/namespace.c:3902
 do_mount fs/namespace.c:4239 [inline]
 __do_sys_mount fs/namespace.c:4450 [inline]
 __se_sys_mount+0x317/0x410 fs/namespace.c:4427
 do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
 do_syscall_64+0xfa/0x3b0 arch/x86/entry/syscall_64.c:94
 entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7fedccd900ca
Code: d8 64 89 02 48 c7 c0 ff ff ff ff eb a6 e8 de 1a 00 00 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 00 49 89 ca b8 a5 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 a8 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007fedcdc28e68 EFLAGS: 00000246 ORIG_RAX: 00000000000000a5
RAX: ffffffffffffffda RBX: 00007fedcdc28ef0 RCX: 00007fedccd900ca
RDX: 00002000000055c0 RSI: 0000200000005600 RDI: 00007fedcdc28eb0
RBP: 00002000000055c0 R08: 00007fedcdc28ef0 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 0000200000005600
R13: 00007fedcdc28eb0 R14: 000000000000559d R15: 0000200000000440
 </TASK>

Allocated by task 8589:
 kasan_save_stack mm/kasan/common.c:47 [inline]
 kasan_save_track+0x3e/0x80 mm/kasan/common.c:68
 poison_kmalloc_redzone mm/kasan/common.c:377 [inline]
 __kasan_kmalloc+0x93/0xb0 mm/kasan/common.c:394
 kasan_kmalloc include/linux/kasan.h:260 [inline]
 __kmalloc_cache_noprof+0x230/0x3d0 mm/slub.c:4396
 kmalloc_noprof include/linux/slab.h:905 [inline]
 kzalloc_noprof include/linux/slab.h:1039 [inline]
 alloc_fs_devices+0x4f/0x1d0 fs/btrfs/volumes.c:384
 device_list_add+0x6b7/0x20b0 fs/btrfs/volumes.c:813
 btrfs_scan_one_device+0x3fd/0x5b0 fs/btrfs/volumes.c:1487
 btrfs_get_tree_super fs/btrfs/super.c:1856 [inline]
 btrfs_get_tree_subvol fs/btrfs/super.c:2073 [inline]
 btrfs_get_tree+0x433/0x17f0 fs/btrfs/super.c:2107
 vfs_get_tree+0x92/0x2b0 fs/super.c:1804
 do_new_mount+0x24a/0xa40 fs/namespace.c:3902
 do_mount fs/namespace.c:4239 [inline]
 __do_sys_mount fs/namespace.c:4450 [inline]
 __se_sys_mount+0x317/0x410 fs/namespace.c:4427
 do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
 do_syscall_64+0xfa/0x3b0 arch/x86/entry/syscall_64.c:94
 entry_SYSCALL_64_after_hwframe+0x77/0x7f

Freed by task 7454:
 kasan_save_stack mm/kasan/common.c:47 [inline]
 kasan_save_track+0x3e/0x80 mm/kasan/common.c:68
 kasan_save_free_info+0x46/0x50 mm/kasan/generic.c:576
 poison_slab_object mm/kasan/common.c:247 [inline]
 __kasan_slab_free+0x62/0x70 mm/kasan/common.c:264
 kasan_slab_free include/linux/kasan.h:233 [inline]
 slab_free_hook mm/slub.c:2417 [inline]
 slab_free mm/slub.c:4680 [inline]
 kfree+0x18e/0x440 mm/slub.c:4879
 btrfs_free_stale_devices+0x61c/0x6b0 fs/btrfs/volumes.c:564
 btrfs_scan_one_device+0x3d5/0x5b0 fs/btrfs/volumes.c:1481
 btrfs_control_ioctl+0x11f/0x360 fs/btrfs/super.c:2256
 vfs_ioctl fs/ioctl.c:51 [inline]
 __do_sys_ioctl fs/ioctl.c:907 [inline]
 __se_sys_ioctl+0xf9/0x170 fs/ioctl.c:893
 do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
 do_syscall_64+0xfa/0x3b0 arch/x86/entry/syscall_64.c:94
 entry_SYSCALL_64_after_hwframe+0x77/0x7f

[CAUSE]
The patch "btrfs: delay btrfs_open_devices() until super block is created"
changed the timing when btrfs_open_devices() is called.

Now it's called after sget_fc(), the changed timing is required for
using super blocks as bdev holder.

The problem is the changed error handling.

Before the change if btrfs_open_devices() failed we error out directly,
but now we need to call deactivate_locked_super().

However since btrfs_open_devices() failed, fs_devices->opened is still
0, meaning any reclaim request can free the fs_devices.

But at the same time, deactivate_locked_super() will also cleanup the
fs_info with btrfs_free_fs_info(), which calls btrfs_close_devices().

This leads to the following race:

       Mount process              |         Scan process
----------------------------------+--------------------------------
btrfs_get_tree_super()            |
|- mutex_lock(&uuid_mutex)        |
|- btrfs_open_devices()           |
|  Which failed.                  |
|  fs_devices->opened is still 0. |
|- mutex_unlock(&uuid_mutex)      |
|                                 | btrfs_control_ioctl()
|                                 | |- btrfs_scan_one_device()
|                                 |    |- btrfs_free_stale_devices()
|                                 |       That fs_devices is freed
|- deactivate_locked_super()      |
   |- btrfs_free_fs_info()        |
      |- btrfs_close_devices()
         Now try to free the same
	 fs_devices that is freed
	 by the scan process.

[FIX]
If btrfs_open_devices() failed, we should not keep a pointer to it, as
it can be freed at any time after uuid_mutex unlocked.

So add an extra handling for btrfs_open_devices() to reset
fs_info->fs_devices to NULL.

This will be folded into the patch "btrfs: delay btrfs_open_devices()
until super block is created".

Reported-by: syzbot+772bdfe41846e057fa83@syzkaller.appspotmail.com
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 fs/btrfs/super.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 3fba3d6309a2..964cf6f260e9 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -1921,6 +1921,17 @@ static int btrfs_get_tree_super(struct fs_context *fc)
 		mutex_lock(&uuid_mutex);
 		btrfs_fs_devices_dec_holding(fs_devices);
 		ret = btrfs_open_devices(fs_devices, mode, sb);
+		/*
+		 * If btrfs_open_devices() failed, fs_devices is not opened and
+		 * can be freed by any reclaim request after uuid_mutex unlocked.
+		 *
+		 * But our fs_info is still using that fs_devices, thus it will
+		 * lead to use-after-free later.
+		 *
+		 * So here we must not use that fs_devices after open failure.
+		 */
+		if (ret < 0)
+			fs_info->fs_devices = NULL;
 		mutex_unlock(&uuid_mutex);
 		if (ret < 0) {
 			deactivate_locked_super(sb);
-- 
2.49.0


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

* Re: [PATCH] btrfs: fix a use-after-free race if btrfs_open_devices() failed
  2025-06-27  0:22 [PATCH] btrfs: fix a use-after-free race if btrfs_open_devices() failed Qu Wenruo
@ 2025-06-27  1:09 ` David Sterba
  2025-06-27  2:37   ` Qu Wenruo
  0 siblings, 1 reply; 3+ messages in thread
From: David Sterba @ 2025-06-27  1:09 UTC (permalink / raw)
  To: Qu Wenruo; +Cc: linux-btrfs, syzbot+772bdfe41846e057fa83

On Fri, Jun 27, 2025 at 09:52:58AM +0930, Qu Wenruo wrote:
> [BUG]
> With the latest v5 version patchset "btrfs: use fs_holder_ops for btrfs"
> merged into linux-next, syzbot reported an use-after-free:
...
> This will be folded into the patch "btrfs: delay btrfs_open_devices()
> until super block is created".

Thanks, patch added to linux-next branch on top of the series for
clarity, for the next iteration it can be folded.

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

* Re: [PATCH] btrfs: fix a use-after-free race if btrfs_open_devices() failed
  2025-06-27  1:09 ` David Sterba
@ 2025-06-27  2:37   ` Qu Wenruo
  0 siblings, 0 replies; 3+ messages in thread
From: Qu Wenruo @ 2025-06-27  2:37 UTC (permalink / raw)
  To: dsterba, Qu Wenruo; +Cc: linux-btrfs, syzbot+772bdfe41846e057fa83



在 2025/6/27 10:39, David Sterba 写道:
> On Fri, Jun 27, 2025 at 09:52:58AM +0930, Qu Wenruo wrote:
>> [BUG]
>> With the latest v5 version patchset "btrfs: use fs_holder_ops for btrfs"
>> merged into linux-next, syzbot reported an use-after-free:
> ...
>> This will be folded into the patch "btrfs: delay btrfs_open_devices()
>> until super block is created".
> 
> Thanks, patch added to linux-next branch on top of the series for
> clarity, for the next iteration it can be folded.
> 

It turns out there are more locations needs similar handling.

I'll need to send out a v2 update for this hot fix unfortunately...

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

end of thread, other threads:[~2025-06-27  2:37 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-27  0:22 [PATCH] btrfs: fix a use-after-free race if btrfs_open_devices() failed Qu Wenruo
2025-06-27  1:09 ` David Sterba
2025-06-27  2:37   ` Qu Wenruo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox