* [syzbot] [ntfs3?] general protection fault in pick_link @ 2024-11-14 18:08 syzbot 2024-11-15 9:49 ` [PATCH] fs: add check for symlink corrupted Lizhi Xu 0 siblings, 1 reply; 22+ messages in thread From: syzbot @ 2024-11-14 18:08 UTC (permalink / raw) To: almaz.alexandrovich, brauner, jack, linux-fsdevel, linux-kernel, ntfs3, syzkaller-bugs, viro Hello, syzbot found the following issue on: HEAD commit: a9cda7c0ffed Merge tag 'irq_urgent_for_v6.12_rc7' of git:/.. git tree: upstream console output: https://syzkaller.appspot.com/x/log.txt?x=11e55ea7980000 kernel config: https://syzkaller.appspot.com/x/.config?x=64aa0d9945bd5c1 dashboard link: https://syzkaller.appspot.com/bug?extid=73d8fc29ec7cba8286fa 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=133fe35f980000 C reproducer: https://syzkaller.appspot.com/x/repro.c?x=13e93e30580000 Downloadable assets: disk image (non-bootable): https://storage.googleapis.com/syzbot-assets/7feb34a89c2a/non_bootable_disk-a9cda7c0.raw.xz vmlinux: https://storage.googleapis.com/syzbot-assets/29b6746965e0/vmlinux-a9cda7c0.xz kernel image: https://storage.googleapis.com/syzbot-assets/455959afbe37/bzImage-a9cda7c0.xz mounted in repro: https://storage.googleapis.com/syzbot-assets/d478326f6d12/mount_0.gz IMPORTANT: if you fix the issue, please add the following tag to the commit: Reported-by: syzbot+73d8fc29ec7cba8286fa@syzkaller.appspotmail.com loop0: detected capacity change from 0 to 4096 ntfs3(loop0): Different NTFS sector size (4096) and media sector size (512). ntfs3(loop0): ino=1b, "file0" attr_set_size ntfs3(loop0): Mark volume as dirty due to NTFS errors Oops: general protection fault, probably for non-canonical address 0xdffffc0000000000: 0000 [#1] PREEMPT SMP KASAN NOPTI KASAN: null-ptr-deref in range [0x0000000000000000-0x0000000000000007] CPU: 0 UID: 0 PID: 5310 Comm: syz-executor255 Not tainted 6.12.0-rc6-syzkaller-00318-ga9cda7c0ffed #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:pick_link+0x51c/0xd50 fs/namei.c:1864 Code: c1 e8 03 42 80 3c 38 00 74 08 48 89 df e8 fc 00 e9 ff 48 8b 2b 48 85 ed 0f 84 92 00 00 00 e8 7b 36 7f ff 48 89 e8 48 c1 e8 03 <42> 0f b6 04 38 84 c0 0f 85 a2 05 00 00 0f b6 5d 00 bf 2f 00 00 00 RSP: 0018:ffffc9000d147998 EFLAGS: 00010246 RAX: 0000000000000000 RBX: ffff88804558dec8 RCX: ffff88801ec7a440 RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000 RBP: 0000000000000002 R08: ffffffff8215a35f R09: 1ffffffff203a13d R10: dffffc0000000000 R11: fffffbfff203a13e R12: 1ffff92001a28f93 R13: ffffc9000d147af8 R14: 1ffff92001a28f5f R15: dffffc0000000000 FS: 0000555577611380(0000) GS:ffff88801fc00000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 00007fcc0a595ed8 CR3: 0000000035760000 CR4: 0000000000352ef0 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 Call Trace: <TASK> step_into+0xca9/0x1080 fs/namei.c:1923 lookup_last fs/namei.c:2556 [inline] path_lookupat+0x16f/0x450 fs/namei.c:2580 filename_lookup+0x256/0x610 fs/namei.c:2609 user_path_at+0x3a/0x60 fs/namei.c:3016 do_mount fs/namespace.c:3844 [inline] __do_sys_mount fs/namespace.c:4057 [inline] __se_sys_mount+0x297/0x3c0 fs/namespace.c:4034 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:0x7f4b18ad5b19 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:00007ffc2e486c48 EFLAGS: 00000246 ORIG_RAX: 00000000000000a5 RAX: ffffffffffffffda RBX: 0030656c69662f2e RCX: 00007f4b18ad5b19 RDX: 0000000000000000 RSI: 00000000200000c0 RDI: 0000000000000000 RBP: 00007f4b18b685f0 R08: 0000000000000000 R09: 00005555776124c0 R10: 0000000000000000 R11: 0000000000000246 R12: 00007ffc2e486c70 R13: 00007ffc2e486e98 R14: 431bde82d7b634db R15: 00007f4b18b1e03b </TASK> Modules linked in: ---[ end trace 0000000000000000 ]--- RIP: 0010:pick_link+0x51c/0xd50 fs/namei.c:1864 Code: c1 e8 03 42 80 3c 38 00 74 08 48 89 df e8 fc 00 e9 ff 48 8b 2b 48 85 ed 0f 84 92 00 00 00 e8 7b 36 7f ff 48 89 e8 48 c1 e8 03 <42> 0f b6 04 38 84 c0 0f 85 a2 05 00 00 0f b6 5d 00 bf 2f 00 00 00 RSP: 0018:ffffc9000d147998 EFLAGS: 00010246 RAX: 0000000000000000 RBX: ffff88804558dec8 RCX: ffff88801ec7a440 RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000 RBP: 0000000000000002 R08: ffffffff8215a35f R09: 1ffffffff203a13d R10: dffffc0000000000 R11: fffffbfff203a13e R12: 1ffff92001a28f93 R13: ffffc9000d147af8 R14: 1ffff92001a28f5f R15: dffffc0000000000 FS: 0000555577611380(0000) GS:ffff88801fc00000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 00007fcc0a595ed8 CR3: 0000000035760000 CR4: 0000000000352ef0 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 ---------------- Code disassembly (best guess): 0: c1 e8 03 shr $0x3,%eax 3: 42 80 3c 38 00 cmpb $0x0,(%rax,%r15,1) 8: 74 08 je 0x12 a: 48 89 df mov %rbx,%rdi d: e8 fc 00 e9 ff call 0xffe9010e 12: 48 8b 2b mov (%rbx),%rbp 15: 48 85 ed test %rbp,%rbp 18: 0f 84 92 00 00 00 je 0xb0 1e: e8 7b 36 7f ff call 0xff7f369e 23: 48 89 e8 mov %rbp,%rax 26: 48 c1 e8 03 shr $0x3,%rax * 2a: 42 0f b6 04 38 movzbl (%rax,%r15,1),%eax <-- trapping instruction 2f: 84 c0 test %al,%al 31: 0f 85 a2 05 00 00 jne 0x5d9 37: 0f b6 5d 00 movzbl 0x0(%rbp),%ebx 3b: bf 2f 00 00 00 mov $0x2f,%edi --- 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 syzkaller@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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH] fs: add check for symlink corrupted 2024-11-14 18:08 [syzbot] [ntfs3?] general protection fault in pick_link syzbot @ 2024-11-15 9:49 ` Lizhi Xu 2024-11-15 11:43 ` Jan Kara 2024-11-15 13:06 ` Al Viro 0 siblings, 2 replies; 22+ messages in thread From: Lizhi Xu @ 2024-11-15 9:49 UTC (permalink / raw) To: syzbot+73d8fc29ec7cba8286fa Cc: almaz.alexandrovich, brauner, jack, linux-fsdevel, linux-kernel, ntfs3, syzkaller-bugs, viro syzbot reported a null-ptr-deref in pick_link. [1] When symlink's inode is corrupted, the value of the i_link is 2 in this case, it will trigger null pointer deref when accessing *res in pick_link(). To avoid this issue, add a check for inode mode, return -EINVAL when it's not symlink. [1] KASAN: null-ptr-deref in range [0x0000000000000000-0x0000000000000007] CPU: 0 UID: 0 PID: 5310 Comm: syz-executor255 Not tainted 6.12.0-rc6-syzkaller-00318-ga9cda7c0ffed #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:pick_link+0x51c/0xd50 fs/namei.c:1864 Code: c1 e8 03 42 80 3c 38 00 74 08 48 89 df e8 fc 00 e9 ff 48 8b 2b 48 85 ed 0f 84 92 00 00 00 e8 7b 36 7f ff 48 89 e8 48 c1 e8 03 <42> 0f b6 04 38 84 c0 0f 85 a2 05 00 00 0f b6 5d 00 bf 2f 00 00 00 RSP: 0018:ffffc9000d147998 EFLAGS: 00010246 RAX: 0000000000000000 RBX: ffff88804558dec8 RCX: ffff88801ec7a440 RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000 RBP: 0000000000000002 R08: ffffffff8215a35f R09: 1ffffffff203a13d R10: dffffc0000000000 R11: fffffbfff203a13e R12: 1ffff92001a28f93 R13: ffffc9000d147af8 R14: 1ffff92001a28f5f R15: dffffc0000000000 FS: 0000555577611380(0000) GS:ffff88801fc00000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 00007fcc0a595ed8 CR3: 0000000035760000 CR4: 0000000000352ef0 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 Call Trace: <TASK> step_into+0xca9/0x1080 fs/namei.c:1923 lookup_last fs/namei.c:2556 [inline] path_lookupat+0x16f/0x450 fs/namei.c:2580 filename_lookup+0x256/0x610 fs/namei.c:2609 user_path_at+0x3a/0x60 fs/namei.c:3016 do_mount fs/namespace.c:3844 [inline] __do_sys_mount fs/namespace.c:4057 [inline] __se_sys_mount+0x297/0x3c0 fs/namespace.c:4034 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:0x7f4b18ad5b19 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:00007ffc2e486c48 EFLAGS: 00000246 ORIG_RAX: 00000000000000a5 RAX: ffffffffffffffda RBX: 0030656c69662f2e RCX: 00007f4b18ad5b19 RDX: 0000000000000000 RSI: 00000000200000c0 RDI: 0000000000000000 RBP: 00007f4b18b685f0 R08: 0000000000000000 R09: 00005555776124c0 R10: 0000000000000000 R11: 0000000000000246 R12: 00007ffc2e486c70 R13: 00007ffc2e486e98 R14: 431bde82d7b634db R15: 00007f4b18b1e03b </TASK> Reported-and-tested-by: syzbot+73d8fc29ec7cba8286fa@syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=73d8fc29ec7cba8286fa Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com> --- fs/namei.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/fs/namei.c b/fs/namei.c index 4a4a22a08ac2..f5dbccb3aafc 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -1844,6 +1844,9 @@ static const char *pick_link(struct nameidata *nd, struct path *link, if (unlikely(error)) return ERR_PTR(error); + if (!S_ISLNK(inode->i_mode)) + return ERR_PTR(-EINVAL); + res = READ_ONCE(inode->i_link); if (!res) { const char * (*get)(struct dentry *, struct inode *, -- 2.43.0 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH] fs: add check for symlink corrupted 2024-11-15 9:49 ` [PATCH] fs: add check for symlink corrupted Lizhi Xu @ 2024-11-15 11:43 ` Jan Kara 2024-11-16 1:02 ` Lizhi Xu 2024-11-15 13:06 ` Al Viro 1 sibling, 1 reply; 22+ messages in thread From: Jan Kara @ 2024-11-15 11:43 UTC (permalink / raw) To: Lizhi Xu Cc: syzbot+73d8fc29ec7cba8286fa, almaz.alexandrovich, brauner, jack, linux-fsdevel, linux-kernel, ntfs3, syzkaller-bugs, viro On Fri 15-11-24 17:49:08, Lizhi Xu wrote: > syzbot reported a null-ptr-deref in pick_link. [1] > When symlink's inode is corrupted, the value of the i_link is 2 in this case, > it will trigger null pointer deref when accessing *res in pick_link(). > > To avoid this issue, add a check for inode mode, return -EINVAL when it's > not symlink. > > [1] > KASAN: null-ptr-deref in range [0x0000000000000000-0x0000000000000007] > CPU: 0 UID: 0 PID: 5310 Comm: syz-executor255 Not tainted 6.12.0-rc6-syzkaller-00318-ga9cda7c0ffed #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:pick_link+0x51c/0xd50 fs/namei.c:1864 Hum, based on line number is: if (*res == '/') { <<<< HERE error = nd_jump_root(nd); if (unlikely(error)) So res would be non-zero but a small number. > Code: c1 e8 03 42 80 3c 38 00 74 08 48 89 df e8 fc 00 e9 ff 48 8b 2b 48 85 ed 0f 84 92 00 00 00 e8 7b 36 7f ff 48 89 e8 48 c1 e8 03 <42> 0f b6 04 38 84 c0 0f 85 a2 05 00 00 0f b6 5d 00 bf 2f 00 00 00 > RSP: 0018:ffffc9000d147998 EFLAGS: 00010246 > RAX: 0000000000000000 RBX: ffff88804558dec8 RCX: ffff88801ec7a440 > RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000 > RBP: 0000000000000002 R08: ffffffff8215a35f R09: 1ffffffff203a13d > R10: dffffc0000000000 R11: fffffbfff203a13e R12: 1ffff92001a28f93 > R13: ffffc9000d147af8 R14: 1ffff92001a28f5f R15: dffffc0000000000 > FS: 0000555577611380(0000) GS:ffff88801fc00000(0000) knlGS:0000000000000000 > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > CR2: 00007fcc0a595ed8 CR3: 0000000035760000 CR4: 0000000000352ef0 > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 > DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 > Call Trace: > <TASK> > step_into+0xca9/0x1080 fs/namei.c:1923 > lookup_last fs/namei.c:2556 [inline] > path_lookupat+0x16f/0x450 fs/namei.c:2580 > filename_lookup+0x256/0x610 fs/namei.c:2609 > user_path_at+0x3a/0x60 fs/namei.c:3016 > do_mount fs/namespace.c:3844 [inline] > __do_sys_mount fs/namespace.c:4057 [inline] > __se_sys_mount+0x297/0x3c0 fs/namespace.c:4034 > 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:0x7f4b18ad5b19 > 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:00007ffc2e486c48 EFLAGS: 00000246 ORIG_RAX: 00000000000000a5 > RAX: ffffffffffffffda RBX: 0030656c69662f2e RCX: 00007f4b18ad5b19 > RDX: 0000000000000000 RSI: 00000000200000c0 RDI: 0000000000000000 > RBP: 00007f4b18b685f0 R08: 0000000000000000 R09: 00005555776124c0 > R10: 0000000000000000 R11: 0000000000000246 R12: 00007ffc2e486c70 > R13: 00007ffc2e486e98 R14: 431bde82d7b634db R15: 00007f4b18b1e03b > </TASK> > > Reported-and-tested-by: syzbot+73d8fc29ec7cba8286fa@syzkaller.appspotmail.com > Closes: https://syzkaller.appspot.com/bug?extid=73d8fc29ec7cba8286fa > Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com> > --- > fs/namei.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/fs/namei.c b/fs/namei.c > index 4a4a22a08ac2..f5dbccb3aafc 100644 > --- a/fs/namei.c > +++ b/fs/namei.c > @@ -1844,6 +1844,9 @@ static const char *pick_link(struct nameidata *nd, struct path *link, > if (unlikely(error)) > return ERR_PTR(error); > > + if (!S_ISLNK(inode->i_mode)) > + return ERR_PTR(-EINVAL); > + So I don't see how we can get here without inode being a symlink. pick_link() is called from step_into() which has among other things: if (likely(!d_is_symlink(path.dentry)) || ...) do something and return so we are checking whether the inode is a symlink before calling pick_link(). And yes, the d_is_symlink() is using cached type in dentry->d_flags so they could mismatch. But inode is not supposed to change its type during its lifetime so if there is a mismatch that is the problem that needs to be fixed. Honza -- Jan Kara <jack@suse.com> SUSE Labs, CR ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] fs: add check for symlink corrupted 2024-11-15 11:43 ` Jan Kara @ 2024-11-16 1:02 ` Lizhi Xu 2024-11-16 1:25 ` Al Viro 0 siblings, 1 reply; 22+ messages in thread From: Lizhi Xu @ 2024-11-16 1:02 UTC (permalink / raw) To: jack Cc: almaz.alexandrovich, brauner, linux-fsdevel, linux-kernel, lizhi.xu, ntfs3, syzbot+73d8fc29ec7cba8286fa, syzkaller-bugs, viro On Fri, 15 Nov 2024 12:43:06 +0100, Jan Kara wrote: > On Fri 15-11-24 17:49:08, Lizhi Xu wrote: > > syzbot reported a null-ptr-deref in pick_link. [1] > > When symlink's inode is corrupted, the value of the i_link is 2 in this case, > > it will trigger null pointer deref when accessing *res in pick_link(). > > > > To avoid this issue, add a check for inode mode, return -EINVAL when it's > > not symlink. > > > > [1] > > KASAN: null-ptr-deref in range [0x0000000000000000-0x0000000000000007] > > CPU: 0 UID: 0 PID: 5310 Comm: syz-executor255 Not tainted 6.12.0-rc6-syzkaller-00318-ga9cda7c0ffed #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:pick_link+0x51c/0xd50 fs/namei.c:1864 > > Hum, based on line number is: > > if (*res == '/') { <<<< HERE > error = nd_jump_root(nd); > if (unlikely(error)) > > So res would be non-zero but a small number. > > > Code: c1 e8 03 42 80 3c 38 00 74 08 48 89 df e8 fc 00 e9 ff 48 8b 2b 48 85 ed 0f 84 92 00 00 00 e8 7b 36 7f ff 48 89 e8 48 c1 e8 03 <42> 0f b6 04 38 84 c0 0f 85 a2 05 00 00 0f b6 5d 00 bf 2f 00 00 00 > > RSP: 0018:ffffc9000d147998 EFLAGS: 00010246 > > RAX: 0000000000000000 RBX: ffff88804558dec8 RCX: ffff88801ec7a440 > > RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000 > > RBP: 0000000000000002 R08: ffffffff8215a35f R09: 1ffffffff203a13d > > R10: dffffc0000000000 R11: fffffbfff203a13e R12: 1ffff92001a28f93 > > R13: ffffc9000d147af8 R14: 1ffff92001a28f5f R15: dffffc0000000000 > > FS: 0000555577611380(0000) GS:ffff88801fc00000(0000) knlGS:0000000000000000 > > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > > CR2: 00007fcc0a595ed8 CR3: 0000000035760000 CR4: 0000000000352ef0 > > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 > > DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 > > Call Trace: > > <TASK> > > step_into+0xca9/0x1080 fs/namei.c:1923 > > lookup_last fs/namei.c:2556 [inline] > > path_lookupat+0x16f/0x450 fs/namei.c:2580 > > filename_lookup+0x256/0x610 fs/namei.c:2609 > > user_path_at+0x3a/0x60 fs/namei.c:3016 > > do_mount fs/namespace.c:3844 [inline] > > __do_sys_mount fs/namespace.c:4057 [inline] > > __se_sys_mount+0x297/0x3c0 fs/namespace.c:4034 > > 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:0x7f4b18ad5b19 > > 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:00007ffc2e486c48 EFLAGS: 00000246 ORIG_RAX: 00000000000000a5 > > RAX: ffffffffffffffda RBX: 0030656c69662f2e RCX: 00007f4b18ad5b19 > > RDX: 0000000000000000 RSI: 00000000200000c0 RDI: 0000000000000000 > > RBP: 00007f4b18b685f0 R08: 0000000000000000 R09: 00005555776124c0 > > R10: 0000000000000000 R11: 0000000000000246 R12: 00007ffc2e486c70 > > R13: 00007ffc2e486e98 R14: 431bde82d7b634db R15: 00007f4b18b1e03b > > </TASK> > > > > Reported-and-tested-by: syzbot+73d8fc29ec7cba8286fa@syzkaller.appspotmail.com > > Closes: https://syzkaller.appspot.com/bug?extid=73d8fc29ec7cba8286fa > > Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com> > > --- > > fs/namei.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/fs/namei.c b/fs/namei.c > > index 4a4a22a08ac2..f5dbccb3aafc 100644 > > --- a/fs/namei.c > > +++ b/fs/namei.c > > @@ -1844,6 +1844,9 @@ static const char *pick_link(struct nameidata *nd, struct path *link, > > if (unlikely(error)) > > return ERR_PTR(error); > > > > + if (!S_ISLNK(inode->i_mode)) > > + return ERR_PTR(-EINVAL); > > + > > So I don't see how we can get here without inode being a symlink. > pick_link() is called from step_into() which has among other things: > > if (likely(!d_is_symlink(path.dentry)) || ...) Our idea is the same. Because d_is_symlink() has confirmed the mode of symlink in step_into(), I will confirm whether the mode of symlink's inode has changed when the value of i_link is 2 in pick_link(). > do something and return > > so we are checking whether the inode is a symlink before calling > pick_link(). And yes, the d_is_symlink() is using cached type in > dentry->d_flags so they could mismatch. But inode is not supposed to change > its type during its lifetime so if there is a mismatch that is the problem > that needs to be fixed. I think syzbot executed the following two syscalls when triggering this problem: link(&(0x7f0000000200)='./file0\x00', &(0x7f0000000240)='./bus\x00') mount$overlay(0x0, &(0x7f00000000c0)='./bus\x00', 0x0, 0x0, 0x0) Obviously, this is to mount a link. Whether the mount operation itself will change or corrupt the i_link value and mode value of the symlink is not clear to me yet. BR, Lizhi ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] fs: add check for symlink corrupted 2024-11-16 1:02 ` Lizhi Xu @ 2024-11-16 1:25 ` Al Viro 0 siblings, 0 replies; 22+ messages in thread From: Al Viro @ 2024-11-16 1:25 UTC (permalink / raw) To: Lizhi Xu Cc: jack, almaz.alexandrovich, brauner, linux-fsdevel, linux-kernel, ntfs3, syzbot+73d8fc29ec7cba8286fa, syzkaller-bugs On Sat, Nov 16, 2024 at 09:02:07AM +0800, Lizhi Xu wrote: > Our idea is the same. Because d_is_symlink() has confirmed the mode of > symlink in step_into(), I will confirm whether the mode of symlink's inode > has changed when the value of i_link is 2 in pick_link(). > > do something and return > > > > so we are checking whether the inode is a symlink before calling > > pick_link(). And yes, the d_is_symlink() is using cached type in > > dentry->d_flags so they could mismatch. But inode is not supposed to change > > its type during its lifetime so if there is a mismatch that is the problem > > that needs to be fixed. > I think syzbot executed the following two syscalls when triggering this problem: > > link(&(0x7f0000000200)='./file0\x00', &(0x7f0000000240)='./bus\x00') > mount$overlay(0x0, &(0x7f00000000c0)='./bus\x00', 0x0, 0x0, 0x0) > > Obviously, this is to mount a link. Whether the mount operation itself will > change or corrupt the i_link value and mode value of the symlink is not > clear to me yet. Odds are, it's not a valid struct inode instance in the first place. It's not inode->i_link that is a problem (*nothing* should ever store that value in there and ntfs doesn't even try that - grep and you'll see); it's inode itself. Have you tried KASAN-enabled build? Might be interesting to see if it catches anything... ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] fs: add check for symlink corrupted 2024-11-15 9:49 ` [PATCH] fs: add check for symlink corrupted Lizhi Xu 2024-11-15 11:43 ` Jan Kara @ 2024-11-15 13:06 ` Al Viro 2024-11-15 13:24 ` Al Viro 1 sibling, 1 reply; 22+ messages in thread From: Al Viro @ 2024-11-15 13:06 UTC (permalink / raw) To: Lizhi Xu Cc: syzbot+73d8fc29ec7cba8286fa, almaz.alexandrovich, brauner, jack, linux-fsdevel, linux-kernel, ntfs3, syzkaller-bugs On Fri, Nov 15, 2024 at 05:49:08PM +0800, Lizhi Xu wrote: > syzbot reported a null-ptr-deref in pick_link. [1] > When symlink's inode is corrupted, the value of the i_link is 2 in this case, > it will trigger null pointer deref when accessing *res in pick_link(). > > To avoid this issue, add a check for inode mode, return -EINVAL when it's > not symlink. NAK. Don't paper over filesystem bugs at pathwalk time - it's the wrong place for that. Fix it at in-core inode creation time. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] fs: add check for symlink corrupted 2024-11-15 13:06 ` Al Viro @ 2024-11-15 13:24 ` Al Viro 2024-11-16 1:39 ` Lizhi Xu 0 siblings, 1 reply; 22+ messages in thread From: Al Viro @ 2024-11-15 13:24 UTC (permalink / raw) To: Lizhi Xu Cc: syzbot+73d8fc29ec7cba8286fa, almaz.alexandrovich, brauner, jack, linux-fsdevel, linux-kernel, ntfs3, syzkaller-bugs On Fri, Nov 15, 2024 at 01:06:15PM +0000, Al Viro wrote: > On Fri, Nov 15, 2024 at 05:49:08PM +0800, Lizhi Xu wrote: > > syzbot reported a null-ptr-deref in pick_link. [1] > > When symlink's inode is corrupted, the value of the i_link is 2 in this case, > > it will trigger null pointer deref when accessing *res in pick_link(). > > > > To avoid this issue, add a check for inode mode, return -EINVAL when it's > > not symlink. > > NAK. Don't paper over filesystem bugs at pathwalk time - it's the wrong > place for that. Fix it at in-core inode creation time. BTW, seeing that ntfs doesn't even touch ->i_link, you are dealing with aftermath of memory corruption, so it's definitely papering over the actual bug here. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] fs: add check for symlink corrupted 2024-11-15 13:24 ` Al Viro @ 2024-11-16 1:39 ` Lizhi Xu 2024-11-16 2:32 ` Al Viro 0 siblings, 1 reply; 22+ messages in thread From: Lizhi Xu @ 2024-11-16 1:39 UTC (permalink / raw) To: viro Cc: almaz.alexandrovich, brauner, jack, linux-fsdevel, linux-kernel, lizhi.xu, ntfs3, syzbot+73d8fc29ec7cba8286fa, syzkaller-bugs On Fri, 15 Nov 2024 13:24:55 +0000, Al Viro wrote: > On Fri, Nov 15, 2024 at 01:06:15PM +0000, Al Viro wrote: > > On Fri, Nov 15, 2024 at 05:49:08PM +0800, Lizhi Xu wrote: > > > syzbot reported a null-ptr-deref in pick_link. [1] > > > When symlink's inode is corrupted, the value of the i_link is 2 in this case, > > > it will trigger null pointer deref when accessing *res in pick_link(). > > > > > > To avoid this issue, add a check for inode mode, return -EINVAL when it's > > > not symlink. > > > > NAK. Don't paper over filesystem bugs at pathwalk time - it's the wrong > > place for that. Fix it at in-core inode creation time. > > BTW, seeing that ntfs doesn't even touch ->i_link, you are dealing Yes, ntfs3 does not handle the relevant code of i_link. > with aftermath of memory corruption, so it's definitely papering over > the actual bug here. I see that finding out how the value of i_link becomes 2 is the key. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] fs: add check for symlink corrupted 2024-11-16 1:39 ` Lizhi Xu @ 2024-11-16 2:32 ` Al Viro 2024-11-19 11:29 ` [PATCH V2] fs: improve the check of whether i_link has been set Lizhi Xu 0 siblings, 1 reply; 22+ messages in thread From: Al Viro @ 2024-11-16 2:32 UTC (permalink / raw) To: Lizhi Xu Cc: almaz.alexandrovich, brauner, jack, linux-fsdevel, linux-kernel, ntfs3, syzbot+73d8fc29ec7cba8286fa, syzkaller-bugs On Sat, Nov 16, 2024 at 09:39:50AM +0800, Lizhi Xu wrote: > On Fri, 15 Nov 2024 13:24:55 +0000, Al Viro wrote: > > On Fri, Nov 15, 2024 at 01:06:15PM +0000, Al Viro wrote: > > > On Fri, Nov 15, 2024 at 05:49:08PM +0800, Lizhi Xu wrote: > > > > syzbot reported a null-ptr-deref in pick_link. [1] > > > > When symlink's inode is corrupted, the value of the i_link is 2 in this case, > > > > it will trigger null pointer deref when accessing *res in pick_link(). > > > > > > > > To avoid this issue, add a check for inode mode, return -EINVAL when it's > > > > not symlink. > > > > > > NAK. Don't paper over filesystem bugs at pathwalk time - it's the wrong > > > place for that. Fix it at in-core inode creation time. > > > > BTW, seeing that ntfs doesn't even touch ->i_link, you are dealing > Yes, ntfs3 does not handle the relevant code of i_link. > > with aftermath of memory corruption, so it's definitely papering over > > the actual bug here. > I see that finding out how the value of i_link becomes 2 is the key. How about 'how the memory currently pointed to by inode had come to be available for use by something that stored 2 at that particular offset'? ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH V2] fs: improve the check of whether i_link has been set 2024-11-16 2:32 ` Al Viro @ 2024-11-19 11:29 ` Lizhi Xu 2024-11-19 16:36 ` Al Viro 0 siblings, 1 reply; 22+ messages in thread From: Lizhi Xu @ 2024-11-19 11:29 UTC (permalink / raw) To: viro Cc: almaz.alexandrovich, brauner, jack, linux-fsdevel, linux-kernel, lizhi.xu, ntfs3, syzbot+73d8fc29ec7cba8286fa, syzkaller-bugs syzbot reported a null-ptr-deref in pick_link. [1] First, i_link and i_dir_seq are in the same union, they share the same memory address, and i_dir_seq will be updated during the execution of walk_component, which makes the value of i_link equal to i_dir_seq. In this case, setting i_dir_seq is triggered by move_mount, and the calltrace is as follows: move_mount()-> user_path_at()-> filename_lookup()-> path_lookupat()-> lookup_last()-> walk_component()-> __lookup_slow()-> ntfs_lookup()-> d_splice_alias()-> __d_add()-> end_dir_add() In pick_link(), the simple "if (!i_link)" is used to determine whether i_link has been set, which is not rigorous enough. On the other hand, the mode value of the symlink inode becomes REG because attr_set_size() fails to set the attribute and calls ntfs_bad_inode(). By confirming that the i_link pointer value is valid, the null-ptr-deref problem in pick_link can be avoided. [1] KASAN: null-ptr-deref in range [0x0000000000000000-0x0000000000000007] CPU: 0 UID: 0 PID: 5310 Comm: syz-executor255 Not tainted 6.12.0-rc6-syzkaller-00318-ga9cda7c0ffed #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:pick_link+0x51c/0xd50 fs/namei.c:1864 Code: c1 e8 03 42 80 3c 38 00 74 08 48 89 df e8 fc 00 e9 ff 48 8b 2b 48 85 ed 0f 84 92 00 00 00 e8 7b 36 7f ff 48 89 e8 48 c1 e8 03 <42> 0f b6 04 38 84 c0 0f 85 a2 05 00 00 0f b6 5d 00 bf 2f 00 00 00 RSP: 0018:ffffc9000d147998 EFLAGS: 00010246 RAX: 0000000000000000 RBX: ffff88804558dec8 RCX: ffff88801ec7a440 RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000 RBP: 0000000000000002 R08: ffffffff8215a35f R09: 1ffffffff203a13d R10: dffffc0000000000 R11: fffffbfff203a13e R12: 1ffff92001a28f93 R13: ffffc9000d147af8 R14: 1ffff92001a28f5f R15: dffffc0000000000 FS: 0000555577611380(0000) GS:ffff88801fc00000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 00007fcc0a595ed8 CR3: 0000000035760000 CR4: 0000000000352ef0 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 Call Trace: <TASK> step_into+0xca9/0x1080 fs/namei.c:1923 lookup_last fs/namei.c:2556 [inline] path_lookupat+0x16f/0x450 fs/namei.c:2580 filename_lookup+0x256/0x610 fs/namei.c:2609 user_path_at+0x3a/0x60 fs/namei.c:3016 do_mount fs/namespace.c:3844 [inline] __do_sys_mount fs/namespace.c:4057 [inline] __se_sys_mount+0x297/0x3c0 fs/namespace.c:4034 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:0x7f4b18ad5b19 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:00007ffc2e486c48 EFLAGS: 00000246 ORIG_RAX: 00000000000000a5 RAX: ffffffffffffffda RBX: 0030656c69662f2e RCX: 00007f4b18ad5b19 RDX: 0000000000000000 RSI: 00000000200000c0 RDI: 0000000000000000 RBP: 00007f4b18b685f0 R08: 0000000000000000 R09: 00005555776124c0 R10: 0000000000000000 R11: 0000000000000246 R12: 00007ffc2e486c70 R13: 00007ffc2e486e98 R14: 431bde82d7b634db R15: 00007f4b18b1e03b </TASK> Fixes: ad6cc4c338f4 ("finally fold get_link() into pick_link()") Reported-by: syzbot+73d8fc29ec7cba8286fa@syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=73d8fc29ec7cba8286fa Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com> --- V1 --> V2: add the root cause of the i_link not set issue and imporve the check fs/namei.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/namei.c b/fs/namei.c index 4a4a22a08ac2..49bd5ac16137 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -1845,7 +1845,7 @@ static const char *pick_link(struct nameidata *nd, struct path *link, return ERR_PTR(error); res = READ_ONCE(inode->i_link); - if (!res) { + if (!virt_addr_valid(res)) { const char * (*get)(struct dentry *, struct inode *, struct delayed_call *); get = inode->i_op->get_link; -- 2.43.0 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH V2] fs: improve the check of whether i_link has been set 2024-11-19 11:29 ` [PATCH V2] fs: improve the check of whether i_link has been set Lizhi Xu @ 2024-11-19 16:36 ` Al Viro 2024-11-20 3:04 ` [PATCH V3] fs/ntfs3: check if the inode is bad before creating symlink Lizhi Xu 0 siblings, 1 reply; 22+ messages in thread From: Al Viro @ 2024-11-19 16:36 UTC (permalink / raw) To: Lizhi Xu Cc: almaz.alexandrovich, brauner, jack, linux-fsdevel, linux-kernel, ntfs3, syzbot+73d8fc29ec7cba8286fa, syzkaller-bugs On Tue, Nov 19, 2024 at 07:29:45PM +0800, Lizhi Xu wrote: > syzbot reported a null-ptr-deref in pick_link. [1] > > First, i_link and i_dir_seq are in the same union, they share the same memory > address, and i_dir_seq will be updated during the execution of walk_component, > which makes the value of i_link equal to i_dir_seq. > In this case, setting i_dir_seq is triggered by move_mount, and the calltrace > is as follows: > move_mount()-> > user_path_at()-> > filename_lookup()-> > path_lookupat()-> > lookup_last()-> > walk_component()-> > __lookup_slow()-> > ntfs_lookup()-> > d_splice_alias()-> > __d_add()-> > end_dir_add() > > In pick_link(), the simple "if (!i_link)" is used to determine whether i_link > has been set, which is not rigorous enough. > > On the other hand, the mode value of the symlink inode becomes REG because > attr_set_size() fails to set the attribute and calls ntfs_bad_inode(). > By confirming that the i_link pointer value is valid, the null-ptr-deref > problem in pick_link can be avoided. So basically your theory is that make_bad_inode() is called on a live directory inode (already reachable from dcache and remaining there), whereas the sucker somehow gets a new dentry alias which looks like a symlink. Right? NAK on the "mitigation", just in case anyone decides to pick that - no matter how we deal with the problem, sprinkling virt_addr_valid() is *NOT* a solution. ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH V3] fs/ntfs3: check if the inode is bad before creating symlink 2024-11-19 16:36 ` Al Viro @ 2024-11-20 3:04 ` Lizhi Xu 2024-11-20 16:10 ` Al Viro 0 siblings, 1 reply; 22+ messages in thread From: Lizhi Xu @ 2024-11-20 3:04 UTC (permalink / raw) To: viro Cc: almaz.alexandrovich, brauner, jack, linux-fsdevel, linux-kernel, lizhi.xu, ntfs3, syzbot+73d8fc29ec7cba8286fa, syzkaller-bugs syzbot reported a null-ptr-deref in pick_link. [1] First, i_link and i_dir_seq are in the same union, they share the same memory address, and i_dir_seq will be updated during the execution of walk_component, which makes the value of i_link equal to i_dir_seq. Secondly, the chmod execution failed, which resulted in setting the mode value of file0's inode to REG when executing ntfs_bad_inode. Third, when creating a symbolic link using the file0 whose inode has been marked as bad, it is not determined whether its inode is bad, which ultimately leads to null-ptr-deref when performing a mount operation on the symbolic link bus because the i_link value is equal to i_dir_seq=2. Note: ("file0, bus" are defined in reproducer [2]) To avoid null-ptr-deref in pick_link, when creating a symbolic link, first check whether the inode of file is already bad. [1] KASAN: null-ptr-deref in range [0x0000000000000000-0x0000000000000007] CPU: 0 UID: 0 PID: 5310 Comm: syz-executor255 Not tainted 6.12.0-rc6-syzkaller-00318-ga9cda7c0ffed #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:pick_link+0x51c/0xd50 fs/namei.c:1864 Code: c1 e8 03 42 80 3c 38 00 74 08 48 89 df e8 fc 00 e9 ff 48 8b 2b 48 85 ed 0f 84 92 00 00 00 e8 7b 36 7f ff 48 89 e8 48 c1 e8 03 <42> 0f b6 04 38 84 c0 0f 85 a2 05 00 00 0f b6 5d 00 bf 2f 00 00 00 RSP: 0018:ffffc9000d147998 EFLAGS: 00010246 RAX: 0000000000000000 RBX: ffff88804558dec8 RCX: ffff88801ec7a440 RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000 RBP: 0000000000000002 R08: ffffffff8215a35f R09: 1ffffffff203a13d R10: dffffc0000000000 R11: fffffbfff203a13e R12: 1ffff92001a28f93 R13: ffffc9000d147af8 R14: 1ffff92001a28f5f R15: dffffc0000000000 FS: 0000555577611380(0000) GS:ffff88801fc00000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 00007fcc0a595ed8 CR3: 0000000035760000 CR4: 0000000000352ef0 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 Call Trace: <TASK> step_into+0xca9/0x1080 fs/namei.c:1923 lookup_last fs/namei.c:2556 [inline] path_lookupat+0x16f/0x450 fs/namei.c:2580 filename_lookup+0x256/0x610 fs/namei.c:2609 user_path_at+0x3a/0x60 fs/namei.c:3016 do_mount fs/namespace.c:3844 [inline] __do_sys_mount fs/namespace.c:4057 [inline] __se_sys_mount+0x297/0x3c0 fs/namespace.c:4034 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:0x7f4b18ad5b19 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:00007ffc2e486c48 EFLAGS: 00000246 ORIG_RAX: 00000000000000a5 RAX: ffffffffffffffda RBX: 0030656c69662f2e RCX: 00007f4b18ad5b19 RDX: 0000000000000000 RSI: 00000000200000c0 RDI: 0000000000000000 RBP: 00007f4b18b685f0 R08: 0000000000000000 R09: 00005555776124c0 R10: 0000000000000000 R11: 0000000000000246 R12: 00007ffc2e486c70 R13: 00007ffc2e486e98 R14: 431bde82d7b634db R15: 00007f4b18b1e03b </TASK> [2] move_mount(0xffffffffffffff9c, &(0x7f00000003c0)='./file0\x00', 0xffffffffffffff9c, &(0x7f0000000400)='./file0/file0\x00', 0x140) chmod(&(0x7f0000000080)='./file0\x00', 0x0) link(&(0x7f0000000200)='./file0\x00', &(0x7f0000000240)='./bus\x00') mount$overlay(0x0, &(0x7f00000000c0)='./bus\x00', 0x0, 0x0, 0x0) Reported-by: syzbot+73d8fc29ec7cba8286fa@syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=73d8fc29ec7cba8286fa Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com> --- V1 --> V2: add the root cause of the i_link not set issue and imporve the check V2 --> V3: when creating a symbolic link, first check whether the inode of file is bad. fs/ntfs3/inode.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/fs/ntfs3/inode.c b/fs/ntfs3/inode.c index be04d2845bb7..fefbdcf75016 100644 --- a/fs/ntfs3/inode.c +++ b/fs/ntfs3/inode.c @@ -1719,6 +1719,9 @@ int ntfs_link_inode(struct inode *inode, struct dentry *dentry) struct ntfs_sb_info *sbi = inode->i_sb->s_fs_info; struct NTFS_DE *de; + if (is_bad_inode(inode)) + return -EIO; + /* Allocate PATH_MAX bytes. */ de = __getname(); if (!de) -- 2.43.0 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH V3] fs/ntfs3: check if the inode is bad before creating symlink 2024-11-20 3:04 ` [PATCH V3] fs/ntfs3: check if the inode is bad before creating symlink Lizhi Xu @ 2024-11-20 16:10 ` Al Viro 2024-11-21 3:13 ` Lizhi Xu 2024-11-22 7:49 ` Lizhi Xu 0 siblings, 2 replies; 22+ messages in thread From: Al Viro @ 2024-11-20 16:10 UTC (permalink / raw) To: Lizhi Xu Cc: almaz.alexandrovich, brauner, jack, linux-fsdevel, linux-kernel, ntfs3, syzbot+73d8fc29ec7cba8286fa, syzkaller-bugs On Wed, Nov 20, 2024 at 11:04:43AM +0800, Lizhi Xu wrote: > syzbot reported a null-ptr-deref in pick_link. [1] > > First, i_link and i_dir_seq are in the same union, they share the same memory > address, and i_dir_seq will be updated during the execution of walk_component, > which makes the value of i_link equal to i_dir_seq. > > Secondly, the chmod execution failed, which resulted in setting the mode value > of file0's inode to REG when executing ntfs_bad_inode. > > Third, when creating a symbolic link using the file0 whose inode has been marked > as bad, it is not determined whether its inode is bad, which ultimately leads to > null-ptr-deref when performing a mount operation on the symbolic link bus because > the i_link value is equal to i_dir_seq=2. > > Note: ("file0, bus" are defined in reproducer [2]) > > To avoid null-ptr-deref in pick_link, when creating a symbolic link, first check > whether the inode of file is already bad. I would really like to understand how the hell did that bad inode end up passed to d_splice_alias()/d_instantiate()/whatever it had been. That's the root cause - and it looks like ntfs is too free with make_bad_inode() in general, which might cause other problems. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH V3] fs/ntfs3: check if the inode is bad before creating symlink 2024-11-20 16:10 ` Al Viro @ 2024-11-21 3:13 ` Lizhi Xu 2024-11-21 3:55 ` Al Viro 2024-11-22 7:49 ` Lizhi Xu 1 sibling, 1 reply; 22+ messages in thread From: Lizhi Xu @ 2024-11-21 3:13 UTC (permalink / raw) To: viro Cc: almaz.alexandrovich, brauner, jack, linux-fsdevel, linux-kernel, lizhi.xu, ntfs3, syzbot+73d8fc29ec7cba8286fa, syzkaller-bugs On Wed, 20 Nov 2024 16:10:45 +0000, Al Viro wrote: > On Wed, Nov 20, 2024 at 11:04:43AM +0800, Lizhi Xu wrote: > > syzbot reported a null-ptr-deref in pick_link. [1] > > > > First, i_link and i_dir_seq are in the same union, they share the same memory > > address, and i_dir_seq will be updated during the execution of walk_component, > > which makes the value of i_link equal to i_dir_seq. > > > > Secondly, the chmod execution failed, which resulted in setting the mode value > > of file0's inode to REG when executing ntfs_bad_inode. > > > > Third, when creating a symbolic link using the file0 whose inode has been marked > > as bad, it is not determined whether its inode is bad, which ultimately leads to > > null-ptr-deref when performing a mount operation on the symbolic link bus because > > the i_link value is equal to i_dir_seq=2. > > > > Note: ("file0, bus" are defined in reproducer [2]) > > > > To avoid null-ptr-deref in pick_link, when creating a symbolic link, first check > > whether the inode of file is already bad. > > I would really like to understand how the hell did that bad inode end up passed > to d_splice_alias()/d_instantiate()/whatever it had been. 1. In the move_mount() process, the inode is created by ntfs_alloc_inode() and enters d_splice_alias() by ntfs_lookup(), at this time inode is good, as shown below: move_mount()-> user_path_at()-> filename_lookup()-> path_lookupat()-> lookup_last()-> walk_component()-> __lookup_slow()-> ntfs_lookup()-> d_splice_alias()-> 2. The subsequent chmod fails, causing the inode to be set to bad. 3. During the link operation, d_instantiate() is executed in ntfs_link() to associate the bad inode with the dentry. 4. During the mount operation, walk_component executes pick_link, triggering null-ptr-deref. Reproducer: move_mount(0xffffffffffffff9c, &(0x7f00000003c0)='./file0\x00', 0xffffffffffffff9c, &(0x7f0000000400)='./file0/file0\x00', 0x140) chmod(&(0x7f0000000080)='./file0\x00', 0x0) link(&(0x7f0000000200)='./file0\x00', &(0x7f0000000240)='./bus\x00') mount$overlay(0x0, &(0x7f00000000c0)='./bus\x00', 0x0, 0x0, 0x0) > > That's the root cause - and it looks like ntfs is too free with make_bad_inode() > in general, which might cause other problems. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH V3] fs/ntfs3: check if the inode is bad before creating symlink 2024-11-21 3:13 ` Lizhi Xu @ 2024-11-21 3:55 ` Al Viro 2024-11-21 5:27 ` Lizhi Xu 0 siblings, 1 reply; 22+ messages in thread From: Al Viro @ 2024-11-21 3:55 UTC (permalink / raw) To: Lizhi Xu Cc: almaz.alexandrovich, brauner, jack, linux-fsdevel, linux-kernel, ntfs3, syzbot+73d8fc29ec7cba8286fa, syzkaller-bugs On Thu, Nov 21, 2024 at 11:13:29AM +0800, Lizhi Xu wrote: > user_path_at()-> > filename_lookup()-> > path_lookupat()-> > lookup_last()-> > walk_component()-> > __lookup_slow()-> > ntfs_lookup()-> > d_splice_alias()-> > > 2. The subsequent chmod fails, causing the inode to be set to bad. What's wrong with "return an error"? > 3. During the link operation, d_instantiate() is executed in ntfs_link() to associate the bad inode with the dentry. Yecchhh... If nothing else, check for is_bad_inode() should be there for as long as make_bad_inode() is done on live inodes. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH V3] fs/ntfs3: check if the inode is bad before creating symlink 2024-11-21 3:55 ` Al Viro @ 2024-11-21 5:27 ` Lizhi Xu 0 siblings, 0 replies; 22+ messages in thread From: Lizhi Xu @ 2024-11-21 5:27 UTC (permalink / raw) To: viro Cc: almaz.alexandrovich, brauner, jack, linux-fsdevel, linux-kernel, lizhi.xu, ntfs3, syzbot+73d8fc29ec7cba8286fa, syzkaller-bugs On Thu, 21 Nov 2024 03:55:29 +0000, Al Viro wrote: > > user_path_at()-> > > filename_lookup()-> > > path_lookupat()-> > > lookup_last()-> > > walk_component()-> > > __lookup_slow()-> > > ntfs_lookup()-> > > d_splice_alias()-> > > > > 2. The subsequent chmod fails, causing the inode to be set to bad. > > What's wrong with "return an error"? make_bad_inode() is executed in attr_set_size() and the error code -ENOENT is returned; attr_set_size() defined in fs/ntfs3/attrib.c > > > 3. During the link operation, d_instantiate() is executed in ntfs_link() to associate the bad inode with the dentry. > > Yecchhh... If nothing else, check for is_bad_inode() should be there > for as long as make_bad_inode() is done on live inodes. I checked is_bad_inode() in ntfs_link(), see line 146 of ntfs_link(), please see my V3 of the patch [2]. [1] fs/ntfs3/namei.c 19 static int ntfs_link(struct dentry *ode, struct inode *dir, struct dentry *de) 18 { 17 int err; 16 struct inode *inode = d_inode(ode); 15 struct ntfs_inode *ni = ntfs_i(inode); 14 13 if (S_ISDIR(inode->i_mode)) 12 return -EPERM; 11 10 if (inode->i_nlink >= NTFS_LINK_MAX) 9 return -EMLINK; 8 7 ni_lock_dir(ntfs_i(dir)); 6 if (inode != dir) 5 ni_lock(ni); 4 3 inc_nlink(inode); 2 ihold(inode); 1 146 err = ntfs_link_inode(inode, de); 1 2 if (!err) { 3 inode_set_ctime_current(inode); 4 inode_set_mtime_to_ts(dir, inode_set_ctime_current(dir)); 5 mark_inode_dirty(inode); 6 mark_inode_dirty(dir); 7 d_instantiate(de, inode); [2] Reported-by: syzbot+73d8fc29ec7cba8286fa@syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=73d8fc29ec7cba8286fa Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com> --- V1 --> V2: add the root cause of the i_link not set issue and imporve the check V2 --> V3: when creating a symbolic link, first check whether the inode of file is bad. fs/ntfs3/inode.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/fs/ntfs3/inode.c b/fs/ntfs3/inode.c index be04d2845bb7..fefbdcf75016 100644 --- a/fs/ntfs3/inode.c +++ b/fs/ntfs3/inode.c @@ -1719,6 +1719,9 @@ int ntfs_link_inode(struct inode *inode, struct dentry *dentry) struct ntfs_sb_info *sbi = inode->i_sb->s_fs_info; struct NTFS_DE *de; + if (is_bad_inode(inode)) + return -EIO; + /* Allocate PATH_MAX bytes. */ de = __getname(); if (!de) ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH V3] fs/ntfs3: check if the inode is bad before creating symlink 2024-11-20 16:10 ` Al Viro 2024-11-21 3:13 ` Lizhi Xu @ 2024-11-22 7:49 ` Lizhi Xu 2024-11-22 8:10 ` [PATCH V4] " Lizhi Xu 2024-11-23 1:09 ` [PATCH V5] " Lizhi Xu 1 sibling, 2 replies; 22+ messages in thread From: Lizhi Xu @ 2024-11-22 7:49 UTC (permalink / raw) To: viro Cc: almaz.alexandrovich, brauner, jack, linux-fsdevel, linux-kernel, lizhi.xu, ntfs3, syzbot+73d8fc29ec7cba8286fa, syzkaller-bugs On Wed, 20 Nov 2024 16:10:45 +0000, Al Viro wrote: > > syzbot reported a null-ptr-deref in pick_link. [1] > > > > First, i_link and i_dir_seq are in the same union, they share the same memory > > address, and i_dir_seq will be updated during the execution of walk_component, > > which makes the value of i_link equal to i_dir_seq. > > > > Secondly, the chmod execution failed, which resulted in setting the mode value > > of file0's inode to REG when executing ntfs_bad_inode. > > > > Third, when creating a symbolic link using the file0 whose inode has been marked > > as bad, it is not determined whether its inode is bad, which ultimately leads to > > null-ptr-deref when performing a mount operation on the symbolic link bus because > > the i_link value is equal to i_dir_seq=2. > > > > Note: ("file0, bus" are defined in reproducer [2]) > > > > To avoid null-ptr-deref in pick_link, when creating a symbolic link, first check > > whether the inode of file is already bad. > > I would really like to understand how the hell did that bad inode end up passed > to d_splice_alias()/d_instantiate()/whatever it had been. > > That's the root cause - and it looks like ntfs is too free with make_bad_inode() > in general, which might cause other problems. I will release the patch of the v4 version and add root cause: During the execution of the link command, it sets the inode of the symlink file to the already bad inode of file0 by calling d_instantiate, which ultimately leads to null-ptr-deref when performing a mount operation on the symbolic link bus because it use bad inode's i_link and its value is equal to i_dir_seq=2. BR, Lizhi ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH V4] fs/ntfs3: check if the inode is bad before creating symlink 2024-11-22 7:49 ` Lizhi Xu @ 2024-11-22 8:10 ` Lizhi Xu 2024-11-22 11:50 ` Jan Kara 2024-11-23 1:09 ` [PATCH V5] " Lizhi Xu 1 sibling, 1 reply; 22+ messages in thread From: Lizhi Xu @ 2024-11-22 8:10 UTC (permalink / raw) To: lizhi.xu Cc: viro, almaz.alexandrovich, brauner, jack, linux-fsdevel, linux-kernel, ntfs3, syzbot+73d8fc29ec7cba8286fa, syzkaller-bugs syzbot reported a null-ptr-deref in pick_link. [1] First, i_link and i_dir_seq are in the same union, they share the same memory address, and i_dir_seq will be updated during the execution of walk_component, which makes the value of i_link equal to i_dir_seq. Secondly, the chmod execution failed, which resulted in setting the mode value of file0's inode to REG when executing ntfs_bad_inode. Third, during the execution of the link command, it sets the inode of the symlink file to the already bad inode of file0 by calling d_instantiate, which ultimately leads to null-ptr-deref when performing a mount operation on the symbolic link bus because it use bad inode's i_link and its value is equal to i_dir_seq=2. Note: ("file0, bus" are defined in reproducer [2]) To avoid null-ptr-deref in pick_link, when creating a symbolic link, first check whether the inode of file is already bad. [1] KASAN: null-ptr-deref in range [0x0000000000000000-0x0000000000000007] CPU: 0 UID: 0 PID: 5310 Comm: syz-executor255 Not tainted 6.12.0-rc6-syzkaller-00318-ga9cda7c0ffed #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:pick_link+0x51c/0xd50 fs/namei.c:1864 Code: c1 e8 03 42 80 3c 38 00 74 08 48 89 df e8 fc 00 e9 ff 48 8b 2b 48 85 ed 0f 84 92 00 00 00 e8 7b 36 7f ff 48 89 e8 48 c1 e8 03 <42> 0f b6 04 38 84 c0 0f 85 a2 05 00 00 0f b6 5d 00 bf 2f 00 00 00 RSP: 0018:ffffc9000d147998 EFLAGS: 00010246 RAX: 0000000000000000 RBX: ffff88804558dec8 RCX: ffff88801ec7a440 RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000 RBP: 0000000000000002 R08: ffffffff8215a35f R09: 1ffffffff203a13d R10: dffffc0000000000 R11: fffffbfff203a13e R12: 1ffff92001a28f93 R13: ffffc9000d147af8 R14: 1ffff92001a28f5f R15: dffffc0000000000 FS: 0000555577611380(0000) GS:ffff88801fc00000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 00007fcc0a595ed8 CR3: 0000000035760000 CR4: 0000000000352ef0 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 Call Trace: <TASK> step_into+0xca9/0x1080 fs/namei.c:1923 lookup_last fs/namei.c:2556 [inline] path_lookupat+0x16f/0x450 fs/namei.c:2580 filename_lookup+0x256/0x610 fs/namei.c:2609 user_path_at+0x3a/0x60 fs/namei.c:3016 do_mount fs/namespace.c:3844 [inline] __do_sys_mount fs/namespace.c:4057 [inline] __se_sys_mount+0x297/0x3c0 fs/namespace.c:4034 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:0x7f4b18ad5b19 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:00007ffc2e486c48 EFLAGS: 00000246 ORIG_RAX: 00000000000000a5 RAX: ffffffffffffffda RBX: 0030656c69662f2e RCX: 00007f4b18ad5b19 RDX: 0000000000000000 RSI: 00000000200000c0 RDI: 0000000000000000 RBP: 00007f4b18b685f0 R08: 0000000000000000 R09: 00005555776124c0 R10: 0000000000000000 R11: 0000000000000246 R12: 00007ffc2e486c70 R13: 00007ffc2e486e98 R14: 431bde82d7b634db R15: 00007f4b18b1e03b </TASK> [2] move_mount(0xffffffffffffff9c, &(0x7f00000003c0)='./file0\x00', 0xffffffffffffff9c, &(0x7f0000000400)='./file0/file0\x00', 0x140) chmod(&(0x7f0000000080)='./file0\x00', 0x0) link(&(0x7f0000000200)='./file0\x00', &(0x7f0000000240)='./bus\x00') mount$overlay(0x0, &(0x7f00000000c0)='./bus\x00', 0x0, 0x0, 0x0) Reported-by: syzbot+73d8fc29ec7cba8286fa@syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=73d8fc29ec7cba8286fa Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com> --- V1 --> V2: add the root cause of the i_link not set issue and imporve the check V2 --> V3: when creating a symbolic link, first check whether the inode of file is bad. V3 --> V4: add comments for symlink use bad inode, it is the root cause fs/ntfs3/inode.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/fs/ntfs3/inode.c b/fs/ntfs3/inode.c index be04d2845bb7..fefbdcf75016 100644 --- a/fs/ntfs3/inode.c +++ b/fs/ntfs3/inode.c @@ -1719,6 +1719,9 @@ int ntfs_link_inode(struct inode *inode, struct dentry *dentry) struct ntfs_sb_info *sbi = inode->i_sb->s_fs_info; struct NTFS_DE *de; + if (is_bad_inode(inode)) + return -EIO; + /* Allocate PATH_MAX bytes. */ de = __getname(); if (!de) -- 2.43.0 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH V4] fs/ntfs3: check if the inode is bad before creating symlink 2024-11-22 8:10 ` [PATCH V4] " Lizhi Xu @ 2024-11-22 11:50 ` Jan Kara 0 siblings, 0 replies; 22+ messages in thread From: Jan Kara @ 2024-11-22 11:50 UTC (permalink / raw) To: Lizhi Xu Cc: viro, almaz.alexandrovich, brauner, jack, linux-fsdevel, linux-kernel, ntfs3, syzbot+73d8fc29ec7cba8286fa, syzkaller-bugs On Fri 22-11-24 16:10:25, Lizhi Xu wrote: > syzbot reported a null-ptr-deref in pick_link. [1] > > First, i_link and i_dir_seq are in the same union, they share the same memory > address, and i_dir_seq will be updated during the execution of walk_component, > which makes the value of i_link equal to i_dir_seq. > > Secondly, the chmod execution failed, which resulted in setting the mode value > of file0's inode to REG when executing ntfs_bad_inode. > > Third, during the execution of the link command, it sets the inode of the > symlink file to the already bad inode of file0 by calling d_instantiate, which > ultimately leads to null-ptr-deref when performing a mount operation on the > symbolic link bus because it use bad inode's i_link and its value is equal to > i_dir_seq=2. > > Note: ("file0, bus" are defined in reproducer [2]) > > To avoid null-ptr-deref in pick_link, when creating a symbolic link, first check > whether the inode of file is already bad. So actually there's no symbolic link involved here at all (which what was confusing me all the time). > move_mount(0xffffffffffffff9c, &(0x7f00000003c0)='./file0\x00', 0xffffffffffffff9c, &(0x7f0000000400)='./file0/file0\x00', 0x140) > chmod(&(0x7f0000000080)='./file0\x00', 0x0) > link(&(0x7f0000000200)='./file0\x00', &(0x7f0000000240)='./bus\x00') > mount$overlay(0x0, &(0x7f00000000c0)='./bus\x00', 0x0, 0x0, 0x0) This creates only a hardlink. And in fact the creation of the link seems to be totally irrelevant for this problem. I believe: move_mount(0xffffffffffffff9c, &(0x7f00000003c0)='./file0\x00', 0xffffffffffffff9c, &(0x7f0000000400)='./file0/file0\x00', 0x140) chmod(&(0x7f0000000080)='./file0\x00', 0x0) mount$overlay(0x0, &(0x7f00000000c0)='./file0\x00', 0x0, 0x0, 0x0) would be as good reproducer of the problem. The core of the problem is that NTFS3 calls make_bad_inode() on inode that is accessible to userspace and is something else than a regular file. As long as that happens, some variant of this NULL-ptr-dereference can happen as well, just the reproducers will be somewhat different. So I don't think patching ntfs_link_inode() makes a lot of sense. If anything, I'd patch NTFS3 to not mark the inode as bad somewhere inside ntfs_setattr() and deal with the error in a better way. Honza > > Reported-by: syzbot+73d8fc29ec7cba8286fa@syzkaller.appspotmail.com > Closes: https://syzkaller.appspot.com/bug?extid=73d8fc29ec7cba8286fa > Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com> > --- > V1 --> V2: add the root cause of the i_link not set issue and imporve the check > V2 --> V3: when creating a symbolic link, first check whether the inode of file is bad. > V3 --> V4: add comments for symlink use bad inode, it is the root cause > > fs/ntfs3/inode.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/fs/ntfs3/inode.c b/fs/ntfs3/inode.c > index be04d2845bb7..fefbdcf75016 100644 > --- a/fs/ntfs3/inode.c > +++ b/fs/ntfs3/inode.c > @@ -1719,6 +1719,9 @@ int ntfs_link_inode(struct inode *inode, struct dentry *dentry) > struct ntfs_sb_info *sbi = inode->i_sb->s_fs_info; > struct NTFS_DE *de; > > + if (is_bad_inode(inode)) > + return -EIO; > + > /* Allocate PATH_MAX bytes. */ > de = __getname(); > if (!de) > -- > 2.43.0 > -- Jan Kara <jack@suse.com> SUSE Labs, CR ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH V5] fs/ntfs3: check if the inode is bad before creating symlink 2024-11-22 7:49 ` Lizhi Xu 2024-11-22 8:10 ` [PATCH V4] " Lizhi Xu @ 2024-11-23 1:09 ` Lizhi Xu 2024-11-23 1:32 ` Al Viro 1 sibling, 1 reply; 22+ messages in thread From: Lizhi Xu @ 2024-11-23 1:09 UTC (permalink / raw) To: lizhi.xu Cc: viro, almaz.alexandrovich, brauner, jack, linux-fsdevel, linux-kernel, ntfs3, syzbot+73d8fc29ec7cba8286fa, syzkaller-bugs syzbot reported a null-ptr-deref in pick_link. [1] First, i_link and i_dir_seq are in the same union, they share the same memory address, and i_dir_seq will be updated during the execution of walk_component, which makes the value of i_link equal to i_dir_seq. Secondly, the chmod execution failed, which resulted in setting the mode value of file0's inode to REG when executing ntfs_bad_inode. Third, during the execution of the link command, it sets the inode of the hardlink file to the already bad inode of file0 by calling d_instantiate, which ultimately leads to null-ptr-deref when performing a mount operation on the hardlink bus because it use bad inode's i_link and its value is equal to i_dir_seq=2. Note: ("file0, bus" are defined in reproducer [2]) To avoid null-ptr-deref in pick_link, when creating a hardlink, first check whether the inode of file is already bad. [1] KASAN: null-ptr-deref in range [0x0000000000000000-0x0000000000000007] CPU: 0 UID: 0 PID: 5310 Comm: syz-executor255 Not tainted 6.12.0-rc6-syzkaller-00318-ga9cda7c0ffed #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:pick_link+0x51c/0xd50 fs/namei.c:1864 Code: c1 e8 03 42 80 3c 38 00 74 08 48 89 df e8 fc 00 e9 ff 48 8b 2b 48 85 ed 0f 84 92 00 00 00 e8 7b 36 7f ff 48 89 e8 48 c1 e8 03 <42> 0f b6 04 38 84 c0 0f 85 a2 05 00 00 0f b6 5d 00 bf 2f 00 00 00 RSP: 0018:ffffc9000d147998 EFLAGS: 00010246 RAX: 0000000000000000 RBX: ffff88804558dec8 RCX: ffff88801ec7a440 RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000 RBP: 0000000000000002 R08: ffffffff8215a35f R09: 1ffffffff203a13d R10: dffffc0000000000 R11: fffffbfff203a13e R12: 1ffff92001a28f93 R13: ffffc9000d147af8 R14: 1ffff92001a28f5f R15: dffffc0000000000 FS: 0000555577611380(0000) GS:ffff88801fc00000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 00007fcc0a595ed8 CR3: 0000000035760000 CR4: 0000000000352ef0 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 Call Trace: <TASK> step_into+0xca9/0x1080 fs/namei.c:1923 lookup_last fs/namei.c:2556 [inline] path_lookupat+0x16f/0x450 fs/namei.c:2580 filename_lookup+0x256/0x610 fs/namei.c:2609 user_path_at+0x3a/0x60 fs/namei.c:3016 do_mount fs/namespace.c:3844 [inline] __do_sys_mount fs/namespace.c:4057 [inline] __se_sys_mount+0x297/0x3c0 fs/namespace.c:4034 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:0x7f4b18ad5b19 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:00007ffc2e486c48 EFLAGS: 00000246 ORIG_RAX: 00000000000000a5 RAX: ffffffffffffffda RBX: 0030656c69662f2e RCX: 00007f4b18ad5b19 RDX: 0000000000000000 RSI: 00000000200000c0 RDI: 0000000000000000 RBP: 00007f4b18b685f0 R08: 0000000000000000 R09: 00005555776124c0 R10: 0000000000000000 R11: 0000000000000246 R12: 00007ffc2e486c70 R13: 00007ffc2e486e98 R14: 431bde82d7b634db R15: 00007f4b18b1e03b </TASK> [2] move_mount(0xffffffffffffff9c, &(0x7f00000003c0)='./file0\x00', 0xffffffffffffff9c, &(0x7f0000000400)='./file0/file0\x00', 0x140) chmod(&(0x7f0000000080)='./file0\x00', 0x0) link(&(0x7f0000000200)='./file0\x00', &(0x7f0000000240)='./bus\x00') mount$overlay(0x0, &(0x7f00000000c0)='./bus\x00', 0x0, 0x0, 0x0) Reported-by: syzbot+73d8fc29ec7cba8286fa@syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=73d8fc29ec7cba8286fa Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com> --- V1 --> V2: add the root cause of the i_link not set issue and imporve the check V2 --> V3: when creating a symbolic link, first check whether the inode of file is bad. V3 --> V4: add comments for symlink use bad inode, it is the root cause V4 --> V5: update comments for link command, it is creating hardlink fs/ntfs3/inode.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/fs/ntfs3/inode.c b/fs/ntfs3/inode.c index be04d2845bb7..fefbdcf75016 100644 --- a/fs/ntfs3/inode.c +++ b/fs/ntfs3/inode.c @@ -1719,6 +1719,9 @@ int ntfs_link_inode(struct inode *inode, struct dentry *dentry) struct ntfs_sb_info *sbi = inode->i_sb->s_fs_info; struct NTFS_DE *de; + if (is_bad_inode(inode)) + return -EIO; + /* Allocate PATH_MAX bytes. */ de = __getname(); if (!de) -- 2.43.0 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH V5] fs/ntfs3: check if the inode is bad before creating symlink 2024-11-23 1:09 ` [PATCH V5] " Lizhi Xu @ 2024-11-23 1:32 ` Al Viro 2024-11-24 4:43 ` [PATCH V6] fs/ntfs3: check if the inode is bad before instantiating dentry Lizhi Xu 0 siblings, 1 reply; 22+ messages in thread From: Al Viro @ 2024-11-23 1:32 UTC (permalink / raw) To: Lizhi Xu Cc: almaz.alexandrovich, brauner, jack, linux-fsdevel, linux-kernel, ntfs3, syzbot+73d8fc29ec7cba8286fa, syzkaller-bugs On Sat, Nov 23, 2024 at 09:09:56AM +0800, Lizhi Xu wrote: [snip] hardlink, surely? ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH V6] fs/ntfs3: check if the inode is bad before instantiating dentry 2024-11-23 1:32 ` Al Viro @ 2024-11-24 4:43 ` Lizhi Xu 0 siblings, 0 replies; 22+ messages in thread From: Lizhi Xu @ 2024-11-24 4:43 UTC (permalink / raw) To: viro Cc: almaz.alexandrovich, brauner, jack, linux-fsdevel, linux-kernel, lizhi.xu, ntfs3, syzbot+73d8fc29ec7cba8286fa, syzkaller-bugs syzbot reported a null-ptr-deref in pick_link. [1] First, i_link and i_dir_seq are in the same union, they share the same memory address, and i_dir_seq will be updated during the execution of walk_component, which makes the value of i_link equal to i_dir_seq. Secondly, the chmod execution failed, which resulted in setting the mode value of file0's inode to REG when executing ntfs_bad_inode. Third, during the execution of the link syscall, two things are done in d_instantiate. One is to set the bad inode to the dentry of the bus. More importantly, the symlink flag obtained in d_flags_for_inode(bad inode) is used to update the d_flags of the dentry of the bus through __d_set_inode_and_type, which will cause the dentry flag of the created bus to have symlink. The bus should have been a hardlink, but it eventually became a symlink. These two points constitute the root cause. When executing the mount syscall, in walk_component(), when executing to step_into() and using d_is_symlink() to judge, because the flag of the dentry of the bus is symlink, it leads to the wrong process, and trigger null-ptr-deref in pick_link(). Note: ("file0, bus" are defined in reproducer [2]) To avoid null-ptr-deref in pick_link, when creating a hardlink, first check whether the inode of file is already bad. [1] KASAN: null-ptr-deref in range [0x0000000000000000-0x0000000000000007] CPU: 0 UID: 0 PID: 5310 Comm: syz-executor255 Not tainted 6.12.0-rc6-syzkaller-00318-ga9cda7c0ffed #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:pick_link+0x51c/0xd50 fs/namei.c:1864 Code: c1 e8 03 42 80 3c 38 00 74 08 48 89 df e8 fc 00 e9 ff 48 8b 2b 48 85 ed 0f 84 92 00 00 00 e8 7b 36 7f ff 48 89 e8 48 c1 e8 03 <42> 0f b6 04 38 84 c0 0f 85 a2 05 00 00 0f b6 5d 00 bf 2f 00 00 00 RSP: 0018:ffffc9000d147998 EFLAGS: 00010246 RAX: 0000000000000000 RBX: ffff88804558dec8 RCX: ffff88801ec7a440 RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000 RBP: 0000000000000002 R08: ffffffff8215a35f R09: 1ffffffff203a13d R10: dffffc0000000000 R11: fffffbfff203a13e R12: 1ffff92001a28f93 R13: ffffc9000d147af8 R14: 1ffff92001a28f5f R15: dffffc0000000000 FS: 0000555577611380(0000) GS:ffff88801fc00000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 00007fcc0a595ed8 CR3: 0000000035760000 CR4: 0000000000352ef0 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 Call Trace: <TASK> step_into+0xca9/0x1080 fs/namei.c:1923 lookup_last fs/namei.c:2556 [inline] path_lookupat+0x16f/0x450 fs/namei.c:2580 filename_lookup+0x256/0x610 fs/namei.c:2609 user_path_at+0x3a/0x60 fs/namei.c:3016 do_mount fs/namespace.c:3844 [inline] __do_sys_mount fs/namespace.c:4057 [inline] __se_sys_mount+0x297/0x3c0 fs/namespace.c:4034 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:0x7f4b18ad5b19 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:00007ffc2e486c48 EFLAGS: 00000246 ORIG_RAX: 00000000000000a5 RAX: ffffffffffffffda RBX: 0030656c69662f2e RCX: 00007f4b18ad5b19 RDX: 0000000000000000 RSI: 00000000200000c0 RDI: 0000000000000000 RBP: 00007f4b18b685f0 R08: 0000000000000000 R09: 00005555776124c0 R10: 0000000000000000 R11: 0000000000000246 R12: 00007ffc2e486c70 R13: 00007ffc2e486e98 R14: 431bde82d7b634db R15: 00007f4b18b1e03b </TASK> [2] move_mount(0xffffffffffffff9c, &(0x7f00000003c0)='./file0\x00', 0xffffffffffffff9c, &(0x7f0000000400)='./file0/file0\x00', 0x140) chmod(&(0x7f0000000080)='./file0\x00', 0x0) link(&(0x7f0000000200)='./file0\x00', &(0x7f0000000240)='./bus\x00') mount$overlay(0x0, &(0x7f00000000c0)='./bus\x00', 0x0, 0x0, 0x0) Reported-by: syzbot+73d8fc29ec7cba8286fa@syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=73d8fc29ec7cba8286fa Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com> --- V1 --> V2: add the root cause of the i_link not set issue and imporve the check V2 --> V3: when creating a symbolic link, first check whether the inode of file is bad. V3 --> V4: add comments for symlink use bad inode, it is the root cause V4 --> V5: update comments for link command, it is creating hardlink V5 --> V6: add comments for link syscall return a symlink and update subject fs/ntfs3/inode.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/fs/ntfs3/inode.c b/fs/ntfs3/inode.c index be04d2845bb7..fefbdcf75016 100644 --- a/fs/ntfs3/inode.c +++ b/fs/ntfs3/inode.c @@ -1719,6 +1719,9 @@ int ntfs_link_inode(struct inode *inode, struct dentry *dentry) struct ntfs_sb_info *sbi = inode->i_sb->s_fs_info; struct NTFS_DE *de; + if (is_bad_inode(inode)) + return -EIO; + /* Allocate PATH_MAX bytes. */ de = __getname(); if (!de) -- 2.43.0 ^ permalink raw reply related [flat|nested] 22+ messages in thread
end of thread, other threads:[~2024-11-24 4:43 UTC | newest] Thread overview: 22+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-11-14 18:08 [syzbot] [ntfs3?] general protection fault in pick_link syzbot 2024-11-15 9:49 ` [PATCH] fs: add check for symlink corrupted Lizhi Xu 2024-11-15 11:43 ` Jan Kara 2024-11-16 1:02 ` Lizhi Xu 2024-11-16 1:25 ` Al Viro 2024-11-15 13:06 ` Al Viro 2024-11-15 13:24 ` Al Viro 2024-11-16 1:39 ` Lizhi Xu 2024-11-16 2:32 ` Al Viro 2024-11-19 11:29 ` [PATCH V2] fs: improve the check of whether i_link has been set Lizhi Xu 2024-11-19 16:36 ` Al Viro 2024-11-20 3:04 ` [PATCH V3] fs/ntfs3: check if the inode is bad before creating symlink Lizhi Xu 2024-11-20 16:10 ` Al Viro 2024-11-21 3:13 ` Lizhi Xu 2024-11-21 3:55 ` Al Viro 2024-11-21 5:27 ` Lizhi Xu 2024-11-22 7:49 ` Lizhi Xu 2024-11-22 8:10 ` [PATCH V4] " Lizhi Xu 2024-11-22 11:50 ` Jan Kara 2024-11-23 1:09 ` [PATCH V5] " Lizhi Xu 2024-11-23 1:32 ` Al Viro 2024-11-24 4:43 ` [PATCH V6] fs/ntfs3: check if the inode is bad before instantiating dentry Lizhi Xu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox