[syzbot] [btrfs?] general protection fault in btrfs_search_slot

[syzbot]

Hello,

syzbot found the following issue on:

HEAD commit: 715ca9dd687f Merge tag 'io_uring-6.12-20241019' of git://g..
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=16c620a7980000
kernel config: https://syzkaller.appspot.com/x/.config?x=16e543edc81a3008
dashboard link: https://syzkaller.appspot.com/bug?extid=3030e17bd57a73d39bd7
compiler: Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=12c8825f980000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=1417c430580000

Downloadable assets:
disk image (non-bootable): https://storage.googleapis.com/syzbot-assets/7feb34a89c2a/non_bootable_disk-715ca9dd.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/ba436e2363b6/vmlinux-715ca9dd.xz
kernel image: https://storage.googleapis.com/syzbot-assets/3ac78a7a1a30/bzImage-715ca9dd.xz
mounted in repro: https://storage.googleapis.com/syzbot-assets/2ff71deb6e6e/mount_0.gz

IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+3030e1...@syzkaller.appspotmail.com

Oops: general protection fault, probably for non-canonical address 0xdffffc0000000041: 0000 [#1] PREEMPT SMP KASAN NOPTI
KASAN: null-ptr-deref in range [0x0000000000000208-0x000000000000020f]
CPU: 0 UID: 0 PID: 5098 Comm: syz-executor113 Not tainted 6.12.0-rc3-syzkaller-00420-g715ca9dd687f #0
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-debian-1.16.3-2~bpo12+1 04/01/2014
RIP: 0010:btrfs_search_slot+0xc5/0x30d0 fs/btrfs/ctree.c:2013
Code: 08 43 c7 44 2c 10 04 f2 04 f3 e8 f6 ed e6 fd 48 89 5c 24 38 48 81 c3 08 02 00 00 48 89 d8 48 c1 e8 03 48 89 84 24 d0 00 00 00 <42> 80 3c 28 00 74 08 48 89 df e8 5c b2 50 fe 48 8b 03 48 89 84 24
RSP: 0018:ffffc90000e379e0 EFLAGS: 00010206
RAX: 0000000000000041 RBX: 0000000000000208 RCX: ffff888000ff4880
RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000
RBP: ffffc90000e37bb0 R08: 0000000000000000 R09: 0000000000000000
R10: dffffc0000000000 R11: ffffed100826400e R12: 1ffff920001c6f58
R13: dffffc0000000000 R14: 0000000000000000 R15: ffff88803e569b00
FS: 0000555591c73380(0000) GS:ffff88801fc00000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007fcd50a190f8 CR3: 000000003deb2000 CR4: 0000000000352ef0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
<TASK>
extent_from_logical+0x1c8/0x880 fs/btrfs/backref.c:2216
iterate_inodes_from_logical+0x13b/0x330 fs/btrfs/backref.c:2560
btrfs_ioctl_logical_to_ino+0x133/0x2a0 fs/btrfs/ioctl.c:3489
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/common.c:52 [inline]
do_syscall_64+0xf3/0x230 arch/x86/entry/common.c:83
entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7fcd5099cc59
Code: 28 00 00 00 75 05 48 83 c4 28 c3 e8 f1 17 00 00 90 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 b8 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007ffdda2f7428 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007fcd5099cc59
RDX: 0000000020000080 RSI: 00000000c0389424 RDI: 0000000000000004
RBP: 00007fcd50a155f0 R08: 0000555591c744c0 R09: 0000555591c744c0
R10: 00000000000055c5 R11: 0000000000000246 R12: 00007ffdda2f7450
R13: 00007ffdda2f7678 R14: 431bde82d7b634db R15: 00007fcd509e503b
</TASK>
Modules linked in:
---[ end trace 0000000000000000 ]---
RIP: 0010:btrfs_search_slot+0xc5/0x30d0 fs/btrfs/ctree.c:2013
Code: 08 43 c7 44 2c 10 04 f2 04 f3 e8 f6 ed e6 fd 48 89 5c 24 38 48 81 c3 08 02 00 00 48 89 d8 48 c1 e8 03 48 89 84 24 d0 00 00 00 <42> 80 3c 28 00 74 08 48 89 df e8 5c b2 50 fe 48 8b 03 48 89 84 24
RSP: 0018:ffffc90000e379e0 EFLAGS: 00010206
RAX: 0000000000000041 RBX: 0000000000000208 RCX: ffff888000ff4880
RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000
RBP: ffffc90000e37bb0 R08: 0000000000000000 R09: 0000000000000000
R10: dffffc0000000000 R11: ffffed100826400e R12: 1ffff920001c6f58
R13: dffffc0000000000 R14: 0000000000000000 R15: ffff88803e569b00
FS: 0000555591c73380(0000) GS:ffff88801fc00000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 000055d064263098 CR3: 000000003deb2000 CR4: 0000000000352ef0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
----------------
Code disassembly (best guess):
0: 08 43 c7 or %al,-0x39(%rbx)
3: 44 2c 10 rex.R sub $0x10,%al
6: 04 f2 add $0xf2,%al
8: 04 f3 add $0xf3,%al
a: e8 f6 ed e6 fd call 0xfde6ee05
f: 48 89 5c 24 38 mov %rbx,0x38(%rsp)
14: 48 81 c3 08 02 00 00 add $0x208,%rbx
1b: 48 89 d8 mov %rbx,%rax
1e: 48 c1 e8 03 shr $0x3,%rax
22: 48 89 84 24 d0 00 00 mov %rax,0xd0(%rsp)
29: 00
* 2a: 42 80 3c 28 00 cmpb $0x0,(%rax,%r13,1) <-- trapping instruction
2f: 74 08 je 0x39
31: 48 89 df mov %rbx,%rdi
34: e8 5c b2 50 fe call 0xfe50b295
39: 48 8b 03 mov (%rbx),%rax
3c: 48 rex.W
3d: 89 .byte 0x89
3e: 84 .byte 0x84
3f: 24 .byte 0x24


---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzk...@googlegroups.com.

syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.

If the report is already addressed, let syzbot know by replying with:
#syz fix: exact-commit-title

If you want syzbot to run the reproducer, reply with:
#syz test: git://repo/address.git branch-or-commit-hash
If you attach or paste a git patch, syzbot will apply it before testing.

If you want to overwrite report's subsystems, reply with:
#syz set subsystems: new-subsystem
(See the list of subsystem names on the web dashboard)

If the report is a duplicate of another one, reply with:
#syz dup: exact-subject-of-another-report

If you want to undo deduplication, reply with:
#syz undup

[Lizhi Xu]

use the input logical can't find the extent root, so add sanity check for
extent root before search slot.

#syz test

diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c
index f8e1d5b2c512..87eaf5dd2d5d 100644
--- a/fs/btrfs/backref.c
+++ b/fs/btrfs/backref.c
@@ -2213,6 +2213,9 @@ int extent_from_logical(struct btrfs_fs_info *fs_info, u64 logical,
key.objectid = logical;
key.offset = (u64)-1;

+ if (!extent_root)
+ return -ENOENT;
+
ret = btrfs_search_slot(NULL, extent_root, &key, path, 0, 0);
if (ret < 0)
return ret;

[syzbot]

Hello,

syzbot has tested the proposed patch and the reproducer did not trigger any issue:

Reported-by: syzbot+3030e1...@syzkaller.appspotmail.com
Tested-by: syzbot+3030e1...@syzkaller.appspotmail.com

Tested on:

commit: ae90f6a6 Merge tag 'bpf-fixes' of git://git.kernel.org..
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=11823287980000
kernel config: https://syzkaller.appspot.com/x/.config?x=fc6f8ce8c5369043

dashboard link: https://syzkaller.appspot.com/bug?extid=3030e17bd57a73d39bd7
compiler: Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40

patch: https://syzkaller.appspot.com/x/patch.diff?x=169c3287980000

Note: testing is done by a robot and is best-effort only.

[Qu Wenruo]

在 2024/10/25 12:53, Lizhi Xu 写道:
> use the input logical can't find the extent root, so add sanity check for
> extent root before search slot.
>
> #syz test
>
> diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c
> index f8e1d5b2c512..87eaf5dd2d5d 100644
> --- a/fs/btrfs/backref.c
> +++ b/fs/btrfs/backref.c
> @@ -2213,6 +2213,9 @@ int extent_from_logical(struct btrfs_fs_info *fs_info, u64 logical,
> key.objectid = logical;
> key.offset = (u64)-1;
>
> + if (!extent_root)
> + return -ENOENT;

Considering we have a lot of such btrfs_search_slot() without checking
if the csum/extent root is NULL, can we move the check into
btrfs_search_slot()?

Thanks,
Qu

[Lizhi Xu]

On Fri, 25 Oct 2024 14:49:48 +1030, Qu Wenruo wrote:
> > use the input logical can't find the extent root, so add sanity check for
> > extent root before search slot.
> >
> > #syz test
> >
> > diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c
> > index f8e1d5b2c512..87eaf5dd2d5d 100644
> > --- a/fs/btrfs/backref.c
> > +++ b/fs/btrfs/backref.c
> > @@ -2213,6 +2213,9 @@ int extent_from_logical(struct btrfs_fs_info *fs_info, u64 logical,
> > key.objectid = logical;
> > key.offset = (u64)-1;
> >
> > + if (!extent_root)
> > + return -ENOENT;
>
> Considering we have a lot of such btrfs_search_slot() without checking
> if the csum/extent root is NULL, can we move the check into
> btrfs_search_slot()?

Yes, judging in btrfs_search_slot can fix the current issue while also
avoiding similar problems.

#syz test

diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
index 0cc919d15b14..9c05cab473f5 100644
--- a/fs/btrfs/ctree.c
+++ b/fs/btrfs/ctree.c
@@ -2010,7 +2010,7 @@ int btrfs_search_slot(struct btrfs_trans_handle *trans, struct btrfs_root *root,
const struct btrfs_key *key, struct btrfs_path *p,
int ins_len, int cow)
{
- struct btrfs_fs_info *fs_info = root->fs_info;
+ struct btrfs_fs_info *fs_info;
struct extent_buffer *b;
int slot;
int ret;
@@ -2023,6 +2023,10 @@ int btrfs_search_slot(struct btrfs_trans_handle *trans, struct btrfs_root *root,
int min_write_lock_level;
int prev_cmp;

+ if (!root)
+ return -EINVAL;
+
+ fs_info = root->fs_info;
might_sleep();

lowest_level = p->lowest_level;

[Qu Wenruo]

在 2024/10/25 15:08, Lizhi Xu 写道:
> On Fri, 25 Oct 2024 14:49:48 +1030, Qu Wenruo wrote:
>>> use the input logical can't find the extent root, so add sanity check for
>>> extent root before search slot.
>>>
>>> #syz test
>>>
>>> diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c
>>> index f8e1d5b2c512..87eaf5dd2d5d 100644
>>> --- a/fs/btrfs/backref.c
>>> +++ b/fs/btrfs/backref.c
>>> @@ -2213,6 +2213,9 @@ int extent_from_logical(struct btrfs_fs_info *fs_info, u64 logical,
>>> key.objectid = logical;
>>> key.offset = (u64)-1;
>>>
>>> + if (!extent_root)
>>> + return -ENOENT;
>>
>> Considering we have a lot of such btrfs_search_slot() without checking
>> if the csum/extent root is NULL, can we move the check into
>> btrfs_search_slot()?
> Yes, judging in btrfs_search_slot can fix the current issue while also
> avoiding similar problems.
>

Can't wait for your formal fix on this.

Thanks,
Qu

[Lizhi Xu]

Syzbot report a null-ptr-deref in btrfs_search_slot.
It use the input logical can't find the extent root in extent_from_logical,
and triger the null-ptr-deref in btrfs_search_slot.
Add sanity check for btrfs root before using it in btrfs_search_slot.

Reported-by: syzbot+3030e1...@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=3030e17bd57a73d39bd7
Signed-off-by: Lizhi Xu <lizh...@windriver.com>
---
fs/btrfs/ctree.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
index 0cc919d15b14..9c05cab473f5 100644
--- a/fs/btrfs/ctree.c
+++ b/fs/btrfs/ctree.c
@@ -2010,7 +2010,7 @@ int btrfs_search_slot(struct btrfs_trans_handle *trans, struct btrfs_root *root,
const struct btrfs_key *key, struct btrfs_path *p,
int ins_len, int cow)
{
- struct btrfs_fs_info *fs_info = root->fs_info;
+ struct btrfs_fs_info *fs_info;
struct extent_buffer *b;
int slot;
int ret;
@@ -2023,6 +2023,10 @@ int btrfs_search_slot(struct btrfs_trans_handle *trans, struct btrfs_root *root,
int min_write_lock_level;
int prev_cmp;

+ if (!root)
+ return -EINVAL;
+
+ fs_info = root->fs_info;
might_sleep();

lowest_level = p->lowest_level;

--
2.43.0

[syzbot]

Hello,

syzbot has tested the proposed patch and the reproducer did not trigger any issue:

Reported-by: syzbot+3030e1...@syzkaller.appspotmail.com
Tested-by: syzbot+3030e1...@syzkaller.appspotmail.com

Tested on:

commit: ae90f6a6 Merge tag 'bpf-fixes' of git://git.kernel.org..
git tree: upstream

console output: https://syzkaller.appspot.com/x/log.txt?x=129e3287980000

kernel config: https://syzkaller.appspot.com/x/.config?x=fc6f8ce8c5369043
dashboard link: https://syzkaller.appspot.com/bug?extid=3030e17bd57a73d39bd7
compiler: Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40

patch: https://syzkaller.appspot.com/x/patch.diff?x=17ca0230580000

[Qu Wenruo]

在 2024/10/25 15:25, Lizhi Xu 写道:
> Syzbot report a null-ptr-deref in btrfs_search_slot.
> It use the input logical can't find the extent root in extent_from_logical,
> and triger the null-ptr-deref in btrfs_search_slot.
> Add sanity check for btrfs root before using it in btrfs_search_slot.

Although I'd prefer to explain a little more about why the NULL root
pointer can happen (caused by the rescue=all mount option), which can
cause NULL root pointer for non-critical tree roots, like
uuid/csum/extent or even device trees.

You don't need to bother sending an update.
I can add such info when pushing to the maintainer's tree.

>
> Reported-by: syzbot+3030e1...@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=3030e17bd57a73d39bd7
> Signed-off-by: Lizhi Xu <lizh...@windriver.com>

Reviewed-by: Qu Wenruo <w...@suse.com>

Thanks,
Qu

[David Sterba]

On Fri, Oct 25, 2024 at 08:23:07PM +1030, Qu Wenruo wrote:
>
>
> 在 2024/10/25 15:25, Lizhi Xu 写道:
> > Syzbot report a null-ptr-deref in btrfs_search_slot.
> > It use the input logical can't find the extent root in extent_from_logical,
> > and triger the null-ptr-deref in btrfs_search_slot.
> > Add sanity check for btrfs root before using it in btrfs_search_slot.
>
> Although I'd prefer to explain a little more about why the NULL root
> pointer can happen (caused by the rescue=all mount option), which can
> cause NULL root pointer for non-critical tree roots, like
> uuid/csum/extent or even device trees.
>
> You don't need to bother sending an update.
> I can add such info when pushing to the maintainer's tree.
>
> >
> > Reported-by: syzbot+3030e1...@syzkaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=3030e17bd57a73d39bd7
> > Signed-off-by: Lizhi Xu <lizh...@windriver.com>
>
> Reviewed-by: Qu Wenruo <w...@suse.com>

> > @@ -2023,6 +2023,10 @@ int btrfs_search_slot(struct btrfs_trans_handle *trans, struct btrfs_root *root,
> > int min_write_lock_level;
> > int prev_cmp;
> >
> > + if (!root)
> > + return -EINVAL;

The function returns errors indirectly so it's not clear which could be
ultimately returned. I did a quick search over the calls starting in
btrfs_search_slot() and it seems that EINVAL is not used so we'd know if
it ends up in some error report. The ones I found: EAGAIN, EIO, EUCLEAN,
ENOMEM.

[Qu Wenruo]

If you want, I can add extra (ratelimited though) error/warning message
for such cases.

Considering this is only possible for rescue=all cases, extra error
messages should be fine.

Or do you prefer some more rare return values?

Thanks,
Qu

[David Sterba]

I haven't thought about printing a message, even with rate limiting it
could be noisy and I don't see what information would it bring to the
user, namely in combination with the rescue=all.

The error from search slot can bubble up from various functions, though
with the missing csum tree it's probably not that many. The only other
error code that's remotely relevant is ENOENT (no such entry), but this
is more suitable for entries in structures, not whole structures.

Even if we'd use some random error, the string for the error code would
be misleading. So let's keep EINVAL for now.

[syzbot]

syzbot has bisected this issue to:

commit 42437a6386ffeaaf200731e73d723ea491f3fe7d
Author: Josef Bacik <jo...@toxicpanda.com>
Date: Fri Oct 16 15:29:18 2020 +0000

btrfs: introduce mount option rescue=ignorebadroots

bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=178b4d5f980000
start commit: 3e5e6c9900c3 Merge tag 'nfsd-6.12-3' of git://git.kernel.o..
git tree: upstream
final oops: https://syzkaller.appspot.com/x/report.txt?x=144b4d5f980000
console output: https://syzkaller.appspot.com/x/log.txt?x=104b4d5f980000
kernel config: https://syzkaller.appspot.com/x/.config?x=f527353e21e067e8
dashboard link: https://syzkaller.appspot.com/bug?extid=3030e17bd57a73d39bd7
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=14bc8d5f980000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=16ba6b40580000

Reported-by: syzbot+3030e1...@syzkaller.appspotmail.com
Fixes: 42437a6386ff ("btrfs: introduce mount option rescue=ignorebadroots")

For information about bisection process see: https://goo.gl/tpsmEJ#bisection