linux-kernel - Re: [PATCH] fs: Prevent non-symlinks from entering pick link

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <tencent_ED33789EDE1B3B4BB21B4325EDBA10BB7F08@qq.com>
Date: Wed, 18 Jun 2025 14:53:55 +0800
From: Edward Adam Davis <eadavis@...com>
To: viro@...iv.linux.org.uk
Cc: almaz.alexandrovich@...agon-software.com,
	brauner@...nel.org,
	eadavis@...com,
	jack@...e.cz,
	linux-fsdevel@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	ntfs3@...ts.linux.dev,
	syzbot+1aa90f0eb1fc3e77d969@...kaller.appspotmail.com,
	syzkaller-bugs@...glegroups.com
Subject: Re: [PATCH] fs: Prevent non-symlinks from entering pick link

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