linux-kernel - Re: dcache: remove trylock loops (was Re: [BUG] lock_parent() breakage when used from shrink_dentry

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: <20180312203346.GP30522@ZenIV.linux.org.uk>
Date:   Mon, 12 Mar 2018 20:33:46 +0000
From:   Al Viro <viro@...IV.linux.org.uk>
To:     NeilBrown <neilb@...e.com>
Cc:     Linus Torvalds <torvalds@...ux-foundation.org>,
        linux-fsdevel <linux-fsdevel@...r.kernel.org>,
        Christoph Hellwig <hch@....de>,
        Thomas Gleixner <tglx@...utronix.de>,
        Peter Zijlstra <peterz@...radead.org>,
        Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Eric Biederman <ebiederm@...ssion.com>
Subject: Re: dcache: remove trylock loops (was Re: [BUG] lock_parent()
 breakage when used from shrink_dentry_list())

On Mon, Mar 12, 2018 at 08:05:41PM +0000, Al Viro wrote:

> Does any of the d_find_alias() callers want those dentries?  That is,
> non-directories from d_obtain_alias() still not attached to a parent;
> note that exportfs_decode_fh() is *not* one of such places - we don't
> use d_find_alias() there at all.  If there's no such caller, we can
> bloody well just drop the discon_alias logics and be done with that;
> if there is, that commit has introduced a bug.  I might have missed
> a part of threads related to that patch, so my apologies if it had
> been discussed.
> 
> Neil, what's the situation there?  A lot of those callers clearly treat
> the "only disconnected IS_ROOT alias exist" same as "no aliases at all";
> it looks like the change might have been the right thing, but it sure
> as hell shouldn't be an undocumented side effect...

FWIW, callers are
drivers/staging/lustre/lustre/llite/llite_lib.c:2448:           dentry = d_find_alias(page->mapping->host);
	definitely doesn't want them
fs/ceph/caps.c:2945:    while ((dn = d_find_alias(inode))) {
fs/ceph/mds_client.c:1930:      dentry = d_find_alias(inode);
	ditto - it's building a pathname.
fs/ceph/mds_client.c:2910:      dentry = d_find_alias(inode);
	ditto.
fs/ceph/mds_client.c:3374:      parent = d_find_alias(inode);
	clearly at least expects a directory (feeds to d_lookup(), etc.)
fs/coda/cache.c:112:    alias_de = d_find_alias(inode);
	directory-only
fs/fat/namei_vfat.c:736:        alias = d_find_alias(inode);
	we will reject any IS_ROOT there anyway
fs/fs-writeback.c:2071:         dentry = d_find_alias(inode);
	wants a name, these don't have one.
fs/fuse/dir.c:966:      dir = d_find_alias(parent);
	directory-only
fs/hostfs/hostfs_kern.c:129:    dentry = d_find_alias(ino);
	wants a pathname
fs/nilfs2/super.c:931:          dentry = d_find_alias(inode);
	directory-only
fs/nilfs2/super.c:1025:                 dentry = d_find_alias(inode);
	bloody better be a directory (root one, at that)
fs/xfs/xfs_filestream.c:293:    dentry = d_find_alias(inode);
	no parent, no love.
mm/shmem.c:3435:                dentry = d_find_alias(inode);
	no d_obtain_alias() on that one
security/commoncap.c:391:       dentry = d_find_alias(inode);
security/lsm_audit.c:295:               dentry = d_find_alias(inode);
	wants a name
security/selinux/hooks.c:1536:                  dentry = d_find_alias(inode);
security/selinux/hooks.c:1646:                          dentry = d_find_alias(inode);

So all but four of them clearly do *not* want those suckers.  And remaining
ones are in
	* ceph invalidate_aliases()
	* cap_inode_getsecurity()
	* selinux inode_doinit_with_dentry() (two call sites, very alike)
Do any of those want that stuff?