Re: [RFC 1/1] shiftfs: uid/gid shifting bind mount

From: Vivek Goyal <vgoyal@redhat.com>
To: Al Viro <viro@ZenIV.linux.org.uk>
Cc: James Bottomley <James.Bottomley@HansenPartnership.com>,
	Djalal Harouni <tixxdz@gmail.com>, Chris Mason <clm@fb.com>,
	Theodore Tso <tytso@mit.edu>,
	Josh Triplett <josh@joshtriplett.org>,
	"Eric W. Biederman" <ebiederm@xmission.com>,
	Andy Lutomirski <luto@kernel.org>,
	Seth Forshee <seth.forshee@canonical.com>,
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-security-module@vger.kernel.org,
	Dongsu Park <dongsu@endocode.com>,
	David Herrmann <dh.herrmann@googlemail.com>,
	Miklos Szeredi <mszeredi@redhat.com>,
	Alban Crequy <alban.crequy@gmail.com>,
	"Serge E. Hallyn" <serge@hallyn.com>,
	Phil Estes <estesp@gmail.com>
Subject: Re: [RFC 1/1] shiftfs: uid/gid shifting bind mount
Date: Fri, 17 Feb 2017 15:27:49 -0500	[thread overview]
Message-ID: <20170217202749.GA15841@redhat.com> (raw)
In-Reply-To: <20170217175118.GF29622@ZenIV.linux.org.uk>

On Fri, Feb 17, 2017 at 05:51:18PM +0000, Al Viro wrote:
> On Fri, Feb 17, 2017 at 09:24:40AM -0800, James Bottomley wrote:
> 
> > > What happens when somebody comes along and creates the damn thing on 
> > > the underlying fs?  _Not_ via your code, that is - using the 
> > > underlying fs mounted elsewhere.
> > 
> > Point taken.  This, I think fixes the dcache revalidation issue.
> 
> No, it doesn't.  Consider a local filesystem.  Those do not have any
> ->d_revalidate() - the kernel bloody well knows what happens to
> directories.  If e.g. a previously absent file gets created, it's
> been done by the kernel itself and dentry has been made positive; if
> a previously existing file has been removed, dentry has either become
> negative or, if it had been pinned (e.g. file was opened at the time,
> or your code had been holding a reference to it, etc.) it will be unhashed
> so that new lookups won't find it, etc.  No need to revalidate anything.
> 
> Now, consider your code.  You've done a lookup in the underlying fs.
> It has, at the time, come negative, so you have your (negative) dentry
> pointing to that on the underlying fs.  If somebody comes and does
> e.g. mkdir() via your fs, it will call vfs_mkdir() on the underlying
> sucker, hopefully turning it positive and associate a new in-core inode
> with your previously negative dentry.  But what happens if mkdir is done
> via underlying fs, or via another instance of yours over the same tree?
> Underlying dentry goes positive; yours is still negative.  The underlying
> fs either doesn't have ->d_revalidate() or, if there is one it says that
> the underlying dentry is valid, thank you very much, no need to invalidate
> anything.
> 
> In other words, your patch does nothing for object getting created.

I thought assumption here is that underlying subtree is not changed
outside of shiftfs. IIUC, overlayfs has the same assumption.

Two shiftfs instances writing to same dir will be a problem though.

Vivek