Received-SPF: pass (google.com: domain of linux-ext4-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 1 Jun 2023 18:13:53 +0200
From:   Jan Kara <jack@suse.cz>
To:     David Laight <David.Laight@ACULAB.COM>
Cc:     'Jan Kara' <jack@suse.cz>, Christian Brauner <brauner@kernel.org>,
        Al Viro <viro@ZenIV.linux.org.uk>,
        "linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>,
        Miklos Szeredi <miklos@szeredi.hu>,
        "Darrick J. Wong" <djwong@kernel.org>, Ted Tso <tytso@mit.edu>,
        Jaegeuk Kim <jaegeuk@kernel.org>,
        "linux-ext4@vger.kernel.org" <linux-ext4@vger.kernel.org>,
        "linux-xfs@vger.kernel.org" <linux-xfs@vger.kernel.org>,
        "linux-f2fs-devel@lists.sourceforge.net" 
        <linux-f2fs-devel@lists.sourceforge.net>,
        "stable@vger.kernel.org" <stable@vger.kernel.org>
Subject: Re: [PATCH v2 4/6] fs: Establish locking order for unrelated
 directories
Message-ID: <20230601161353.4o6but7hb7i7qfki@quack3>
References: <20230601104525.27897-1-jack@suse.cz>
 <20230601105830.13168-4-jack@suse.cz>
 <20230601-gebracht-gesehen-c779a56b3bf3@brauner>
 <20230601152449.h4ur5zrfqjqygujd@quack3>
 <c5f209a6263b4f039c5eafcafddf90ca@AcuMS.aculab.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <c5f209a6263b4f039c5eafcafddf90ca@AcuMS.aculab.com>
Precedence: bulk

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.

								Honza

> 
> IIRC the problematic code used unshare() to 'escape' from
> a network natespace.
> If it was inside a chroot (that wasn't on a mount point) there
> ware two copies of the 'chroot /' inode and the match failed.
> 
> I might be able to find the test case.
> 
> 	David
> 
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> Registration No: 1397386 (Wales)
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR