[syzbot] [ntfs3?] general protection fault in pick_link (2)

18 views
Skip to first unread message

syzbot

unread,
Jun 17, 2025, 4:01:30 AM6/17/25
to almaz.ale...@paragon-software.com, bra...@kernel.org, ja...@suse.cz, linux-...@vger.kernel.org, linux-...@vger.kernel.org, nt...@lists.linux.dev, syzkall...@googlegroups.com, vi...@zeniv.linux.org.uk
Hello,

syzbot found the following issue on:

HEAD commit: 9afe652958c3 Merge tag 'x86_urgent_for_6.16-rc3' of git://..
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=10cf95d4580000
kernel config: https://syzkaller.appspot.com/x/.config?x=d11f52d3049c3790
dashboard link: https://syzkaller.appspot.com/bug?extid=1aa90f0eb1fc3e77d969
compiler: Debian clang version 20.1.6 (++20250514063057+1e4d39e07757-1~exp1~20250514183223.118), Debian LLD 20.1.6
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=11969e82580000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=14fd450c580000

Downloadable assets:
disk image (non-bootable): https://storage.googleapis.com/syzbot-assets/d900f083ada3/non_bootable_disk-9afe6529.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/46695f4e5fdb/vmlinux-9afe6529.xz
kernel image: https://storage.googleapis.com/syzbot-assets/4357674be01a/bzImage-9afe6529.xz
mounted in repro: https://storage.googleapis.com/syzbot-assets/dd817d4f3932/mount_0.gz

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

ntfs3(loop0): ino=1b, "file0" ntfs_rename
Oops: general protection fault, probably for non-canonical address 0xdffffc0000000000: 0000 [#1] SMP KASAN NOPTI
KASAN: null-ptr-deref in range [0x0000000000000000-0x0000000000000007]
CPU: 0 UID: 0 PID: 5313 Comm: syz-executor352 Not tainted 6.16.0-rc2-syzkaller-00024-g9afe652958c3 #0 PREEMPT(full)
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+0x4f1/0xe80 fs/namei.c:1949
Code: 4c 89 f7 e8 81 fa ea ff 4d 8b 36 4d 85 f6 0f 84 9b 00 00 00 e8 50 7c 87 ff 49 bf 00 00 00 00 00 fc ff df 4c 89 f0 48 c1 e8 03 <42> 0f b6 04 38 84 c0 0f 85 b9 06 00 00 41 0f b6 2e bf 2f 00 00 00
RSP: 0018:ffffc9000d2477e8 EFLAGS: 00010246
RAX: 0000000000000000 RBX: ffffc9000d247908 RCX: ffff88801a34c880
RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000
RBP: 0000000000000000 R08: 0000000000000000 R09: ffffffff823d3d98
R10: 000000003b9aca00 R11: ffffffff823cb0a0 R12: 1ffff92001a48f8b
R13: ffffc9000d247c20 R14: 0000000000000002 R15: dffffc0000000000
FS: 00005555725f0380(0000) GS:ffff88808d251000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f4a1afaf000 CR3: 00000000438a8000 CR4: 0000000000352ef0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
<TASK>
step_into+0xc5d/0xf30 fs/namei.c:2008
open_last_lookups fs/namei.c:3843 [inline]
path_openat+0x1bc6/0x3830 fs/namei.c:4052
do_filp_open+0x1fa/0x410 fs/namei.c:4082
do_sys_openat2+0x121/0x1c0 fs/open.c:1437
do_sys_open fs/open.c:1452 [inline]
__do_sys_open fs/open.c:1460 [inline]
__se_sys_open fs/open.c:1456 [inline]
__x64_sys_open+0x11e/0x150 fs/open.c:1456
do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
do_syscall_64+0xfa/0x3b0 arch/x86/entry/syscall_64.c:94
entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7f5aa2adeed9
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:00007ffc2e7cf7e8 EFLAGS: 00000246 ORIG_RAX: 0000000000000002
RAX: ffffffffffffffda RBX: 00007f5aa2b47ac0 RCX: 00007f5aa2adeed9
RDX: 0000000000000000 RSI: 0000000000048500 RDI: 0000200000000a80
RBP: 0000200000001240 R08: 00005555725f14c0 R09: 00005555725f14c0
R10: 00005555725f14c0 R11: 0000000000000246 R12: 0000000000000000
R13: 00007ffc2e7cfa38 R14: 431bde82d7b634db R15: 00007f5aa2b2803b
</TASK>
Modules linked in:
---[ end trace 0000000000000000 ]---
RIP: 0010:pick_link+0x4f1/0xe80 fs/namei.c:1949
Code: 4c 89 f7 e8 81 fa ea ff 4d 8b 36 4d 85 f6 0f 84 9b 00 00 00 e8 50 7c 87 ff 49 bf 00 00 00 00 00 fc ff df 4c 89 f0 48 c1 e8 03 <42> 0f b6 04 38 84 c0 0f 85 b9 06 00 00 41 0f b6 2e bf 2f 00 00 00
RSP: 0018:ffffc9000d2477e8 EFLAGS: 00010246
RAX: 0000000000000000 RBX: ffffc9000d247908 RCX: ffff88801a34c880
RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000
RBP: 0000000000000000 R08: 0000000000000000 R09: ffffffff823d3d98
R10: 000000003b9aca00 R11: ffffffff823cb0a0 R12: 1ffff92001a48f8b
R13: ffffc9000d247c20 R14: 0000000000000002 R15: dffffc0000000000
FS: 00005555725f0380(0000) GS:ffff88808d251000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 000055ff6c3df070 CR3: 00000000438a8000 CR4: 0000000000352ef0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
----------------
Code disassembly (best guess):
0: 4c 89 f7 mov %r14,%rdi
3: e8 81 fa ea ff call 0xffeafa89
8: 4d 8b 36 mov (%r14),%r14
b: 4d 85 f6 test %r14,%r14
e: 0f 84 9b 00 00 00 je 0xaf
14: e8 50 7c 87 ff call 0xff877c69
19: 49 bf 00 00 00 00 00 movabs $0xdffffc0000000000,%r15
20: fc ff df
23: 4c 89 f0 mov %r14,%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 b9 06 00 00 jne 0x6f0
37: 41 0f b6 2e movzbl (%r14),%ebp
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 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

Edward Adam Davis

unread,
Jun 17, 2025, 8:47:10 AM6/17/25
to syzbot+1aa90f...@syzkaller.appspotmail.com, linux-...@vger.kernel.org, syzkall...@googlegroups.com
#syz test

diff --git a/fs/namei.c b/fs/namei.c
index f761cafaeaad..5b8a69d882d1 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -2005,6 +2005,10 @@ static const char *step_into(struct nameidata *nd, int flags,
if (path.mnt == nd->path.mnt)
mntget(path.mnt);
}
+
+ if (inode && is_bad_inode(inode))
+ return NULL;
+
return pick_link(nd, &path, inode, flags);
}

