Re: [RFC][PATCH] btrfs_get_tree_subvol(): switch from fc_mount() to vfs_create_mount()

Klara Modin
2025-05-06T13:36:08Z

[-- Attachment #1: Type: text/plain, Size: 9130 bytes --]

Hi,

On 2025-05-05 04:03:45 +0100, Al Viro wrote:
> it's simpler to do btrfs_reconfigure_for_mount() right after vfs_get_tree() -
> no need to mess with ->s_umount.
>
> Objections?
>

I hit an oops on today's next-20250506 which seems to point here, and
reverting makes it go away.

Let me know if there's anything else you need.

Regards,
Klara Modin

BUG: kernel NULL pointer dereference, address: 0000000000000068
#PF: supervisor read access in kernel mode
#PF: error_code(0x0000) - not-present page
PGD 0 P4D 0
Oops: Oops: 0000 [#1] SMP NOPTI
CPU: 0 UID: 0 PID: 154 Comm: mount Not tainted 6.15.0-rc5-next-20250506 #495 PREEMPTLAZY
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS edk2-stable202408-prebuilt.qemu.org 08/13/2024
RIP: 0010:btrfs_get_tree (fs/btrfs/super.c:2065 fs/btrfs/super.c:2106)
Code: 24 e8 40 3d 96 00 8b 1c 24 e9 e9 fd ff ff f6 83 a8 00 00 00 01 75 0e 48 8b 43 60 48 8b 40 68 f6 40 50 01 75 48 49 8b 44 24 60 <48> 8b 78 68 48 83 c7 70 e8 2f c2 b3 ff 4c 89 e7 e8 d7 87 e5 ff 49
All code
========
0: 24 e8 and $0xe8,%al
2: 40 3d 96 00 8b 1c rex cmp $0x1c8b0096,%eax
8: 24 e9 and $0xe9,%al
a: e9 fd ff ff f6 jmp 0xfffffffff700000c
f: 83 a8 00 00 00 01 75 subl $0x75,0x1000000(%rax)
16: 0e (bad)
17: 48 8b 43 60 mov 0x60(%rbx),%rax
1b: 48 8b 40 68 mov 0x68(%rax),%rax
1f: f6 40 50 01 testb $0x1,0x50(%rax)
23: 75 48 jne 0x6d
25: 49 8b 44 24 60 mov 0x60(%r12),%rax
2a:* 48 8b 78 68 mov 0x68(%rax),%rdi <-- trapping instruction
2e: 48 83 c7 70 add $0x70,%rdi
32: e8 2f c2 b3 ff call 0xffffffffffb3c266
37: 4c 89 e7 mov %r12,%rdi
3a: e8 d7 87 e5 ff call 0xffffffffffe58816
3f: 49 rex.WB

Code starting with the faulting instruction
===========================================
0: 48 8b 78 68 mov 0x68(%rax),%rdi
4: 48 83 c7 70 add $0x70,%rdi
8: e8 2f c2 b3 ff call 0xffffffffffb3c23c
d: 4c 89 e7 mov %r12,%rdi
10: e8 d7 87 e5 ff call 0xffffffffffe587ec
15: 49 rex.WB
RSP: 0018:ffffc15200583df8 EFLAGS: 00010246
RAX: 0000000000000000 RBX: ffff9f49450f7e40 RCX: 0000000000000000
RDX: 7fffffffffffffff RSI: 0000000000000000 RDI: ffffffffaf773b50
RBP: 0000000000000000 R08: ffff9f4941c5dca8 R09: ffff9f4942054d00
R10: ffffffffad2c3ce2 R11: ffff9f494482e000 R12: ffff9f4945224a80
R13: 0000000000000000 R14: ffff9f4944957f40 R15: 00000000ffffffff
FS: 00007f9b03861b80(0000) GS:ffff9f49ccd7b000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000000068 CR3: 0000000005385000 CR4: 0000000000350ef0
Call Trace:
<TASK>
? vfs_parse_fs_string (fs/fs_context.c:191)
vfs_get_tree (fs/super.c:1810)
path_mount (fs/namespace.c:3882 fs/namespace.c:4208)
__x64_sys_mount (fs/namespace.c:4222 fs/namespace.c:4432 fs/namespace.c:4409 fs/namespace.c:4409)
do_syscall_64 (arch/x86/entry/syscall_64.c:63 (discriminator 1) arch/x86/entry/syscall_64.c:94 (discriminator 1))
entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:130)
RIP: 0033:0x7f9b0397ffae
[ 0.869107] Code: 48 8b 0d 4d 5e 0d 00 f7 d8 64 89 01 48 83 c8 ff c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa 49 89 ca b8 a5 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 1a 5e 0d 00 f7 d8 64 89 01 48
All code
========
0: 48 8b 0d 4d 5e 0d 00 mov 0xd5e4d(%rip),%rcx # 0xd5e54
7: f7 d8 neg %eax
9: 64 89 01 mov %eax,%fs:(%rcx)
c: 48 83 c8 ff or $0xffffffffffffffff,%rax
10: c3 ret
11: 66 2e 0f 1f 84 00 00 cs nopw 0x0(%rax,%rax,1)
18: 00 00 00
1b: 90 nop
1c: f3 0f 1e fa endbr64
20: 49 89 ca mov %rcx,%r10
23: b8 a5 00 00 00 mov $0xa5,%eax
28: 0f 05 syscall
2a:* 48 3d 01 f0 ff ff cmp $0xfffffffffffff001,%rax <-- trapping instruction
30: 73 01 jae 0x33
32: c3 ret
33: 48 8b 0d 1a 5e 0d 00 mov 0xd5e1a(%rip),%rcx # 0xd5e54
3a: f7 d8 neg %eax
3c: 64 89 01 mov %eax,%fs:(%rcx)
3f: 48 rex.W

Code starting with the faulting instruction
===========================================
0: 48 3d 01 f0 ff ff cmp $0xfffffffffffff001,%rax
6: 73 01 jae 0x9
8: c3 ret
9: 48 8b 0d 1a 5e 0d 00 mov 0xd5e1a(%rip),%rcx # 0xd5e2a
10: f7 d8 neg %eax
12: 64 89 01 mov %eax,%fs:(%rcx)
15: 48 rex.W
RSP: 002b:00007ffdca32ba68 EFLAGS: 00000202 ORIG_RAX: 00000000000000a5
RAX: ffffffffffffffda RBX: 0000000000008000 RCX: 00007f9b0397ffae
RDX: 00005595a592a2e0 RSI: 00007ffdca32cf7b RDI: 00007ffdca32cf6e
RBP: 00007ffdca32cf6e R08: 00005595a59292e0 R09: 0000000000000002
R10: 0000000000008000 R11: 0000000000000202 R12: 00007ffdca32cf7b
R13: 00005595a592a2e0 R14: 0000000000008000 R15: 00005595a59292e0
</TASK>
Modules linked in:
CR2: 0000000000000068
---[ end trace 0000000000000000 ]---
RIP: 0010:btrfs_get_tree (fs/btrfs/super.c:2065 fs/btrfs/super.c:2106)
Code: 24 e8 40 3d 96 00 8b 1c 24 e9 e9 fd ff ff f6 83 a8 00 00 00 01 75 0e 48 8b 43 60 48 8b 40 68 f6 40 50 01 75 48 49 8b 44 24 60 <48> 8b 78 68 48 83 c7 70 e8 2f c2 b3 ff 4c 89 e7 e8 d7 87 e5 ff 49
All code
========
0: 24 e8 and $0xe8,%al
2: 40 3d 96 00 8b 1c rex cmp $0x1c8b0096,%eax
8: 24 e9 and $0xe9,%al
a: e9 fd ff ff f6 jmp 0xfffffffff700000c
f: 83 a8 00 00 00 01 75 subl $0x75,0x1000000(%rax)
16: 0e (bad)
17: 48 8b 43 60 mov 0x60(%rbx),%rax
1b: 48 8b 40 68 mov 0x68(%rax),%rax
1f: f6 40 50 01 testb $0x1,0x50(%rax)
23: 75 48 jne 0x6d
25: 49 8b 44 24 60 mov 0x60(%r12),%rax
2a:* 48 8b 78 68 mov 0x68(%rax),%rdi <-- trapping instruction
2e: 48 83 c7 70 add $0x70,%rdi
32: e8 2f c2 b3 ff call 0xffffffffffb3c266
37: 4c 89 e7 mov %r12,%rdi
3a: e8 d7 87 e5 ff call 0xffffffffffe58816
3f: 49 rex.WB

Code starting with the faulting instruction
===========================================
0: 48 8b 78 68 mov 0x68(%rax),%rdi
4: 48 83 c7 70 add $0x70,%rdi
8: e8 2f c2 b3 ff call 0xffffffffffb3c23c
d: 4c 89 e7 mov %r12,%rdi
10: e8 d7 87 e5 ff call 0xffffffffffe587ec
15: 49 rex.WB
RSP: 0018:ffffc15200583df8 EFLAGS: 00010246
RAX: 0000000000000000 RBX: ffff9f49450f7e40 RCX: 0000000000000000
RDX: 7fffffffffffffff RSI: 0000000000000000 RDI: ffffffffaf773b50
RBP: 0000000000000000 R08: ffff9f4941c5dca8 R09: ffff9f4942054d00
R10: ffffffffad2c3ce2 R11: ffff9f494482e000 R12: ffff9f4945224a80
R13: 0000000000000000 R14: ffff9f4944957f40 R15: 00000000ffffffff
FS: 00007f9b03861b80(0000) GS:ffff9f49ccd7b000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000000068 CR3: 0000000005385000 CR4: 0000000000350ef0
note: mount[154] exited with irqs disabled

> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---
> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
> index 7121d8c7a318..a3634e7f2304 100644
> --- a/fs/btrfs/super.c
> +++ b/fs/btrfs/super.c
> @@ -1984,17 +1984,13 @@ static int btrfs_get_tree_super(struct fs_context *fc)
> * btrfs or not, setting the whole super block RO. To make per-subvolume mounting
> * work with different options work we need to keep backward compatibility.
> */
> -static int btrfs_reconfigure_for_mount(struct fs_context *fc, struct vfsmount *mnt)
> +static int btrfs_reconfigure_for_mount(struct fs_context *fc)
> {
> int ret = 0;
>
> - if (fc->sb_flags & SB_RDONLY)
> - return ret;
> -
> - down_write(&mnt->mnt_sb->s_umount);
> - if (!(fc->sb_flags & SB_RDONLY) && (mnt->mnt_sb->s_flags & SB_RDONLY))
> + if (!(fc->sb_flags & SB_RDONLY) && (fc->root->d_sb->s_flags & SB_RDONLY))
> ret = btrfs_reconfigure(fc);
> - up_write(&mnt->mnt_sb->s_umount);
> +
> return ret;
> }
>
> @@ -2047,17 +2043,18 @@ static int btrfs_get_tree_subvol(struct fs_context *fc)
> security_free_mnt_opts(&fc->security);
> fc->security = NULL;
>
> - mnt = fc_mount(dup_fc);
> - if (IS_ERR(mnt)) {
> - put_fs_context(dup_fc);
> - return PTR_ERR(mnt);
> + ret = vfs_get_tree(dup_fc);
> + if (!ret) {
> + ret = btrfs_reconfigure_for_mount(dup_fc);
> + up_write(&fc->root->d_sb->s_umount);
> }
> - ret = btrfs_reconfigure_for_mount(dup_fc, mnt);
> + if (!ret)
> + mnt = vfs_create_mount(fc);
> + else
> + mnt = ERR_PTR(ret);
> put_fs_context(dup_fc);
> - if (ret) {
> - mntput(mnt);
> - return ret;
> - }
> + if (IS_ERR(mnt))
> + return PTR_ERR(mnt);
>
> /*
> * This free's ->subvol_name, because if it isn't set we have to

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 45130 bytes --]