linux-ext4 - Re: [PATCH v2 4/6] fs: Establish locking order for unrelated directories

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: <20230601-flora-hemmung-31a1e66b5179@brauner>
Date:   Thu, 1 Jun 2023 18:21:00 +0200
From:   Christian Brauner <brauner@...nel.org>
To:     Jan Kara <jack@...e.cz>
Cc:     David Laight <David.Laight@...LAB.COM>,
        Al Viro <viro@...IV.linux.org.uk>,
        "linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>,
        Miklos Szeredi <miklos@...redi.hu>,
        "Darrick J. Wong" <djwong@...nel.org>, Ted Tso <tytso@....edu>,
        Jaegeuk Kim <jaegeuk@...nel.org>,
        "linux-ext4@...r.kernel.org" <linux-ext4@...r.kernel.org>,
        "linux-xfs@...r.kernel.org" <linux-xfs@...r.kernel.org>,
        "linux-f2fs-devel@...ts.sourceforge.net" 
        <linux-f2fs-devel@...ts.sourceforge.net>,
        "stable@...r.kernel.org" <stable@...r.kernel.org>
Subject: Re: [PATCH v2 4/6] fs: Establish locking order for unrelated
 directories

On Thu, Jun 01, 2023 at 06:13:53PM +0200, Jan Kara wrote:
> On Thu 01-06-23 15:37:32, David Laight wrote:
> > ...
> > > > > + * Lock any non-NULL argument. The caller must make sure that if he is passing
> > > > > + * in two directories, one is not ancestor of the other
> > 
> > Not directly relevant to this change but is the 'not an ancestor'
> > check actually robust?
> > 
> > I found a condition in which the kernel 'pwd' code (which follows
> > the inode chain) failed to stop at the base of a chroot.
> > 
> > I suspect that the ancestor check would fail the same way.
> 
> Honestly, I'm not sure how this could be the case but I'm not a dcache
> expert. d_ancestor() works on dentries and the whole dcache code pretty
> much relies on the fact that there always is at most one dentry for any
> directory. Also in case we call d_ancestor() from this code, we have the
> whole filesystem locked from any other directory moves so the ancestor
> relationship of two dirs cannot change (which is different from pwd code
> AFAIK). So IMHO no failure is possible in our case.

Yes, this is a red herring. What matters is that the tree topology can't
change which is up to the caller to guarantee. And where it's called
we're under s_vfs_rename_mutex. It's also literally mentioned in the
directory locking documentation.