Re: [patch 00/13] vfs: add helpers to check r/o bind mounts

["J. Bruce Fields" <bfields@fieldses.org>]

On Thu, Apr 24, 2008 at 07:13:32PM +0100, Al Viro wrote:
> On Thu, Apr 24, 2008 at 01:29:49PM -0400, Erez Zadok wrote:
> > In message <20080424142857.GF15214@ZenIV.linux.org.uk>, Al Viro writes:
> > > On Thu, Apr 24, 2008 at 04:09:18PM +0200, Miklos Szeredi wrote:
> > [...]
> > > FWIW, I'm not all that happy about the way ecryptfs_interpose() is done,
> > > while we are at it.  We get the sucker opened by whoever steps on given
> > > place in the tree first, with subsequent operations done using the resulting
> > > struct file.  With fallback to r/o open.  What happens to somebody who
> > > tries to open it with enough permissions to do r/w?
> > 
> > Yes, ecryptfs_interpose() calls ecryptfs_init_persistent_file() which calls
> > dentry_open(O_RDWR).  What's the proposed solution for this in the face of
> > r/o vfsmounts?  How could ecryptfs avoid calling this dentry_open in the
> > first place?
> 
> Doesn't have anything to do with vfsmounts (you have one to deal with and
> if it's r/o, it's equivalent to just doing the entire thing on top of r/o
> fs; not interesting).
> 
> No, what I'm worried about is much simpler.  Look: we have a file on
> underlying fs, owned by root.root with 644 for permissions.  Comes a
> luser and tries to open the counterpart of that file in ecryptfs; that
> triggers ecryptfs_interpose() and attempts to open file.  Of course,
> that's going to fail - it's not world-writable.  So then it (actually
> ecryptfs_init_persistent_file()) falls back to opening with O_RDONLY.
> Which succeeds just fine and file (opened r/o) is set as ->lower_file.
> 
> Now comes root and tries to open the damn thing r/w.  It should be able
> to and if it came first it'd get it; as it is, what it gets is ->lower_file
> and that puppy is opened read-only and you have no guarantee that underlying
> fs will not go bonkers seeing write attempts on it (e.g. open for write
> doing a bit more setup of ->private_data, etc.).

This looks like the same ugly stuff that the nfsv4 server is currently
doing; see, e.g., fs/nfsd/nfs4state.c:nfs4_upgrade_open().

A concrete example of a bug (e.g., export FOOfs over nfsd, do an open
upgrade, watch the oops) would be great motivation to finally get this
fixed, if you know of such an example off the top of your head....

--b.