[syzbot] [jfs?] general protection fault in dtInsertEntry

[syzbot]

Hello,

syzbot found the following issue on:

HEAD commit: fe46a7dd189e Merge tag 'sound-6.9-rc1' of git://git.kernel..
git tree: upstream
console+strace: https://syzkaller.appspot.com/x/log.txt?x=10056223180000
kernel config: https://syzkaller.appspot.com/x/.config?x=4d90a36f0cab495a
dashboard link: https://syzkaller.appspot.com/bug?extid=bba84aef3a26fb93deb9
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=10548115180000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=136ecbb5180000

Downloadable assets:
disk image: https://storage.googleapis.com/syzbot-assets/72ab73815344/disk-fe46a7dd.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/2d6d6b0d7071/vmlinux-fe46a7dd.xz
kernel image: https://storage.googleapis.com/syzbot-assets/48e275e5478b/bzImage-fe46a7dd.xz
mounted in repro: https://storage.googleapis.com/syzbot-assets/06e004bee618/mount_0.gz

Bisection is inconclusive: the issue happens on the oldest tested release.

bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=1401ee15180000
final oops: https://syzkaller.appspot.com/x/report.txt?x=1601ee15180000
console output: https://syzkaller.appspot.com/x/log.txt?x=1201ee15180000

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

loop0: detected capacity change from 0 to 32768
general protection fault, probably for non-canonical address 0xdffffc0000000001: 0000 [#1] PREEMPT SMP KASAN PTI
KASAN: null-ptr-deref in range [0x0000000000000008-0x000000000000000f]
CPU: 0 PID: 5061 Comm: syz-executor404 Not tainted 6.8.0-syzkaller-08951-gfe46a7dd189e #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 03/27/2024
RIP: 0010:dtInsertEntry+0xd0c/0x1780 fs/jfs/jfs_dtree.c:3713
Code: 83 e6 02 31 ff e8 a4 3f 75 fe 83 e3 02 75 3a e8 9a 3c 75 fe 48 8b 9c 24 a8 00 00 00 48 83 c3 08 48 89 d8 48 c1 e8 03 4c 89 f2 <42> 0f b6 04 30 84 c0 74 3e 89 d9 80 e1 07 38 c1 7c 35 48 89 df e8
RSP: 0018:ffffc9000381f060 EFLAGS: 00010202
RAX: 0000000000000001 RBX: 0000000000000008 RCX: ffff88801a715a00
RDX: dffffc0000000000 RSI: 0000000000000000 RDI: 0000000000000000
RBP: ffffc9000381f210 R08: ffffffff831fb7ac R09: ffffffff832296af
R10: 0000000000000004 R11: ffff88801a715a00 R12: ffff88807adfb130
R13: ffffffffffffffff R14: dffffc0000000000 R15: 000000000000000d
FS: 000055558fe16380(0000) GS:ffff8880b9400000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 000000000066c7e0 CR3: 000000002ca48000 CR4: 00000000003506f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
<TASK>
dtInsert+0xbf1/0x6b00 fs/jfs/jfs_dtree.c:891
jfs_create+0x7ba/0xb90 fs/jfs/namei.c:137
lookup_open fs/namei.c:3497 [inline]
open_last_lookups fs/namei.c:3566 [inline]
path_openat+0x1425/0x3240 fs/namei.c:3796
do_filp_open+0x235/0x490 fs/namei.c:3826
do_sys_openat2+0x13e/0x1d0 fs/open.c:1406
do_sys_open fs/open.c:1421 [inline]
__do_sys_openat fs/open.c:1437 [inline]
__se_sys_openat fs/open.c:1432 [inline]
__x64_sys_openat+0x247/0x2a0 fs/open.c:1432
do_syscall_64+0xfb/0x240
entry_SYSCALL_64_after_hwframe+0x6d/0x75
RIP: 0033:0x7f30b7a475f9
Code: 28 00 00 00 75 05 48 83 c4 28 c3 e8 61 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:00007fffd9c3bfb8 EFLAGS: 00000246 ORIG_RAX: 0000000000000101
RAX: ffffffffffffffda RBX: 00007fffd9c3c198 RCX: 00007f30b7a475f9
RDX: 000000000000275a RSI: 0000000020000080 RDI: 00000000ffffff9c
RBP: 00007f30b7ac0610 R08: 0000000000005e33 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000001
R13: 00007fffd9c3c188 R14: 0000000000000001 R15: 0000000000000001
</TASK>
Modules linked in:
---[ end trace 0000000000000000 ]---
RIP: 0010:dtInsertEntry+0xd0c/0x1780 fs/jfs/jfs_dtree.c:3713
Code: 83 e6 02 31 ff e8 a4 3f 75 fe 83 e3 02 75 3a e8 9a 3c 75 fe 48 8b 9c 24 a8 00 00 00 48 83 c3 08 48 89 d8 48 c1 e8 03 4c 89 f2 <42> 0f b6 04 30 84 c0 74 3e 89 d9 80 e1 07 38 c1 7c 35 48 89 df e8
RSP: 0018:ffffc9000381f060 EFLAGS: 00010202
RAX: 0000000000000001 RBX: 0000000000000008 RCX: ffff88801a715a00
RDX: dffffc0000000000 RSI: 0000000000000000 RDI: 0000000000000000
RBP: ffffc9000381f210 R08: ffffffff831fb7ac R09: ffffffff832296af
R10: 0000000000000004 R11: ffff88801a715a00 R12: ffff88807adfb130
R13: ffffffffffffffff R14: dffffc0000000000 R15: 000000000000000d
FS: 000055558fe16380(0000) GS:ffff8880b9500000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000557bf42357e0 CR3: 000000002ca48000 CR4: 00000000003506f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
----------------
Code disassembly (best guess):
0: 83 e6 02 and $0x2,%esi
3: 31 ff xor %edi,%edi
5: e8 a4 3f 75 fe call 0xfe753fae
a: 83 e3 02 and $0x2,%ebx
d: 75 3a jne 0x49
f: e8 9a 3c 75 fe call 0xfe753cae
14: 48 8b 9c 24 a8 00 00 mov 0xa8(%rsp),%rbx
1b: 00
1c: 48 83 c3 08 add $0x8,%rbx
20: 48 89 d8 mov %rbx,%rax
23: 48 c1 e8 03 shr $0x3,%rax
27: 4c 89 f2 mov %r14,%rdx
* 2a: 42 0f b6 04 30 movzbl (%rax,%r14,1),%eax <-- trapping instruction
2f: 84 c0 test %al,%al
31: 74 3e je 0x71
33: 89 d9 mov %ebx,%ecx
35: 80 e1 07 and $0x7,%cl
38: 38 c1 cmp %al,%cl
3a: 7c 35 jl 0x71
3c: 48 89 df mov %rbx,%rdi
3f: e8 .byte 0xe8


---
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.
For information about bisection process see: https://goo.gl/tpsmEJ#bisection

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

[Edward Adam Davis]

[syzbot reported]

general protection fault, probably for non-canonical address 0xdffffc0000000001: 0000 [#1] PREEMPT SMP KASAN PTI
KASAN: null-ptr-deref in range [0x0000000000000008-0x000000000000000f]
CPU: 0 PID: 5061 Comm: syz-executor404 Not tainted 6.8.0-syzkaller-08951-gfe46a7dd189e #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 03/27/2024
RIP: 0010:dtInsertEntry+0xd0c/0x1780 fs/jfs/jfs_dtree.c:3713

...
[Analyze]
When the pointer h has the same value as p, after writing name in UniStrncpy_to_le(),
p->header.flag will be cleared.
This will cause the previously true judgment "p->header.flag & BT-LEAF" to change
to no after writing the name operation, this leads to entering an incorrect branch
and accessing the uninitialized object ih when judging this condition for the
second time.
[Fix]
When allocating slots from the freelist, we start from the second one to preserve
the header of p from being incorrectly modified.

Reported-by: syzbot+bba84a...@syzkaller.appspotmail.com
Signed-off-by: Edward Adam Davis <ead...@qq.com>
---
fs/jfs/jfs_dtree.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/jfs/jfs_dtree.c b/fs/jfs/jfs_dtree.c
index 031d8f570f58..deb2a5cc78d8 100644
--- a/fs/jfs/jfs_dtree.c
+++ b/fs/jfs/jfs_dtree.c
@@ -3618,7 +3618,8 @@ static void dtInsertEntry(dtpage_t * p, int index, struct component_name * key,
kname = key->name;

/* allocate a free slot */
- hsi = fsi = p->header.freelist;
+ hsi = fsi = p->header.freelist = p->header.freelist == 0 ?
+ 1 : p->header.freelist;
h = &p->slot[fsi];
p->header.freelist = h->next;
--p->header.freecnt;
--
2.43.0

[Dave Kleikamp]

On 4/10/24 2:05AM, Edward Adam Davis via Jfs-discussion wrote:
> [syzbot reported]
> general protection fault, probably for non-canonical address 0xdffffc0000000001: 0000 [#1] PREEMPT SMP KASAN PTI
> KASAN: null-ptr-deref in range [0x0000000000000008-0x000000000000000f]
> CPU: 0 PID: 5061 Comm: syz-executor404 Not tainted 6.8.0-syzkaller-08951-gfe46a7dd189e #0
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 03/27/2024
> RIP: 0010:dtInsertEntry+0xd0c/0x1780 fs/jfs/jfs_dtree.c:3713
> ...
> [Analyze]
> When the pointer h has the same value as p, after writing name in UniStrncpy_to_le(),
> p->header.flag will be cleared.
> This will cause the previously true judgment "p->header.flag & BT-LEAF" to change
> to no after writing the name operation, this leads to entering an incorrect branch
> and accessing the uninitialized object ih when judging this condition for the
> second time.
> [Fix]
> When allocating slots from the freelist, we start from the second one to preserve
> the header of p from being incorrectly modified.

The freelist is simply corrupted. It should never be set to 0. We cannot
assume that slot[1] is on the free list. Probably the best we can do is
to add more sanity checking to the freelist value, and/or any slot's
next & prev value. That could potentially involve a lot more checks.
I've been accepting patches here and there for specific syzbot failures,
but any comprehensive sanity checking of every data structure would be a
huge effort.

What makes a fix a little more difficult is that dtInsertEntry returns
void and it would be difficult to gracefully recover. One could change
it to return an error and have all the callers check that. But I'm
afraid, just using a valid slot number would only lead to further data
corruption.

Thanks,
Shaggy

[Edward Adam Davis]

[syzbot reported]
general protection fault, probably for non-canonical address 0xdffffc0000000001: 0000 [#1] PREEMPT SMP KASAN PTI
KASAN: null-ptr-deref in range [0x0000000000000008-0x000000000000000f]
CPU: 0 PID: 5061 Comm: syz-executor404 Not tainted 6.8.0-syzkaller-08951-gfe46a7dd189e #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 03/27/2024
RIP: 0010:dtInsertEntry+0xd0c/0x1780 fs/jfs/jfs_dtree.c:3713
...
[Analyze]

In dtInsertEntry(), when the pointer h has the same value as p, after writing

name in UniStrncpy_to_le(), p->header.flag will be cleared. This will cause the
previously true judgment "p->header.flag & BT-LEAF" to change to no after writing
the name operation, this leads to entering an incorrect branch and accessing the
uninitialized object ih when judging this condition for the second time.

[Fix]

After got the page, check freelist first, if freelist == 0 then exit dtInsert()
and return -EINVAL.

Reported-by: syzbot+bba84a...@syzkaller.appspotmail.com
Signed-off-by: Edward Adam Davis <ead...@qq.com>
---

fs/jfs/jfs_dtree.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/fs/jfs/jfs_dtree.c b/fs/jfs/jfs_dtree.c
index 031d8f570f58..5d3127ca68a4 100644
--- a/fs/jfs/jfs_dtree.c
+++ b/fs/jfs/jfs_dtree.c
@@ -834,6 +834,8 @@ int dtInsert(tid_t tid, struct inode *ip,
* the full page.
*/
DT_GETSEARCH(ip, btstack->top, bn, mp, p, index);
+ if (p->header.freelist == 0)
+ return -EINVAL;

/*
* insert entry for new key
--
2.43.0