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

[Al Viro]

On Thu, Apr 24, 2008 at 04:09:18PM +0200, Miklos Szeredi wrote:
> > Because you are mixing the "this sucker will be used for write access for
> > this interval" and "do what is needed to create a file".  The latter is
> > not guaranteed to coincide with the former and that in itself is enough.
> 
> I lost you there, sorry.  Can you please rephrase a bit less
> abstractedly?

The area to be protected against remount is not guaranteed to coincide with
vfs_...() or to contain nothing specific to this caller.

> > Bullshit.  It's not just "prevent modification".  It's "make sure that
> > no remount r/o happens while we do that".
> 
> Sure.
> 
> >  fh_verify() doesn't modify.
> > It does check, though, and later we have that check duplicated by
> > will_write/wont_write pair bracketing a part of sequence.
> 
> So what?  All the other checks are also duplicated within
> vfs_create()->may_create()->permission().

RTFS.  permission() doesn't do "is that vfsmount read-only" checks, exactly
because it's 100% bogus - either you cover it with entire area where we
are guaranteed to stay r/w, or it's by definition racy.

> > ecryptfs should not use the bloody vfsmount, for fuck sake!  You are
> > confusing access to fs with access to fs via specific vfsmount.  And
> > pretending that the latter is fundamental operation.
> 
> Umm, isn't it?  Want to redo open() without a vfsmount?

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?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/