diff --git a/fs/ntfs3/frecord.c b/fs/ntfs3/frecord.c
index 756e1306fe6c..10006241fa8e 100644
--- a/fs/ntfs3/frecord.c
+++ b/fs/ntfs3/frecord.c
@@ -3027,8 +3027,10 @@ int ni_rename(struct ntfs_inode *dir_ni, struct ntfs_inode *new_dir_ni,
err = ni_add_name(new_dir_ni, ni, new_de);
if (!err) {
err = ni_remove_name(dir_ni, ni, de, &de2, &undo);
- if (err && ni_remove_name(new_dir_ni, ni, new_de, &de2, &undo))
+ if (err) {
+ ni_remove_name(new_dir_ni, ni, new_de, &de2, &undo);
*is_bad = true;
+ }
}

/*

syzbot

unread,
Jun 17, 2025, 9:19:05 AM6/17/25
to ead...@qq.com, linux-...@vger.kernel.org, syzkall...@googlegroups.com
Hello,

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

Reported-by: syzbot+1aa90f...@syzkaller.appspotmail.com
Tested-by: syzbot+1aa90f...@syzkaller.appspotmail.com

Tested on:

commit: 9afe6529 Merge tag 'x86_urgent_for_6.16-rc3' of git://..
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=14eac50c580000
kernel config: https://syzkaller.appspot.com/x/.config?x=6a237c32900fc479
dashboard link: https://syzkaller.appspot.com/bug?extid=1aa90f0eb1fc3e77d969
compiler: Debian clang version 20.1.6 (++20250514063057+1e4d39e07757-1~exp1~20250514183223.118), Debian LLD 20.1.6
patch: https://syzkaller.appspot.com/x/patch.diff?x=1098c370580000

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

Edward Adam Davis

unread,
Jun 17, 2025, 10:33:26 PM6/17/25
to syzbot+1aa90f...@syzkaller.appspotmail.com, linux-...@vger.kernel.org, syzkall...@googlegroups.com
#syz test

diff --git a/fs/namei.c b/fs/namei.c
index 4bb889fc980b..291f29a04e09 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -2005,6 +2005,10 @@ static const char *step_into(struct nameidata *nd, int flags,
if (path.mnt == nd->path.mnt)
mntget(path.mnt);
}
+
+ if (inode && !S_ISLINK(inode->mode))

syzbot

unread,
Jun 17, 2025, 10:55:06 PM6/17/25
to ead...@qq.com, linux-...@vger.kernel.org, syzkall...@googlegroups.com
Hello,

syzbot tried to test the proposed patch but the build/boot failed:

fs/namei.c:2009:16: error: call to undeclared function 'S_ISLINK'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
fs/namei.c:2009:32: error: no member named 'mode' in 'struct inode'


Tested on:

commit: 52da431b Merge tag 'libnvdimm-fixes-6.16-rc3' of git:/..
git tree: upstream
kernel config: https://syzkaller.appspot.com/x/.config?x=6a237c32900fc479
dashboard link: https://syzkaller.appspot.com/bug?extid=1aa90f0eb1fc3e77d969
compiler: Debian clang version 20.1.6 (++20250514063057+1e4d39e07757-1~exp1~20250514183223.118), Debian LLD 20.1.6
patch: https://syzkaller.appspot.com/x/patch.diff?x=1128750c580000

Edward Adam Davis

unread,
Jun 17, 2025, 11:04:05 PM6/17/25
to syzbot+1aa90f...@syzkaller.appspotmail.com, linux-...@vger.kernel.org, syzkall...@googlegroups.com
#syz test

diff --git a/fs/namei.c b/fs/namei.c
index 4bb889fc980b..291f29a04e09 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -2005,6 +2005,10 @@ static const char *step_into(struct nameidata *nd, int flags,
if (path.mnt == nd->path.mnt)
mntget(path.mnt);
}
+
+ if (inode && !S_ISLNK(inode->i_mode))

syzbot

unread,
Jun 17, 2025, 11:30:04 PM6/17/25
to ead...@qq.com, linux-...@vger.kernel.org, syzkall...@googlegroups.com
Hello,

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

Reported-by: syzbot+1aa90f...@syzkaller.appspotmail.com
Tested-by: syzbot+1aa90f...@syzkaller.appspotmail.com

Tested on:

commit: 52da431b Merge tag 'libnvdimm-fixes-6.16-rc3' of git:/..
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=143ef90c580000
kernel config: https://syzkaller.appspot.com/x/.config?x=6a237c32900fc479
dashboard link: https://syzkaller.appspot.com/bug?extid=1aa90f0eb1fc3e77d969
compiler: Debian clang version 20.1.6 (++20250514063057+1e4d39e07757-1~exp1~20250514183223.118), Debian LLD 20.1.6
patch: https://syzkaller.appspot.com/x/patch.diff?x=104ef90c580000

Edward Adam Davis

unread,
Jun 17, 2025, 11:31:00 PM6/17/25
to syzbot+1aa90f...@syzkaller.appspotmail.com, almaz.ale...@paragon-software.com, bra...@kernel.org, ja...@suse.cz, linux-...@vger.kernel.org, linux-...@vger.kernel.org, nt...@lists.linux.dev, syzkall...@googlegroups.com, vi...@zeniv.linux.org.uk
The reproducer uses a file0 on a ntfs3 file system with a corrupted i_link.
When renaming, the file0's inode is marked as a bad inode because the file
name cannot be deleted. However, before renaming, file0 is a directory.
After the renaming fails, it is marked as a bad inode, which makes it a
regular file. In any case, when opening it after creating a hard link,
pick_link() should not be entered because it is not a symbolic link from
beginning to end.

Add a check on the symbolic link before entering pick_link() to avoid
triggering unknown exceptions when performing the i_link acquisition
operation on other types of files.

Reported-by: syzbot+1aa90f...@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=1aa90f0eb1fc3e77d969
Tested-by: syzbot+1aa90f...@syzkaller.appspotmail.com
Signed-off-by: Edward Adam Davis <ead...@qq.com>
---
fs/namei.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/fs/namei.c b/fs/namei.c
index 4bb889fc980b..1524a5359d46 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -2005,6 +2005,10 @@ static const char *step_into(struct nameidata *nd, int flags,
if (path.mnt == nd->path.mnt)
mntget(path.mnt);
}
+
+ if (inode && !S_ISLNK(inode->i_mode))
+ return NULL;
+
return pick_link(nd, &path, inode, flags);
}

--
2.43.0

Al Viro

unread,
Jun 18, 2025, 12:50:24 AM6/18/25
to Edward Adam Davis, syzbot+1aa90f...@syzkaller.appspotmail.com, almaz.ale...@paragon-software.com, bra...@kernel.org, ja...@suse.cz, linux-...@vger.kernel.org, linux-...@vger.kernel.org, nt...@lists.linux.dev, syzkall...@googlegroups.com
On Wed, Jun 18, 2025 at 11:30:48AM +0800, Edward Adam Davis wrote:
> The reproducer uses a file0 on a ntfs3 file system with a corrupted i_link.
> When renaming, the file0's inode is marked as a bad inode because the file
> name cannot be deleted. However, before renaming, file0 is a directory.
> After the renaming fails, it is marked as a bad inode, which makes it a
> regular file. In any case, when opening it after creating a hard link,
> pick_link() should not be entered because it is not a symbolic link from
> beginning to end.
>
> Add a check on the symbolic link before entering pick_link() to avoid
> triggering unknown exceptions when performing the i_link acquisition
> operation on other types of files.
>
> Reported-by: syzbot+1aa90f...@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=1aa90f0eb1fc3e77d969
> Tested-by: syzbot+1aa90f...@syzkaller.appspotmail.com
> Signed-off-by: Edward Adam Davis <ead...@qq.com>

NAK. This is not the first time that garbage is suggested and no,
we are not going to paper over that shite in fs/namei.c.

Not going to happen.

You ARE NOT ALLOWED to call make_bad_inode() on a live inode, period.
Never, ever to be done.

There's a lot of assertions it violates and there's no chance in
hell to plaster each with that kind of checks.

Fix NTFS. End of story.

Al Viro

unread,
Jun 18, 2025, 1:02:06 AM6/18/25
to Edward Adam Davis, syzbot+1aa90f...@syzkaller.appspotmail.com, almaz.ale...@paragon-software.com, bra...@kernel.org, ja...@suse.cz, linux-...@vger.kernel.org, linux-...@vger.kernel.org, nt...@lists.linux.dev, syzkall...@googlegroups.com
On Wed, Jun 18, 2025 at 05:50:16AM +0100, Al Viro wrote:

> NAK. This is not the first time that garbage is suggested and no,
> we are not going to paper over that shite in fs/namei.c.
>
> Not going to happen.
>
> You ARE NOT ALLOWED to call make_bad_inode() on a live inode, period.
> Never, ever to be done.
>
> There's a lot of assertions it violates and there's no chance in
> hell to plaster each with that kind of checks.
>
> Fix NTFS. End of story.

To elaborate a bit: if you look at the end of e.g. their attr_set_size(),
you'll see
out:
if (is_bad) {
bad_inode:
_ntfs_bad_inode(&ni->vfs_inode);
}
return err;
}

This is a bug. So are similar places all over the place there.
You are not supposed to use make_bad_inode() as a general-purpose
"something went wrong, don't wanna see it anymore" tool.

And as long as it stays there, any fuzzing reports of ntfs are pretty
much worthless - any of those places (easily located by grepping for
_ntfs_bad_inode) can fuck the kernel up. Once ntfs folks get around
to saner error recovery, it would make sense to start looking into
fuzzing that thing again. Until then - nope. Again, this is *NOT*
going to be papered over in a random set of places (pretty certain
to remain incomplete) in VFS.

Al Viro

unread,
Jun 18, 2025, 1:27:53 AM6/18/25
to Edward Adam Davis, syzbot+1aa90f...@syzkaller.appspotmail.com, almaz.ale...@paragon-software.com, bra...@kernel.org, ja...@suse.cz, linux-...@vger.kernel.org, linux-...@vger.kernel.org, nt...@lists.linux.dev, syzkall...@googlegroups.com
Note that anything that calls __d_add(dentry, inode) with is_bad_inode(inode)
(or d_add(), or d_instantiate(), or d_splice_alias() under the same conditions)
is also FUBAR.

So's anything that calls make_bad_inode() on a struct inode that might be
in process of being passed to one of those functions by another thread.

This is fundamentally wrong; bad inodes are not supposed to end up attached
to dentries.

Edward Adam Davis

unread,
Jun 18, 2025, 1:34:35 AM6/18/25
to vi...@zeniv.linux.org.uk, almaz.ale...@paragon-software.com, bra...@kernel.org, ead...@qq.com, ja...@suse.cz, linux-...@vger.kernel.org, linux-...@vger.kernel.org, nt...@lists.linux.dev, syzbot+1aa90f...@syzkaller.appspotmail.com, syzkall...@googlegroups.com
On Wed, 18 Jun 2025 06:27:47 +0100, Al Viro wrote:
> Note that anything that calls __d_add(dentry, inode) with is_bad_inode(inode)
> (or d_add(), or d_instantiate(), or d_splice_alias() under the same conditions)
> is also FUBAR.
>
> So's anything that calls make_bad_inode() on a struct inode that might be
> in process of being passed to one of those functions by another thread.
>
> This is fundamentally wrong; bad inodes are not supposed to end up attached
> to dentries.
As far as I know, pick_link() is used to resolve the target path of a
symbolic link (symlink). Can you explain why pick_link() is executed on
a directory or a regular file?

Al Viro

unread,
Jun 18, 2025, 2:18:23 AM6/18/25
to Edward Adam Davis, almaz.ale...@paragon-software.com, bra...@kernel.org, ja...@suse.cz, linux-...@vger.kernel.org, linux-...@vger.kernel.org, nt...@lists.linux.dev, syzbot+1aa90f...@syzkaller.appspotmail.com, syzkall...@googlegroups.com
Because the inode_operations of that thing contains ->get_link(). Which means
"symlink" to dcache. Again, there is code all over the place written in
assumption that no dentry will ever have ->d_inode pointing to any of those.

No, we are not going to paper over that in __d_add() or __d_instantiate() either;
it's fundamentally a losing game. _Maybe_ a couple of WARN_ON() when built with
CONFIG_DEBUG_VFS or something similar, but that would only make for slightly
more specific diagnostics; not all that useful, since you can literally grep for
_ntfs_bad_inode to pick the location of actual underlying bugs.

Again, the underlying bug is that make_bad_inode() is called on a live inode.
In some cases it's "icache lookup finds a normal inode, d_splice_alias() is called
to attach it to dentry, while another thread decides to call make_bad_inode() on
it - that would evict it from icache, but we'd already found it there earlier".
In some it's outright "we have an inode attached to dentry - that's how we got
it in the first place; let's call make_bad_inode() on it just for shits and giggles".
Either is a bug.

_ntfs_bad_inode() uses are completely broken. Matter of fact, we probably ought to
retire make_bad_inode() - there are few callers and most of them don't actually
need anything other than remove_inode_hash() (e.g. iget_failed()). In any case,
whether there is a case for several new helpers or not, the kind of use
_ntfs_bad_inode() gets is right out.

Edward Adam Davis

unread,
Jun 18, 2025, 2:54:13 AM6/18/25
to vi...@zeniv.linux.org.uk, almaz.ale...@paragon-software.com, bra...@kernel.org, ead...@qq.com, ja...@suse.cz, linux-...@vger.kernel.org, linux-...@vger.kernel.org, nt...@lists.linux.dev, syzbot+1aa90f...@syzkaller.appspotmail.com, syzkall...@googlegroups.com
On Wed, 18 Jun 2025 07:18:15 +0100, Al Viro wrote:
> > On Wed, 18 Jun 2025 06:27:47 +0100, Al Viro wrote:
> > > Note that anything that calls __d_add(dentry, inode) with is_bad_inode(inode)
> > > (or d_add(), or d_instantiate(), or d_splice_alias() under the same conditions)
> > > is also FUBAR.
> > >
> > > So's anything that calls make_bad_inode() on a struct inode that might be
> > > in process of being passed to one of those functions by another thread.
> > >
> > > This is fundamentally wrong; bad inodes are not supposed to end up attached
> > > to dentries.
> > As far as I know, pick_link() is used to resolve the target path of a
> > symbolic link (symlink). Can you explain why pick_link() is executed on
> > a directory or a regular file?
>
> Because the inode_operations of that thing contains ->get_link().
I removed _ntfs_bad_inode() and it fixed the problem.
I should have thought more carefully about what you said about the bad inode.
> Again, the underlying bug is that make_bad_inode() is called on a live inode.
> In some cases it's "icache lookup finds a normal inode, d_splice_alias() is called
> to attach it to dentry, while another thread decides to call make_bad_inode() on
> it - that would evict it from icache, but we'd already found it there earlier".
> In some it's outright "we have an inode attached to dentry - that's how we got
> it in the first place; let's call make_bad_inode() on it just for shits and giggles".

BR,
Edward

Edward Adam Davis

unread,
Jun 18, 2025, 3:32:11 AM6/18/25
to ead...@qq.com, almaz.ale...@paragon-software.com, bra...@kernel.org, ja...@suse.cz, linux-...@vger.kernel.org, linux-...@vger.kernel.org, nt...@lists.linux.dev, syzbot+1aa90f...@syzkaller.appspotmail.com, syzkall...@googlegroups.com, vi...@zeniv.linux.org.uk
The reproducer uses a file0 on a ntfs3 file system with a corrupted i_link.
When renaming, the file0's inode is marked as a bad inode because the file
name cannot be deleted.

The underlying bug is that make_bad_inode() is called on a live inode.
In some cases it's "icache lookup finds a normal inode, d_splice_alias()
is called to attach it to dentry, while another thread decides to call
make_bad_inode() on it - that would evict it from icache, but we'd already
found it there earlier".
In some it's outright "we have an inode attached to dentry - that's how we
got it in the first place; let's call make_bad_inode() on it just for shits
and giggles".

Fixes: 78ab59fee07f ("fs/ntfs3: Rework file operations")
Signed-off-by: Edward Adam Davis <ead...@qq.com>
---
V1 -> V2: fix it by removing set bad inode

fs/ntfs3/frecord.c | 7 +++----
fs/ntfs3/namei.c | 10 +++-------
fs/ntfs3/ntfs_fs.h | 3 +--
3 files changed, 7 insertions(+), 13 deletions(-)

diff --git a/fs/ntfs3/frecord.c b/fs/ntfs3/frecord.c
index 756e1306fe6c..7afbb4418eb2 100644
--- a/fs/ntfs3/frecord.c
+++ b/fs/ntfs3/frecord.c
@@ -3003,8 +3003,7 @@ int ni_add_name(struct ntfs_inode *dir_ni, struct ntfs_inode *ni,
* ni_rename - Remove one name and insert new name.
*/
int ni_rename(struct ntfs_inode *dir_ni, struct ntfs_inode *new_dir_ni,
- struct ntfs_inode *ni, struct NTFS_DE *de, struct NTFS_DE *new_de,
- bool *is_bad)
+ struct ntfs_inode *ni, struct NTFS_DE *de, struct NTFS_DE *new_de)
{
int err;
struct NTFS_DE *de2 = NULL;
@@ -3027,8 +3026,8 @@ int ni_rename(struct ntfs_inode *dir_ni, struct ntfs_inode *new_dir_ni,
err = ni_add_name(new_dir_ni, ni, new_de);
if (!err) {
err = ni_remove_name(dir_ni, ni, de, &de2, &undo);
- if (err && ni_remove_name(new_dir_ni, ni, new_de, &de2, &undo))
- *is_bad = true;
+ WARN_ON(err && ni_remove_name(new_dir_ni, ni, new_de, &de2,
+ &undo));
}

/*
diff --git a/fs/ntfs3/namei.c b/fs/ntfs3/namei.c
index b807744fc6a9..0db7ca3b64ea 100644
--- a/fs/ntfs3/namei.c
+++ b/fs/ntfs3/namei.c
@@ -244,7 +244,7 @@ static int ntfs_rename(struct mnt_idmap *idmap, struct inode *dir,
struct ntfs_inode *ni = ntfs_i(inode);
struct inode *new_inode = d_inode(new_dentry);
struct NTFS_DE *de, *new_de;
- bool is_same, is_bad;
+ bool is_same;
/*
* de - memory of PATH_MAX bytes:
* [0-1024) - original name (dentry->d_name)
@@ -313,12 +313,8 @@ static int ntfs_rename(struct mnt_idmap *idmap, struct inode *dir,
if (dir_ni != new_dir_ni)
ni_lock_dir2(new_dir_ni);

- is_bad = false;
- err = ni_rename(dir_ni, new_dir_ni, ni, de, new_de, &is_bad);
- if (is_bad) {
- /* Restore after failed rename failed too. */
- _ntfs_bad_inode(inode);
- } else if (!err) {
+ err = ni_rename(dir_ni, new_dir_ni, ni, de, new_de);
+ if (!err) {
simple_rename_timestamp(dir, dentry, new_dir, new_dentry);
mark_inode_dirty(inode);
mark_inode_dirty(dir);
diff --git a/fs/ntfs3/ntfs_fs.h b/fs/ntfs3/ntfs_fs.h
index 36b8052660d5..f54635df18fa 100644
--- a/fs/ntfs3/ntfs_fs.h
+++ b/fs/ntfs3/ntfs_fs.h
@@ -577,8 +577,7 @@ int ni_add_name(struct ntfs_inode *dir_ni, struct ntfs_inode *ni,
struct NTFS_DE *de);

int ni_rename(struct ntfs_inode *dir_ni, struct ntfs_inode *new_dir_ni,
- struct ntfs_inode *ni, struct NTFS_DE *de, struct NTFS_DE *new_de,
- bool *is_bad);
+ struct ntfs_inode *ni, struct NTFS_DE *de, struct NTFS_DE *new_de);

bool ni_is_dirty(struct inode *inode);

--
2.43.0

Reply all
Reply to author
Forward
0 new messages