[patch 00/10] vfs: add helpers to check r/o bind mounts v3

[patch 00/10] vfs: add helpers to check r/o bind mounts v3

[Miklos Szeredi]

Resend of the path_* API patches.  This is against the VFS cleanup
patches posted recently.

Description is in the first patch header.

Changes from the previous submission:

 - rename() API is made more symmetric
 - all impacted vfs_* functions are made static

Comments (not just Al's) are welcome ;)

Thanks,
Miklos

[patch 01/10] vfs: add path_create() and path_mknod()

[Miklos Szeredi]

[-- Attachment #1: path_create.patch --]
[-- Type: text/plain, Size: 13263 bytes --]

From: Miklos Szeredi <mszeredi@suse.cz>

R/O bind mounts require operations which modify the filesystem to be
wrapped in mnt_want_write()/mnt_drop_write().  Create helpers which do
this, so callers won't need to bother, and more importantly, cannot
forget!  Call these path_*, analogous to vfs_*.  Since there are no
callers of vfs_* left, make them static.

Overall this patchset is just 23 lines in the red, but at the same
time it fixes several places in nfsd and the whole of ecryptfs, where
the mnt_want_write/drop_write() calls were missing.

It will also help with merging certain security modules, which need to
know the path within the namespace, and not just within the
filesystem.  These helpers will allow the security hooks to be in a
common place, and need not be repeated in all callers.

Note, that the mnt_want_write/drop_write() bracketing provided by the
path_* functions strictly necessary in all cases, since the caller may
do it's own bracketing to span multiple VFS calls.  However, this does
not make the checks in path_* incorrect, just unnecessary.

The advantages of the path_* API are:

  - it's consistent

  - it provides some (not all) guarantees, i.e. it's easier to prove
    that all callers play by the rules

  - for the syscall case it has zero cost

  - for all the other cases it has either zero, or minimal cost

It does require the caller to have a vfsmount available, but it's hard
to imagine that the caller does not have it:

  - most userspace calls do have it, as they are either operating on a
    path or a file descriptor.  There are some exceptions like sync(2)
    and ustat(2), the latter not being a very exemplary interface, and
    neither of them being relevant to this discussion.

  - all kernel callers (nfs export, stacking) should have it, as they
    need it for open() anyway

And even if some theoretical caller didn't have the vfsmount, it still
should be easy to allocate one, providing a clean way to do the r/o
bracketing, and not requiring another mechanism to be exported that
provides the same functionality on the superblock.


This patch:

Introduce path_create() and path_mknod().  Make vfs_create() and
vfs_mknod() static.

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---
 fs/ecryptfs/inode.c |   33 ++++++++++------------
 fs/namei.c          |   75 +++++++++++++++++++++++++++-------------------------
 fs/nfsd/vfs.c       |   19 +++++++++----
 include/linux/fs.h  |    4 +-
 ipc/mqueue.c        |    6 +++-
 net/unix/af_unix.c  |    6 ----
 6 files changed, 76 insertions(+), 67 deletions(-)

Index: linux-2.6/fs/namei.c
===================================================================
--- linux-2.6.orig/fs/namei.c	2008-05-05 11:29:27.000000000 +0200
+++ linux-2.6/fs/namei.c	2008-05-05 11:29:29.000000000 +0200
@@ -1579,7 +1579,7 @@ void unlock_rename(struct dentry *p1, st
 	}
 }
 
-int vfs_create(struct inode *dir, struct dentry *dentry, int mode,
+static int vfs_create(struct inode *dir, struct dentry *dentry, int mode,
 		struct nameidata *nd)
 {
 	int error = may_create(dir, dentry, nd);
@@ -1601,6 +1601,20 @@ int vfs_create(struct inode *dir, struct
 	return error;
 }
 
+int path_create(struct path *dir_path, struct dentry *dentry, int mode,
+		struct nameidata *nd)
+{
+	int error = mnt_want_write(dir_path->mnt);
+
+	if (!error) {
+		error = vfs_create(dir_path->dentry->d_inode, dentry, mode, nd);
+		mnt_drop_write(dir_path->mnt);
+	}
+
+	return error;
+}
+EXPORT_SYMBOL(path_create);
+
 int may_open(struct nameidata *nd, int acc_mode, int flag)
 {
 	struct dentry *dentry = nd->path.dentry;
@@ -2017,7 +2031,7 @@ fail:
 }
 EXPORT_SYMBOL_GPL(lookup_create);
 
-int vfs_mknod(struct inode *dir, struct dentry *dentry, int mode, dev_t dev)
+static int vfs_mknod(struct inode *dir, struct dentry *dentry, int mode, dev_t dev)
 {
 	int error = may_create(dir, dentry, NULL);
 
@@ -2045,22 +2059,19 @@ int vfs_mknod(struct inode *dir, struct 
 	return error;
 }
 
-static int may_mknod(mode_t mode)
+int path_mknod(struct path *dir_path, struct dentry *dentry, int mode,
+	       dev_t dev)
 {
-	switch (mode & S_IFMT) {
-	case S_IFREG:
-	case S_IFCHR:
-	case S_IFBLK:
-	case S_IFIFO:
-	case S_IFSOCK:
-	case 0: /* zero mode translates to S_IFREG */
-		return 0;
-	case S_IFDIR:
-		return -EPERM;
-	default:
-		return -EINVAL;
+	int error = mnt_want_write(dir_path->mnt);
+
+	if (!error) {
+		error = vfs_mknod(dir_path->dentry->d_inode, dentry, mode, dev);
+		mnt_drop_write(dir_path->mnt);
 	}
+
+	return error;
 }
+EXPORT_SYMBOL(path_mknod);
 
 asmlinkage long sys_mknodat(int dfd, const char __user *filename, int mode,
 				unsigned dev)
@@ -2086,26 +2097,22 @@ asmlinkage long sys_mknodat(int dfd, con
 	}
 	if (!IS_POSIXACL(nd.path.dentry->d_inode))
 		mode &= ~current->fs->umask;
-	error = may_mknod(mode);
-	if (error)
-		goto out_dput;
-	error = mnt_want_write(nd.path.mnt);
-	if (error)
-		goto out_dput;
 	switch (mode & S_IFMT) {
-		case 0: case S_IFREG:
-			error = vfs_create(nd.path.dentry->d_inode,dentry,mode,&nd);
-			break;
-		case S_IFCHR: case S_IFBLK:
-			error = vfs_mknod(nd.path.dentry->d_inode,dentry,mode,
-					new_decode_dev(dev));
-			break;
-		case S_IFIFO: case S_IFSOCK:
-			error = vfs_mknod(nd.path.dentry->d_inode,dentry,mode,0);
-			break;
+	case 0: case S_IFREG:
+		error = path_create(&nd.path, dentry, mode, &nd);
+		break;
+	case S_IFCHR: case S_IFBLK:
+		error = path_mknod(&nd.path, dentry, mode, new_decode_dev(dev));
+		break;
+	case S_IFIFO: case S_IFSOCK:
+		error = path_mknod(&nd.path, dentry, mode, 0);
+		break;
+	case S_IFDIR:
+		error = -EPERM;
+		break;
+	default:
+		error = -EINVAL;
 	}
-	mnt_drop_write(nd.path.mnt);
-out_dput:
 	dput(dentry);
 out_unlock:
 	mutex_unlock(&nd.path.dentry->d_inode->i_mutex);
@@ -2972,11 +2979,9 @@ EXPORT_SYMBOL(permission);
 EXPORT_SYMBOL(vfs_permission);
 EXPORT_SYMBOL(file_permission);
 EXPORT_SYMBOL(unlock_rename);
-EXPORT_SYMBOL(vfs_create);
 EXPORT_SYMBOL(vfs_follow_link);
 EXPORT_SYMBOL(vfs_link);
 EXPORT_SYMBOL(vfs_mkdir);
-EXPORT_SYMBOL(vfs_mknod);
 EXPORT_SYMBOL(generic_permission);
 EXPORT_SYMBOL(vfs_readlink);
 EXPORT_SYMBOL(vfs_rename);
Index: linux-2.6/include/linux/fs.h
===================================================================
--- linux-2.6.orig/include/linux/fs.h	2008-05-05 11:29:29.000000000 +0200
+++ linux-2.6/include/linux/fs.h	2008-05-05 11:29:29.000000000 +0200
@@ -1124,9 +1124,9 @@ extern void unlock_super(struct super_bl
  * VFS helper functions..
  */
 extern int vfs_permission(struct nameidata *, int);
-extern int vfs_create(struct inode *, struct dentry *, int, struct nameidata *);
+extern int path_create(struct path *, struct dentry *, int, struct nameidata *);
 extern int vfs_mkdir(struct inode *, struct dentry *, int);
-extern int vfs_mknod(struct inode *, struct dentry *, int, dev_t);
+extern int path_mknod(struct path *, struct dentry *, int, dev_t);
 extern int vfs_symlink(struct inode *, struct dentry *, const char *, int);
 extern int vfs_link(struct dentry *, struct inode *, struct dentry *);
 extern int vfs_rmdir(struct inode *, struct dentry *);
Index: linux-2.6/fs/ecryptfs/inode.c
===================================================================
--- linux-2.6.orig/fs/ecryptfs/inode.c	2008-05-05 11:29:24.000000000 +0200
+++ linux-2.6/fs/ecryptfs/inode.c	2008-05-05 11:29:29.000000000 +0200
@@ -61,23 +61,19 @@ static void unlock_dir(struct dentry *di
  * Returns zero on success; non-zero on error condition
  */
 static int
-ecryptfs_create_underlying_file(struct inode *lower_dir_inode,
+ecryptfs_create_underlying_file(struct dentry *lower_dir_dentry,
 				struct dentry *dentry, int mode,
 				struct nameidata *nd)
 {
 	struct dentry *lower_dentry = ecryptfs_dentry_to_lower(dentry);
-	struct vfsmount *lower_mnt = ecryptfs_dentry_to_lower_mnt(dentry);
-	struct dentry *dentry_save;
-	struct vfsmount *vfsmount_save;
+	struct path save;
 	int rc;
 
-	dentry_save = nd->path.dentry;
-	vfsmount_save = nd->path.mnt;
-	nd->path.dentry = lower_dentry;
-	nd->path.mnt = lower_mnt;
-	rc = vfs_create(lower_dir_inode, lower_dentry, mode, nd);
-	nd->path.dentry = dentry_save;
-	nd->path.mnt = vfsmount_save;
+	save = nd->path;
+	nd->path.mnt = ecryptfs_dentry_to_lower_mnt(dentry);
+	nd->path.dentry = lower_dir_dentry;
+	rc = path_create(&nd->path, lower_dentry, mode, nd);
+	nd->path = save;
 	return rc;
 }
 
@@ -111,7 +107,7 @@ ecryptfs_do_create(struct inode *directo
 		rc = PTR_ERR(lower_dir_dentry);
 		goto out;
 	}
-	rc = ecryptfs_create_underlying_file(lower_dir_dentry->d_inode,
+	rc = ecryptfs_create_underlying_file(lower_dir_dentry,
 					     ecryptfs_dentry, mode, nd);
 	if (rc) {
 		printk(KERN_ERR "%s: Failure to create dentry in lower fs; "
@@ -530,20 +526,21 @@ ecryptfs_mknod(struct inode *dir, struct
 {
 	int rc;
 	struct dentry *lower_dentry;
-	struct dentry *lower_dir_dentry;
+	struct path lower_dir;
 
 	lower_dentry = ecryptfs_dentry_to_lower(dentry);
-	lower_dir_dentry = lock_parent(lower_dentry);
-	rc = vfs_mknod(lower_dir_dentry->d_inode, lower_dentry, mode, dev);
+	lower_dir.mnt = ecryptfs_dentry_to_lower_mnt(dentry);
+	lower_dir.dentry = lock_parent(lower_dentry);
+	rc = path_mknod(&lower_dir, lower_dentry, mode, dev);
 	if (rc || !lower_dentry->d_inode)
 		goto out;
 	rc = ecryptfs_interpose(lower_dentry, dentry, dir->i_sb, 0);
 	if (rc)
 		goto out;
-	fsstack_copy_attr_times(dir, lower_dir_dentry->d_inode);
-	fsstack_copy_inode_size(dir, lower_dir_dentry->d_inode);
+	fsstack_copy_attr_times(dir, lower_dir.dentry->d_inode);
+	fsstack_copy_inode_size(dir, lower_dir.dentry->d_inode);
 out:
-	unlock_dir(lower_dir_dentry);
+	unlock_dir(lower_dir.dentry);
 	if (!dentry->d_inode)
 		d_drop(dentry);
 	return rc;
Index: linux-2.6/ipc/mqueue.c
===================================================================
--- linux-2.6.orig/ipc/mqueue.c	2008-05-05 11:29:20.000000000 +0200
+++ linux-2.6/ipc/mqueue.c	2008-05-05 11:29:29.000000000 +0200
@@ -599,6 +599,7 @@ static struct file *do_create(struct den
 {
 	struct mq_attr attr;
 	struct file *result;
+	struct path dir_path;
 	int ret;
 
 	if (u_attr) {
@@ -616,7 +617,10 @@ static struct file *do_create(struct den
 	ret = mnt_want_write(mqueue_mnt);
 	if (ret)
 		goto out;
-	ret = vfs_create(dir->d_inode, dentry, mode, NULL);
+
+	dir_path.mnt = mqueue_mnt;
+	dir_path.dentry = dir;
+	ret = path_create(&dir_path, dentry, mode, NULL);
 	dentry->d_fsdata = NULL;
 	if (ret)
 		goto out_drop_write;
Index: linux-2.6/net/unix/af_unix.c
===================================================================
--- linux-2.6.orig/net/unix/af_unix.c	2008-05-05 11:29:20.000000000 +0200
+++ linux-2.6/net/unix/af_unix.c	2008-05-05 11:29:29.000000000 +0200
@@ -819,11 +819,7 @@ static int unix_bind(struct socket *sock
 		 */
 		mode = S_IFSOCK |
 		       (SOCK_INODE(sock)->i_mode & ~current->fs->umask);
-		err = mnt_want_write(nd.path.mnt);
-		if (err)
-			goto out_mknod_dput;
-		err = vfs_mknod(nd.path.dentry->d_inode, dentry, mode, 0);
-		mnt_drop_write(nd.path.mnt);
+		err = path_mknod(&nd.path, dentry, mode, 0);
 		if (err)
 			goto out_mknod_dput;
 		mutex_unlock(&nd.path.dentry->d_inode->i_mutex);
Index: linux-2.6/fs/nfsd/vfs.c
===================================================================
--- linux-2.6.orig/fs/nfsd/vfs.c	2008-05-05 11:29:24.000000000 +0200
+++ linux-2.6/fs/nfsd/vfs.c	2008-05-05 11:29:29.000000000 +0200
@@ -130,6 +130,12 @@ out:
 	return err;
 }
 
+static void fh_to_path(struct svc_fh *fhp, struct path *path)
+{
+	path->dentry = fhp->fh_dentry;
+	path->mnt = fhp->fh_export->ex_path.mnt;
+}
+
 __be32
 nfsd_lookup_dentry(struct svc_rqst *rqstp, struct svc_fh *fhp,
 		   const char *name, unsigned int len,
@@ -1184,6 +1190,7 @@ nfsd_create(struct svc_rqst *rqstp, stru
 		char *fname, int flen, struct iattr *iap,
 		int type, dev_t rdev, struct svc_fh *resfhp)
 {
+	struct path	dir_path;
 	struct dentry	*dentry, *dchild = NULL;
 	struct inode	*dirp;
 	__be32		err;
@@ -1259,13 +1266,11 @@ nfsd_create(struct svc_rqst *rqstp, stru
 	if (host_err)
 		goto out_nfserr;
 
-	/*
-	 * Get the dir op function pointer.
-	 */
+	fh_to_path(fhp, &dir_path);
 	err = 0;
 	switch (type) {
 	case S_IFREG:
-		host_err = vfs_create(dirp, dchild, iap->ia_mode, NULL);
+		host_err = path_create(&dir_path, dchild, iap->ia_mode, NULL);
 		break;
 	case S_IFDIR:
 		host_err = vfs_mkdir(dirp, dchild, iap->ia_mode);
@@ -1274,7 +1279,7 @@ nfsd_create(struct svc_rqst *rqstp, stru
 	case S_IFBLK:
 	case S_IFIFO:
 	case S_IFSOCK:
-		host_err = vfs_mknod(dirp, dchild, iap->ia_mode, rdev);
+		host_err = path_mknod(&dir_path, dchild, iap->ia_mode, rdev);
 		break;
 	}
 	if (host_err < 0) {
@@ -1316,6 +1321,7 @@ nfsd_create_v3(struct svc_rqst *rqstp, s
 		struct svc_fh *resfhp, int createmode, u32 *verifier,
 	        int *truncp, int *created)
 {
+	struct path 	dir_path;
 	struct dentry	*dentry, *dchild = NULL;
 	struct inode	*dirp;
 	__be32		err;
@@ -1406,7 +1412,8 @@ nfsd_create_v3(struct svc_rqst *rqstp, s
 		goto out;
 	}
 
-	host_err = vfs_create(dirp, dchild, iap->ia_mode, NULL);
+	fh_to_path(fhp, &dir_path);
+	host_err = path_create(&dir_path, dchild, iap->ia_mode, NULL);
 	if (host_err < 0) {
 		mnt_drop_write(fhp->fh_export->ex_path.mnt);
 		goto out_nfserr;

--

Re: [patch 01/10] vfs: add path_create() and path_mknod()

[Andrew Morton]

On Mon, 05 May 2008 12:16:22 +0200 Miklos Szeredi <miklos@szeredi.hu> wrote:

> R/O bind mounts require operations which modify the filesystem to be
> wrapped in mnt_want_write()/mnt_drop_write().  Create helpers which do
> this, so callers won't need to bother, and more importantly, cannot
> forget!  Call these path_*, analogous to vfs_*.  Since there are no
> callers of vfs_* left, make them static.

ooh, yum.  This appears to address my main complaint about the r-o-bind-mount
stuff: fragility.

> Overall this patchset is just 23 lines in the red, but at the same
> time it fixes several places in nfsd and the whole of ecryptfs, where
> the mnt_want_write/drop_write() calls were missing.

Yeah, like that.

Re: [patch 01/10] vfs: add path_create() and path_mknod()

[Al Viro]

On Mon, May 05, 2008 at 09:12:51PM -0700, Andrew Morton wrote:
> On Mon, 05 May 2008 12:16:22 +0200 Miklos Szeredi <miklos@szeredi.hu> wrote:
> 
> > R/O bind mounts require operations which modify the filesystem to be
> > wrapped in mnt_want_write()/mnt_drop_write().  Create helpers which do
> > this, so callers won't need to bother, and more importantly, cannot
> > forget!  Call these path_*, analogous to vfs_*.  Since there are no
> > callers of vfs_* left, make them static.
> 
> ooh, yum.  This appears to address my main complaint about the r-o-bind-mount
> stuff: fragility.
> 
> > Overall this patchset is just 23 lines in the red, but at the same
> > time it fixes several places in nfsd and the whole of ecryptfs, where
> > the mnt_want_write/drop_write() calls were missing.
> 
> Yeah, like that.

Except that it fixes nothing in nfsd, as we'd already figured out and
"solution" for ecryptfs is more than slightly dubious.  Not that nfsd
one wasn't...

While we are at it, I call bullshit on "make vfs_...() static" and I suspect
that Miklos won't be happy with it once he cares to think through just how
little is going to be guaranteed about those vfsmounts.  As in "not promised
to be attached anywhere", for starters...

Re: [patch 01/10] vfs: add path_create() and path_mknod()

[Andrew Morton]

On Tue, 6 May 2008 05:24:26 +0100 Al Viro <viro@ZenIV.linux.org.uk> wrote:

> On Mon, May 05, 2008 at 09:12:51PM -0700, Andrew Morton wrote:
> > On Mon, 05 May 2008 12:16:22 +0200 Miklos Szeredi <miklos@szeredi.hu> wrote:
> > 
> > > R/O bind mounts require operations which modify the filesystem to be
> > > wrapped in mnt_want_write()/mnt_drop_write().  Create helpers which do
> > > this, so callers won't need to bother, and more importantly, cannot
> > > forget!  Call these path_*, analogous to vfs_*.  Since there are no
> > > callers of vfs_* left, make them static.
> > 
> > ooh, yum.  This appears to address my main complaint about the r-o-bind-mount
> > stuff: fragility.
> > 
> > > Overall this patchset is just 23 lines in the red, but at the same
> > > time it fixes several places in nfsd and the whole of ecryptfs, where
> > > the mnt_want_write/drop_write() calls were missing.
> > 
> > Yeah, like that.
> 
> Except that it fixes nothing in nfsd, as we'd already figured out and
> "solution" for ecryptfs is more than slightly dubious.  Not that nfsd
> one wasn't...

Well OK.  But those open-coded mnt_want_write() calls all over the place
still stink.

Re: [patch 01/10] vfs: add path_create() and path_mknod()

[Miklos Szeredi]

> > > R/O bind mounts require operations which modify the filesystem to be
> > > wrapped in mnt_want_write()/mnt_drop_write().  Create helpers which do
> > > this, so callers won't need to bother, and more importantly, cannot
> > > forget!  Call these path_*, analogous to vfs_*.  Since there are no
> > > callers of vfs_* left, make them static.
> > 
> > ooh, yum.  This appears to address my main complaint about the
> > r-o-bind-mount stuff: fragility.
> > 
> > > Overall this patchset is just 23 lines in the red, but at the same
> > > time it fixes several places in nfsd and the whole of ecryptfs, where
> > > the mnt_want_write/drop_write() calls were missing.
> > 
> > Yeah, like that.
> 
> Except that it fixes nothing in nfsd, as we'd already figured out

Oh, it does!  It fixes at least one race, where nfsd is checking for
r/o mounts but is not keeping the mount from being marked r/o during
the operation.

I also suspect it fixes more than that, that we all just missed when
going over the callers of vfs_*().

> and
> "solution" for ecryptfs is more than slightly dubious.  Not that nfsd
> one wasn't...
> 
> While we are at it, I call bullshit on "make vfs_...() static" and I suspect
> that Miklos won't be happy with it once he cares to think through just how
> little is going to be guaranteed about those vfsmounts.  As in "not promised
> to be attached anywhere", for starters...

How does attachment figure into being marked r/o?  Or into anyting
else, for that matter: no path looked up is ever guaranteed to be
attached, and we've been getting along fine with that.

Al, I've thought this over very well, and it seems to me that you've
just been trying to find excuses.  Which is OK, I guess: that's what
review is about.  But at some point we shouold really just move on.

Miklos

[patch 02/10] vfs: add path_mkdir()

[Miklos Szeredi]

[-- Attachment #1: path_mkdir.patch --]
[-- Type: text/plain, Size: 5466 bytes --]

From: Miklos Szeredi <mszeredi@suse.cz>

Introduce path_mkdir().  Make vfs_mkdir() static.

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---
 fs/ecryptfs/inode.c   |   15 ++++++++-------
 fs/namei.c            |   23 +++++++++++++++--------
 fs/nfsd/nfs4recover.c |    6 +-----
 fs/nfsd/vfs.c         |    2 +-
 include/linux/fs.h    |    2 +-
 5 files changed, 26 insertions(+), 22 deletions(-)

Index: linux-2.6/fs/ecryptfs/inode.c
===================================================================
--- linux-2.6.orig/fs/ecryptfs/inode.c	2008-05-05 11:29:29.000000000 +0200
+++ linux-2.6/fs/ecryptfs/inode.c	2008-05-05 11:29:29.000000000 +0200
@@ -478,21 +478,22 @@ static int ecryptfs_mkdir(struct inode *
 {
 	int rc;
 	struct dentry *lower_dentry;
-	struct dentry *lower_dir_dentry;
+	struct path lower_dir;
 
 	lower_dentry = ecryptfs_dentry_to_lower(dentry);
-	lower_dir_dentry = lock_parent(lower_dentry);
-	rc = vfs_mkdir(lower_dir_dentry->d_inode, lower_dentry, mode);
+	lower_dir.mnt = ecryptfs_dentry_to_lower_mnt(dentry);
+	lower_dir.dentry = lock_parent(lower_dentry);
+	rc = path_mkdir(&lower_dir, lower_dentry, mode);
 	if (rc || !lower_dentry->d_inode)
 		goto out;
 	rc = ecryptfs_interpose(lower_dentry, dentry, dir->i_sb, 0);
 	if (rc)
 		goto out;
-	fsstack_copy_attr_times(dir, lower_dir_dentry->d_inode);
-	fsstack_copy_inode_size(dir, lower_dir_dentry->d_inode);
-	dir->i_nlink = lower_dir_dentry->d_inode->i_nlink;
+	fsstack_copy_attr_times(dir, lower_dir.dentry->d_inode);
+	fsstack_copy_inode_size(dir, lower_dir.dentry->d_inode);
+	dir->i_nlink = lower_dir.dentry->d_inode->i_nlink;
 out:
-	unlock_dir(lower_dir_dentry);
+	unlock_dir(lower_dir.dentry);
 	if (!dentry->d_inode)
 		d_drop(dentry);
 	return rc;
Index: linux-2.6/fs/namei.c
===================================================================
--- linux-2.6.orig/fs/namei.c	2008-05-05 11:29:29.000000000 +0200
+++ linux-2.6/fs/namei.c	2008-05-05 11:29:29.000000000 +0200
@@ -2128,7 +2128,7 @@ asmlinkage long sys_mknod(const char __u
 	return sys_mknodat(AT_FDCWD, filename, mode, dev);
 }
 
-int vfs_mkdir(struct inode *dir, struct dentry *dentry, int mode)
+static int vfs_mkdir(struct inode *dir, struct dentry *dentry, int mode)
 {
 	int error = may_create(dir, dentry, NULL);
 
@@ -2150,6 +2150,19 @@ int vfs_mkdir(struct inode *dir, struct 
 	return error;
 }
 
+int path_mkdir(struct path *dir_path, struct dentry *dentry, int mode)
+{
+	int error = mnt_want_write(dir_path->mnt);
+
+	if (!error) {
+		error = vfs_mkdir(dir_path->dentry->d_inode, dentry, mode);
+		mnt_drop_write(dir_path->mnt);
+	}
+
+	return error;
+}
+EXPORT_SYMBOL(path_mkdir);
+
 asmlinkage long sys_mkdirat(int dfd, const char __user *pathname, int mode)
 {
 	int error = 0;
@@ -2172,12 +2185,7 @@ asmlinkage long sys_mkdirat(int dfd, con
 
 	if (!IS_POSIXACL(nd.path.dentry->d_inode))
 		mode &= ~current->fs->umask;
-	error = mnt_want_write(nd.path.mnt);
-	if (error)
-		goto out_dput;
-	error = vfs_mkdir(nd.path.dentry->d_inode, dentry, mode);
-	mnt_drop_write(nd.path.mnt);
-out_dput:
+	error = path_mkdir(&nd.path, dentry, mode);
 	dput(dentry);
 out_unlock:
 	mutex_unlock(&nd.path.dentry->d_inode->i_mutex);
@@ -2981,7 +2989,6 @@ EXPORT_SYMBOL(file_permission);
 EXPORT_SYMBOL(unlock_rename);
 EXPORT_SYMBOL(vfs_follow_link);
 EXPORT_SYMBOL(vfs_link);
-EXPORT_SYMBOL(vfs_mkdir);
 EXPORT_SYMBOL(generic_permission);
 EXPORT_SYMBOL(vfs_readlink);
 EXPORT_SYMBOL(vfs_rename);
Index: linux-2.6/fs/nfsd/nfs4recover.c
===================================================================
--- linux-2.6.orig/fs/nfsd/nfs4recover.c	2008-05-05 11:29:20.000000000 +0200
+++ linux-2.6/fs/nfsd/nfs4recover.c	2008-05-05 11:29:29.000000000 +0200
@@ -155,11 +155,7 @@ nfsd4_create_clid_dir(struct nfs4_client
 		dprintk("NFSD: nfsd4_create_clid_dir: DIRECTORY EXISTS\n");
 		goto out_put;
 	}
-	status = mnt_want_write(rec_dir.path.mnt);
-	if (status)
-		goto out_put;
-	status = vfs_mkdir(rec_dir.path.dentry->d_inode, dentry, S_IRWXU);
-	mnt_drop_write(rec_dir.path.mnt);
+	status = path_mkdir(&rec_dir.path, dentry, S_IRWXU);
 out_put:
 	dput(dentry);
 out_unlock:
Index: linux-2.6/fs/nfsd/vfs.c
===================================================================
--- linux-2.6.orig/fs/nfsd/vfs.c	2008-05-05 11:29:29.000000000 +0200
+++ linux-2.6/fs/nfsd/vfs.c	2008-05-05 11:29:29.000000000 +0200
@@ -1273,7 +1273,7 @@ nfsd_create(struct svc_rqst *rqstp, stru
 		host_err = path_create(&dir_path, dchild, iap->ia_mode, NULL);
 		break;
 	case S_IFDIR:
-		host_err = vfs_mkdir(dirp, dchild, iap->ia_mode);
+		host_err = path_mkdir(&dir_path, dchild, iap->ia_mode);
 		break;
 	case S_IFCHR:
 	case S_IFBLK:
Index: linux-2.6/include/linux/fs.h
===================================================================
--- linux-2.6.orig/include/linux/fs.h	2008-05-05 11:29:29.000000000 +0200
+++ linux-2.6/include/linux/fs.h	2008-05-05 11:29:29.000000000 +0200
@@ -1125,7 +1125,7 @@ extern void unlock_super(struct super_bl
  */
 extern int vfs_permission(struct nameidata *, int);
 extern int path_create(struct path *, struct dentry *, int, struct nameidata *);
-extern int vfs_mkdir(struct inode *, struct dentry *, int);
+extern int path_mkdir(struct path *, struct dentry *, int);
 extern int path_mknod(struct path *, struct dentry *, int, dev_t);
 extern int vfs_symlink(struct inode *, struct dentry *, const char *, int);
 extern int vfs_link(struct dentry *, struct inode *, struct dentry *);

--

[patch 03/10] vfs: add path_rmdir()

[Miklos Szeredi]

[-- Attachment #1: path_rmdir.patch --]
[-- Type: text/plain, Size: 5868 bytes --]

From: Miklos Szeredi <mszeredi@suse.cz>

Introduce path_rmdir().

The only user of vfs_rmdir() remaining is reiserfs_delete_xattrs().

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---
 fs/ecryptfs/inode.c   |   13 +++++++------
 fs/namei.c            |   23 +++++++++++++++--------
 fs/nfsd/nfs4recover.c |    3 ++-
 fs/nfsd/vfs.c         |    4 +++-
 include/linux/fs.h    |    2 +-
 5 files changed, 28 insertions(+), 17 deletions(-)

Index: linux-2.6/fs/namei.c
===================================================================
--- linux-2.6.orig/fs/namei.c	2008-05-05 11:29:29.000000000 +0200
+++ linux-2.6/fs/namei.c	2008-05-05 11:29:30.000000000 +0200
@@ -2228,7 +2228,7 @@ void dentry_unhash(struct dentry *dentry
 	spin_unlock(&dcache_lock);
 }
 
-int vfs_rmdir(struct inode *dir, struct dentry *dentry)
+static int vfs_rmdir(struct inode *dir, struct dentry *dentry)
 {
 	int error = may_delete(dir, dentry, 1);
 
@@ -2261,6 +2261,19 @@ int vfs_rmdir(struct inode *dir, struct 
 	return error;
 }
 
+int path_rmdir(struct path *dir_path, struct dentry *dentry)
+{
+	int error = mnt_want_write(dir_path->mnt);
+
+	if (!error) {
+		error = vfs_rmdir(dir_path->dentry->d_inode, dentry);
+		mnt_drop_write(dir_path->mnt);
+	}
+
+	return error;
+}
+EXPORT_SYMBOL(path_rmdir);
+
 static long do_rmdir(int dfd, const char __user *pathname)
 {
 	int error = 0;
@@ -2292,12 +2305,7 @@ static long do_rmdir(int dfd, const char
 	error = PTR_ERR(dentry);
 	if (IS_ERR(dentry))
 		goto exit2;
-	error = mnt_want_write(nd.path.mnt);
-	if (error)
-		goto exit3;
-	error = vfs_rmdir(nd.path.dentry->d_inode, dentry);
-	mnt_drop_write(nd.path.mnt);
-exit3:
+	error = path_rmdir(&nd.path, dentry);
 	dput(dentry);
 exit2:
 	mutex_unlock(&nd.path.dentry->d_inode->i_mutex);
@@ -2992,7 +3000,6 @@ EXPORT_SYMBOL(vfs_link);
 EXPORT_SYMBOL(generic_permission);
 EXPORT_SYMBOL(vfs_readlink);
 EXPORT_SYMBOL(vfs_rename);
-EXPORT_SYMBOL(vfs_rmdir);
 EXPORT_SYMBOL(vfs_symlink);
 EXPORT_SYMBOL(vfs_unlink);
 EXPORT_SYMBOL(dentry_unhash);
Index: linux-2.6/fs/ecryptfs/inode.c
===================================================================
--- linux-2.6.orig/fs/ecryptfs/inode.c	2008-05-05 11:29:29.000000000 +0200
+++ linux-2.6/fs/ecryptfs/inode.c	2008-05-05 11:29:30.000000000 +0200
@@ -502,20 +502,21 @@ out:
 static int ecryptfs_rmdir(struct inode *dir, struct dentry *dentry)
 {
 	struct dentry *lower_dentry;
-	struct dentry *lower_dir_dentry;
+	struct path lower_dir;
 	int rc;
 
 	lower_dentry = ecryptfs_dentry_to_lower(dentry);
 	dget(dentry);
-	lower_dir_dentry = lock_parent(lower_dentry);
+	lower_dir.mnt = ecryptfs_dentry_to_lower_mnt(dentry);
+	lower_dir.dentry = lock_parent(lower_dentry);
 	dget(lower_dentry);
-	rc = vfs_rmdir(lower_dir_dentry->d_inode, lower_dentry);
+	rc = path_rmdir(&lower_dir, lower_dentry);
 	dput(lower_dentry);
 	if (!rc)
 		d_delete(lower_dentry);
-	fsstack_copy_attr_times(dir, lower_dir_dentry->d_inode);
-	dir->i_nlink = lower_dir_dentry->d_inode->i_nlink;
-	unlock_dir(lower_dir_dentry);
+	fsstack_copy_attr_times(dir, lower_dir.dentry->d_inode);
+	dir->i_nlink = lower_dir.dentry->d_inode->i_nlink;
+	unlock_dir(lower_dir.dentry);
 	if (!rc)
 		d_drop(dentry);
 	dput(dentry);
Index: linux-2.6/fs/nfsd/nfs4recover.c
===================================================================
--- linux-2.6.orig/fs/nfsd/nfs4recover.c	2008-05-05 11:29:29.000000000 +0200
+++ linux-2.6/fs/nfsd/nfs4recover.c	2008-05-05 11:29:30.000000000 +0200
@@ -267,6 +267,7 @@ nfsd4_remove_clid_file(struct dentry *di
 static int
 nfsd4_clear_clid_dir(struct dentry *dir, struct dentry *dentry)
 {
+	struct path dir_path = { .dentry = dir, .mnt = rec_dir.path.mnt };
 	int status;
 
 	/* For now this directory should already be empty, but we empty it of
@@ -274,7 +275,7 @@ nfsd4_clear_clid_dir(struct dentry *dir,
 	 * a kernel from the future.... */
 	nfsd4_list_rec_dir(dentry, nfsd4_remove_clid_file);
 	mutex_lock_nested(&dir->d_inode->i_mutex, I_MUTEX_PARENT);
-	status = vfs_rmdir(dir->d_inode, dentry);
+	status = path_rmdir(&dir_path, dentry);
 	mutex_unlock(&dir->d_inode->i_mutex);
 	return status;
 }
Index: linux-2.6/fs/nfsd/vfs.c
===================================================================
--- linux-2.6.orig/fs/nfsd/vfs.c	2008-05-05 11:29:29.000000000 +0200
+++ linux-2.6/fs/nfsd/vfs.c	2008-05-05 11:29:30.000000000 +0200
@@ -1764,6 +1764,7 @@ __be32
 nfsd_unlink(struct svc_rqst *rqstp, struct svc_fh *fhp, int type,
 				char *fname, int flen)
 {
+	struct path 	dir_path;
 	struct dentry	*dentry, *rdentry;
 	struct inode	*dirp;
 	__be32		err;
@@ -1798,6 +1799,7 @@ nfsd_unlink(struct svc_rqst *rqstp, stru
 	if (host_err)
 		goto out_nfserr;
 
+	fh_to_path(fhp, &dir_path);
 	if (type != S_IFDIR) { /* It's UNLINK */
 #ifdef MSNFS
 		if ((fhp->fh_export->ex_flags & NFSEXP_MSNFS) &&
@@ -1807,7 +1809,7 @@ nfsd_unlink(struct svc_rqst *rqstp, stru
 #endif
 		host_err = vfs_unlink(dirp, rdentry);
 	} else { /* It's RMDIR */
-		host_err = vfs_rmdir(dirp, rdentry);
+		host_err = path_rmdir(&dir_path, rdentry);
 	}
 
 	dput(rdentry);
Index: linux-2.6/include/linux/fs.h
===================================================================
--- linux-2.6.orig/include/linux/fs.h	2008-05-05 11:29:29.000000000 +0200
+++ linux-2.6/include/linux/fs.h	2008-05-05 11:29:30.000000000 +0200
@@ -1129,7 +1129,7 @@ extern int path_mkdir(struct path *, str
 extern int path_mknod(struct path *, struct dentry *, int, dev_t);
 extern int vfs_symlink(struct inode *, struct dentry *, const char *, int);
 extern int vfs_link(struct dentry *, struct inode *, struct dentry *);
-extern int vfs_rmdir(struct inode *, struct dentry *);
+extern int path_rmdir(struct path *, struct dentry *);
 extern int vfs_unlink(struct inode *, struct dentry *);
 extern int vfs_rename(struct inode *, struct dentry *, struct inode *, struct dentry *);
 

--

[patch 04/10] vfs: add path_unlink()

[Miklos Szeredi]

[-- Attachment #1: path_unlink.patch --]
[-- Type: text/plain, Size: 7192 bytes --]

From: Miklos Szeredi <mszeredi@suse.cz>

Introduce path_unlink().  Make vfs_unlink() static.

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---
 fs/ecryptfs/inode.c   |   12 ++++++------
 fs/namei.c            |   22 +++++++++++++++-------
 fs/nfsd/nfs4recover.c |    3 ++-
 fs/nfsd/vfs.c         |   12 ++----------
 include/linux/fs.h    |    2 +-
 ipc/mqueue.c          |   10 +++++-----
 6 files changed, 31 insertions(+), 30 deletions(-)

Index: linux-2.6/fs/ecryptfs/inode.c
===================================================================
--- linux-2.6.orig/fs/ecryptfs/inode.c	2008-05-05 11:29:30.000000000 +0200
+++ linux-2.6/fs/ecryptfs/inode.c	2008-05-05 11:29:31.000000000 +0200
@@ -415,22 +415,22 @@ static int ecryptfs_unlink(struct inode 
 {
 	int rc = 0;
 	struct dentry *lower_dentry = ecryptfs_dentry_to_lower(dentry);
-	struct inode *lower_dir_inode = ecryptfs_inode_to_lower(dir);
-	struct dentry *lower_dir_dentry;
+	struct path lower_dir;
 
-	lower_dir_dentry = lock_parent(lower_dentry);
-	rc = vfs_unlink(lower_dir_inode, lower_dentry);
+	lower_dir.mnt = ecryptfs_dentry_to_lower_mnt(dentry);
+	lower_dir.dentry = lock_parent(lower_dentry);
+	rc = path_unlink(&lower_dir, lower_dentry);
 	if (rc) {
 		printk(KERN_ERR "Error in vfs_unlink; rc = [%d]\n", rc);
 		goto out_unlock;
 	}
-	fsstack_copy_attr_times(dir, lower_dir_inode);
+	fsstack_copy_attr_times(dir, lower_dir.dentry->d_inode);
 	dentry->d_inode->i_nlink =
 		ecryptfs_inode_to_lower(dentry->d_inode)->i_nlink;
 	dentry->d_inode->i_ctime = dir->i_ctime;
 	d_drop(dentry);
 out_unlock:
-	unlock_dir(lower_dir_dentry);
+	unlock_dir(lower_dir.dentry);
 	return rc;
 }
 
Index: linux-2.6/fs/namei.c
===================================================================
--- linux-2.6.orig/fs/namei.c	2008-05-05 11:29:30.000000000 +0200
+++ linux-2.6/fs/namei.c	2008-05-05 11:29:31.000000000 +0200
@@ -2321,7 +2321,7 @@ asmlinkage long sys_rmdir(const char __u
 	return do_rmdir(AT_FDCWD, pathname);
 }
 
-int vfs_unlink(struct inode *dir, struct dentry *dentry)
+static int vfs_unlink(struct inode *dir, struct dentry *dentry)
 {
 	int error = may_delete(dir, dentry, 0);
 
@@ -2352,6 +2352,19 @@ int vfs_unlink(struct inode *dir, struct
 	return error;
 }
 
+int path_unlink(struct path *dir_path, struct dentry *dentry)
+{
+	int error = mnt_want_write(dir_path->mnt);
+
+	if (!error) {
+		error = vfs_unlink(dir_path->dentry->d_inode, dentry);
+		mnt_drop_write(dir_path->mnt);
+	}
+
+	return error;
+}
+EXPORT_SYMBOL(path_unlink);
+
 /*
  * Make sure that the actual truncation of the file will occur outside its
  * directory's i_mutex.  Truncate can take a long time if there is a lot of
@@ -2386,11 +2399,7 @@ static long do_unlinkat(int dfd, const c
 		inode = dentry->d_inode;
 		if (inode)
 			atomic_inc(&inode->i_count);
-		error = mnt_want_write(nd.path.mnt);
-		if (error)
-			goto exit2;
-		error = vfs_unlink(nd.path.dentry->d_inode, dentry);
-		mnt_drop_write(nd.path.mnt);
+		error = path_unlink(&nd.path, dentry);
 	exit2:
 		dput(dentry);
 	}
@@ -3001,6 +3010,5 @@ EXPORT_SYMBOL(generic_permission);
 EXPORT_SYMBOL(vfs_readlink);
 EXPORT_SYMBOL(vfs_rename);
 EXPORT_SYMBOL(vfs_symlink);
-EXPORT_SYMBOL(vfs_unlink);
 EXPORT_SYMBOL(dentry_unhash);
 EXPORT_SYMBOL(generic_readlink);
Index: linux-2.6/fs/nfsd/nfs4recover.c
===================================================================
--- linux-2.6.orig/fs/nfsd/nfs4recover.c	2008-05-05 11:29:30.000000000 +0200
+++ linux-2.6/fs/nfsd/nfs4recover.c	2008-05-05 11:29:31.000000000 +0200
@@ -252,6 +252,7 @@ out:
 static int
 nfsd4_remove_clid_file(struct dentry *dir, struct dentry *dentry)
 {
+	struct path dir_path = { .dentry = dir, .mnt = rec_dir.path.mnt };
 	int status;
 
 	if (!S_ISREG(dir->d_inode->i_mode)) {
@@ -259,7 +260,7 @@ nfsd4_remove_clid_file(struct dentry *di
 		return -EINVAL;
 	}
 	mutex_lock_nested(&dir->d_inode->i_mutex, I_MUTEX_PARENT);
-	status = vfs_unlink(dir->d_inode, dentry);
+	status = path_unlink(&dir_path, dentry);
 	mutex_unlock(&dir->d_inode->i_mutex);
 	return status;
 }
Index: linux-2.6/fs/nfsd/vfs.c
===================================================================
--- linux-2.6.orig/fs/nfsd/vfs.c	2008-05-05 11:29:30.000000000 +0200
+++ linux-2.6/fs/nfsd/vfs.c	2008-05-05 11:29:31.000000000 +0200
@@ -1766,7 +1766,6 @@ nfsd_unlink(struct svc_rqst *rqstp, stru
 {
 	struct path 	dir_path;
 	struct dentry	*dentry, *rdentry;
-	struct inode	*dirp;
 	__be32		err;
 	int		host_err;
 
@@ -1779,7 +1778,6 @@ nfsd_unlink(struct svc_rqst *rqstp, stru
 
 	fh_lock_nested(fhp, I_MUTEX_PARENT);
 	dentry = fhp->fh_dentry;
-	dirp = dentry->d_inode;
 
 	rdentry = lookup_one_len(fname, dentry, flen);
 	host_err = PTR_ERR(rdentry);
@@ -1795,10 +1793,6 @@ nfsd_unlink(struct svc_rqst *rqstp, stru
 	if (!type)
 		type = rdentry->d_inode->i_mode & S_IFMT;
 
-	host_err = mnt_want_write(fhp->fh_export->ex_path.mnt);
-	if (host_err)
-		goto out_nfserr;
-
 	fh_to_path(fhp, &dir_path);
 	if (type != S_IFDIR) { /* It's UNLINK */
 #ifdef MSNFS
@@ -1807,7 +1801,7 @@ nfsd_unlink(struct svc_rqst *rqstp, stru
 			host_err = -EPERM;
 		} else
 #endif
-		host_err = vfs_unlink(dirp, rdentry);
+		host_err = path_unlink(&dir_path, rdentry);
 	} else { /* It's RMDIR */
 		host_err = path_rmdir(&dir_path, rdentry);
 	}
@@ -1815,12 +1809,10 @@ nfsd_unlink(struct svc_rqst *rqstp, stru
 	dput(rdentry);
 
 	if (host_err)
-		goto out_drop;
+		goto out_nfserr;
 	if (EX_ISSYNC(fhp->fh_export))
 		host_err = nfsd_sync_dir(dentry);
 
-out_drop:
-	mnt_drop_write(fhp->fh_export->ex_path.mnt);
 out_nfserr:
 	err = nfserrno(host_err);
 out:
Index: linux-2.6/include/linux/fs.h
===================================================================
--- linux-2.6.orig/include/linux/fs.h	2008-05-05 11:29:30.000000000 +0200
+++ linux-2.6/include/linux/fs.h	2008-05-05 11:29:31.000000000 +0200
@@ -1130,7 +1130,7 @@ extern int path_mknod(struct path *, str
 extern int vfs_symlink(struct inode *, struct dentry *, const char *, int);
 extern int vfs_link(struct dentry *, struct inode *, struct dentry *);
 extern int path_rmdir(struct path *, struct dentry *);
-extern int vfs_unlink(struct inode *, struct dentry *);
+extern int path_unlink(struct path *, struct dentry *);
 extern int vfs_rename(struct inode *, struct dentry *, struct inode *, struct dentry *);
 
 /*
Index: linux-2.6/ipc/mqueue.c
===================================================================
--- linux-2.6.orig/ipc/mqueue.c	2008-05-05 11:29:29.000000000 +0200
+++ linux-2.6/ipc/mqueue.c	2008-05-05 11:29:31.000000000 +0200
@@ -736,6 +736,7 @@ asmlinkage long sys_mq_unlink(const char
 	char *name;
 	struct dentry *dentry;
 	struct inode *inode = NULL;
+	struct path dir_path;
 
 	name = getname(u_name);
 	if (IS_ERR(name))
@@ -757,11 +758,10 @@ asmlinkage long sys_mq_unlink(const char
 	inode = dentry->d_inode;
 	if (inode)
 		atomic_inc(&inode->i_count);
-	err = mnt_want_write(mqueue_mnt);
-	if (err)
-		goto out_err;
-	err = vfs_unlink(dentry->d_parent->d_inode, dentry);
-	mnt_drop_write(mqueue_mnt);
+
+	dir_path.mnt = mqueue_mnt;
+	dir_path.dentry = dentry->d_parent;
+	err = path_unlink(&dir_path, dentry);
 out_err:
 	dput(dentry);
 

--

[patch 05/10] vfs: add path_symlink()

[Miklos Szeredi]

[-- Attachment #1: path_symlink.patch --]
[-- Type: text/plain, Size: 6302 bytes --]

From: Miklos Szeredi <mszeredi@suse.cz>

Introduce path_symlink().  Make vfs_symlink() static.

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---
 fs/ecryptfs/inode.c |   14 +++++++-------
 fs/namei.c          |   26 ++++++++++++++++++--------
 fs/nfsd/vfs.c       |   12 ++++--------
 include/linux/fs.h  |    2 +-
 4 files changed, 30 insertions(+), 24 deletions(-)

Index: linux-2.6/fs/ecryptfs/inode.c
===================================================================
--- linux-2.6.orig/fs/ecryptfs/inode.c	2008-05-05 11:29:31.000000000 +0200
+++ linux-2.6/fs/ecryptfs/inode.c	2008-05-05 11:29:31.000000000 +0200
@@ -439,7 +439,7 @@ static int ecryptfs_symlink(struct inode
 {
 	int rc;
 	struct dentry *lower_dentry;
-	struct dentry *lower_dir_dentry;
+	struct path lower_dir;
 	umode_t mode;
 	char *encoded_symname;
 	int encoded_symlen;
@@ -447,7 +447,8 @@ static int ecryptfs_symlink(struct inode
 
 	lower_dentry = ecryptfs_dentry_to_lower(dentry);
 	dget(lower_dentry);
-	lower_dir_dentry = lock_parent(lower_dentry);
+	lower_dir.mnt = ecryptfs_dentry_to_lower_mnt(dentry);
+	lower_dir.dentry = lock_parent(lower_dentry);
 	mode = S_IALLUGO;
 	encoded_symlen = ecryptfs_encode_filename(crypt_stat, symname,
 						  strlen(symname),
@@ -456,18 +457,17 @@ static int ecryptfs_symlink(struct inode
 		rc = encoded_symlen;
 		goto out_lock;
 	}
-	rc = vfs_symlink(lower_dir_dentry->d_inode, lower_dentry,
-			 encoded_symname, mode);
+	rc = path_symlink(&lower_dir, lower_dentry, encoded_symname, mode);
 	kfree(encoded_symname);
 	if (rc || !lower_dentry->d_inode)
 		goto out_lock;
 	rc = ecryptfs_interpose(lower_dentry, dentry, dir->i_sb, 0);
 	if (rc)
 		goto out_lock;
-	fsstack_copy_attr_times(dir, lower_dir_dentry->d_inode);
-	fsstack_copy_inode_size(dir, lower_dir_dentry->d_inode);
+	fsstack_copy_attr_times(dir, lower_dir.dentry->d_inode);
+	fsstack_copy_inode_size(dir, lower_dir.dentry->d_inode);
 out_lock:
-	unlock_dir(lower_dir_dentry);
+	unlock_dir(lower_dir.dentry);
 	dput(lower_dentry);
 	if (!dentry->d_inode)
 		d_drop(dentry);
Index: linux-2.6/fs/namei.c
===================================================================
--- linux-2.6.orig/fs/namei.c	2008-05-05 11:29:31.000000000 +0200
+++ linux-2.6/fs/namei.c	2008-05-05 11:29:31.000000000 +0200
@@ -2434,7 +2434,7 @@ asmlinkage long sys_unlink(const char __
 	return do_unlinkat(AT_FDCWD, pathname);
 }
 
-int vfs_symlink(struct inode *dir, struct dentry *dentry, const char *oldname, int mode)
+static int vfs_symlink(struct inode *dir, struct dentry *dentry, const char *oldname, int mode)
 {
 	int error = may_create(dir, dentry, NULL);
 
@@ -2455,6 +2455,22 @@ int vfs_symlink(struct inode *dir, struc
 	return error;
 }
 
+int path_symlink(struct path *dir_path, struct dentry *dentry,
+		 const char *oldname, int mode)
+{
+	int error = mnt_want_write(dir_path->mnt);
+
+	if (!error) {
+		struct inode *dir = dir_path->dentry->d_inode;
+
+		error = vfs_symlink(dir, dentry, oldname, mode);
+		mnt_drop_write(dir_path->mnt);
+	}
+
+	return error;
+}
+EXPORT_SYMBOL(path_symlink);
+
 asmlinkage long sys_symlinkat(const char __user *oldname,
 			      int newdfd, const char __user *newname)
 {
@@ -2480,12 +2496,7 @@ asmlinkage long sys_symlinkat(const char
 	if (IS_ERR(dentry))
 		goto out_unlock;
 
-	error = mnt_want_write(nd.path.mnt);
-	if (error)
-		goto out_dput;
-	error = vfs_symlink(nd.path.dentry->d_inode, dentry, from, S_IALLUGO);
-	mnt_drop_write(nd.path.mnt);
-out_dput:
+	error = path_symlink(&nd.path, dentry, from, S_IALLUGO);
 	dput(dentry);
 out_unlock:
 	mutex_unlock(&nd.path.dentry->d_inode->i_mutex);
@@ -3009,6 +3020,5 @@ EXPORT_SYMBOL(vfs_link);
 EXPORT_SYMBOL(generic_permission);
 EXPORT_SYMBOL(vfs_readlink);
 EXPORT_SYMBOL(vfs_rename);
-EXPORT_SYMBOL(vfs_symlink);
 EXPORT_SYMBOL(dentry_unhash);
 EXPORT_SYMBOL(generic_readlink);
Index: linux-2.6/fs/nfsd/vfs.c
===================================================================
--- linux-2.6.orig/fs/nfsd/vfs.c	2008-05-05 11:29:31.000000000 +0200
+++ linux-2.6/fs/nfsd/vfs.c	2008-05-05 11:29:31.000000000 +0200
@@ -1518,6 +1518,7 @@ nfsd_symlink(struct svc_rqst *rqstp, str
 				struct svc_fh *resfhp,
 				struct iattr *iap)
 {
+	struct path 	dir_path;
 	struct dentry	*dentry, *dnew;
 	__be32		err, cerr;
 	int		host_err;
@@ -1545,10 +1546,7 @@ nfsd_symlink(struct svc_rqst *rqstp, str
 	if (iap && (iap->ia_valid & ATTR_MODE))
 		mode = iap->ia_mode & S_IALLUGO;
 
-	host_err = mnt_want_write(fhp->fh_export->ex_path.mnt);
-	if (host_err)
-		goto out_nfserr;
-
+	fh_to_path(fhp, &dir_path);
 	if (unlikely(path[plen] != 0)) {
 		char *path_alloced = kmalloc(plen+1, GFP_KERNEL);
 		if (path_alloced == NULL)
@@ -1556,11 +1554,11 @@ nfsd_symlink(struct svc_rqst *rqstp, str
 		else {
 			strncpy(path_alloced, path, plen);
 			path_alloced[plen] = 0;
-			host_err = vfs_symlink(dentry->d_inode, dnew, path_alloced, mode);
+			host_err = path_symlink(&dir_path, dnew, path_alloced, mode);
 			kfree(path_alloced);
 		}
 	} else
-		host_err = vfs_symlink(dentry->d_inode, dnew, path, mode);
+		host_err = path_symlink(&dir_path, dnew, path, mode);
 
 	if (!host_err) {
 		if (EX_ISSYNC(fhp->fh_export))
@@ -1569,8 +1567,6 @@ nfsd_symlink(struct svc_rqst *rqstp, str
 	err = nfserrno(host_err);
 	fh_unlock(fhp);
 
-	mnt_drop_write(fhp->fh_export->ex_path.mnt);
-
 	cerr = fh_compose(resfhp, fhp->fh_export, dnew, fhp);
 	dput(dnew);
 	if (err==0) err = cerr;
Index: linux-2.6/include/linux/fs.h
===================================================================
--- linux-2.6.orig/include/linux/fs.h	2008-05-05 11:29:31.000000000 +0200
+++ linux-2.6/include/linux/fs.h	2008-05-05 11:29:31.000000000 +0200
@@ -1127,7 +1127,7 @@ extern int vfs_permission(struct nameida
 extern int path_create(struct path *, struct dentry *, int, struct nameidata *);
 extern int path_mkdir(struct path *, struct dentry *, int);
 extern int path_mknod(struct path *, struct dentry *, int, dev_t);
-extern int vfs_symlink(struct inode *, struct dentry *, const char *, int);
+extern int path_symlink(struct path *, struct dentry *, const char *, int);
 extern int vfs_link(struct dentry *, struct inode *, struct dentry *);
 extern int path_rmdir(struct path *, struct dentry *);
 extern int path_unlink(struct path *, struct dentry *);

--

[patch 06/10] vfs: add path_link()

[Miklos Szeredi]

[-- Attachment #1: path_link.patch --]
[-- Type: text/plain, Size: 5971 bytes --]

From: Miklos Szeredi <mszeredi@suse.cz>

Introduce path_link().  Make vfs_link() static.

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---
 fs/ecryptfs/inode.c |   10 +++++-----
 fs/namei.c          |   26 ++++++++++++++++++--------
 fs/nfsd/vfs.c       |   14 ++++----------
 include/linux/fs.h  |    2 +-
 4 files changed, 28 insertions(+), 24 deletions(-)

Index: linux-2.6/fs/ecryptfs/inode.c
===================================================================
--- linux-2.6.orig/fs/ecryptfs/inode.c	2008-05-05 11:29:31.000000000 +0200
+++ linux-2.6/fs/ecryptfs/inode.c	2008-05-05 11:29:32.000000000 +0200
@@ -379,7 +379,7 @@ static int ecryptfs_link(struct dentry *
 {
 	struct dentry *lower_old_dentry;
 	struct dentry *lower_new_dentry;
-	struct dentry *lower_dir_dentry;
+	struct path lower_dir;
 	u64 file_size_save;
 	int rc;
 
@@ -388,9 +388,9 @@ static int ecryptfs_link(struct dentry *
 	lower_new_dentry = ecryptfs_dentry_to_lower(new_dentry);
 	dget(lower_old_dentry);
 	dget(lower_new_dentry);
-	lower_dir_dentry = lock_parent(lower_new_dentry);
-	rc = vfs_link(lower_old_dentry, lower_dir_dentry->d_inode,
-		      lower_new_dentry);
+	lower_dir.mnt = ecryptfs_dentry_to_lower_mnt(new_dentry);
+	lower_dir.dentry = lock_parent(lower_new_dentry);
+	rc = path_link(lower_old_dentry, &lower_dir, lower_new_dentry);
 	if (rc || !lower_new_dentry->d_inode)
 		goto out_lock;
 	rc = ecryptfs_interpose(lower_new_dentry, new_dentry, dir->i_sb, 0);
@@ -402,7 +402,7 @@ static int ecryptfs_link(struct dentry *
 		ecryptfs_inode_to_lower(old_dentry->d_inode)->i_nlink;
 	i_size_write(new_dentry->d_inode, file_size_save);
 out_lock:
-	unlock_dir(lower_dir_dentry);
+	unlock_dir(lower_dir.dentry);
 	dput(lower_new_dentry);
 	dput(lower_old_dentry);
 	d_drop(lower_old_dentry);
Index: linux-2.6/fs/namei.c
===================================================================
--- linux-2.6.orig/fs/namei.c	2008-05-05 11:29:31.000000000 +0200
+++ linux-2.6/fs/namei.c	2008-05-05 11:29:32.000000000 +0200
@@ -2513,7 +2513,7 @@ asmlinkage long sys_symlink(const char _
 	return sys_symlinkat(oldname, AT_FDCWD, newname);
 }
 
-int vfs_link(struct dentry *old_dentry, struct inode *dir, struct dentry *new_dentry)
+static int vfs_link(struct dentry *old_dentry, struct inode *dir, struct dentry *new_dentry)
 {
 	struct inode *inode = old_dentry->d_inode;
 	int error;
@@ -2551,6 +2551,22 @@ int vfs_link(struct dentry *old_dentry, 
 	return error;
 }
 
+int path_link(struct dentry *old_dentry, struct path *dir_path,
+	      struct dentry *new_dentry)
+{
+	int error = mnt_want_write(dir_path->mnt);
+
+	if (!error) {
+		struct inode *dir = dir_path->dentry->d_inode;
+
+		error = vfs_link(old_dentry, dir, new_dentry);
+		mnt_drop_write(dir_path->mnt);
+	}
+
+	return error;
+}
+EXPORT_SYMBOL(path_link);
+
 /*
  * Hardlinks are often used in delicate situations.  We avoid
  * security-related surprises by not following symlinks on the
@@ -2591,12 +2607,7 @@ asmlinkage long sys_linkat(int olddfd, c
 	error = PTR_ERR(new_dentry);
 	if (IS_ERR(new_dentry))
 		goto out_unlock;
-	error = mnt_want_write(nd.path.mnt);
-	if (error)
-		goto out_dput;
-	error = vfs_link(old_nd.path.dentry, nd.path.dentry->d_inode, new_dentry);
-	mnt_drop_write(nd.path.mnt);
-out_dput:
+	error = path_link(old_nd.path.dentry, &nd.path, new_dentry);
 	dput(new_dentry);
 out_unlock:
 	mutex_unlock(&nd.path.dentry->d_inode->i_mutex);
@@ -3016,7 +3027,6 @@ EXPORT_SYMBOL(vfs_permission);
 EXPORT_SYMBOL(file_permission);
 EXPORT_SYMBOL(unlock_rename);
 EXPORT_SYMBOL(vfs_follow_link);
-EXPORT_SYMBOL(vfs_link);
 EXPORT_SYMBOL(generic_permission);
 EXPORT_SYMBOL(vfs_readlink);
 EXPORT_SYMBOL(vfs_rename);
Index: linux-2.6/fs/nfsd/vfs.c
===================================================================
--- linux-2.6.orig/fs/nfsd/vfs.c	2008-05-05 11:29:31.000000000 +0200
+++ linux-2.6/fs/nfsd/vfs.c	2008-05-05 11:29:32.000000000 +0200
@@ -1586,8 +1586,9 @@ __be32
 nfsd_link(struct svc_rqst *rqstp, struct svc_fh *ffhp,
 				char *name, int len, struct svc_fh *tfhp)
 {
+	struct path 	dir_path;
 	struct dentry	*ddir, *dnew, *dold;
-	struct inode	*dirp, *dest;
+	struct inode	*dest;
 	__be32		err;
 	int		host_err;
 
@@ -1607,7 +1608,6 @@ nfsd_link(struct svc_rqst *rqstp, struct
 
 	fh_lock_nested(ffhp, I_MUTEX_PARENT);
 	ddir = ffhp->fh_dentry;
-	dirp = ddir->d_inode;
 
 	dnew = lookup_one_len(name, ddir, len);
 	host_err = PTR_ERR(dnew);
@@ -1617,12 +1617,8 @@ nfsd_link(struct svc_rqst *rqstp, struct
 	dold = tfhp->fh_dentry;
 	dest = dold->d_inode;
 
-	host_err = mnt_want_write(tfhp->fh_export->ex_path.mnt);
-	if (host_err) {
-		err = nfserrno(host_err);
-		goto out_dput;
-	}
-	host_err = vfs_link(dold, dirp, dnew);
+	fh_to_path(ffhp, &dir_path);
+	host_err = path_link(dold, &dir_path, dnew);
 	if (!host_err) {
 		if (EX_ISSYNC(ffhp->fh_export)) {
 			err = nfserrno(nfsd_sync_dir(ddir));
@@ -1635,8 +1631,6 @@ nfsd_link(struct svc_rqst *rqstp, struct
 		else
 			err = nfserrno(host_err);
 	}
-	mnt_drop_write(tfhp->fh_export->ex_path.mnt);
-out_dput:
 	dput(dnew);
 out_unlock:
 	fh_unlock(ffhp);
Index: linux-2.6/include/linux/fs.h
===================================================================
--- linux-2.6.orig/include/linux/fs.h	2008-05-05 11:29:31.000000000 +0200
+++ linux-2.6/include/linux/fs.h	2008-05-05 11:29:32.000000000 +0200
@@ -1128,7 +1128,7 @@ extern int path_create(struct path *, st
 extern int path_mkdir(struct path *, struct dentry *, int);
 extern int path_mknod(struct path *, struct dentry *, int, dev_t);
 extern int path_symlink(struct path *, struct dentry *, const char *, int);
-extern int vfs_link(struct dentry *, struct inode *, struct dentry *);
+extern int path_link(struct dentry *, struct path *, struct dentry *);
 extern int path_rmdir(struct path *, struct dentry *);
 extern int path_unlink(struct path *, struct dentry *);
 extern int vfs_rename(struct inode *, struct dentry *, struct inode *, struct dentry *);

--

[patch 07/10] vfs: add path_rename()

[Miklos Szeredi]

[-- Attachment #1: path_rename.patch --]
[-- Type: text/plain, Size: 6710 bytes --]

From: Miklos Szeredi <mszeredi@suse.cz>

Introduce path_rename().  Make vfs_rename() static.

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---
 fs/ecryptfs/inode.c |   22 ++++++++++++----------
 fs/namei.c          |   33 ++++++++++++++++++++++++---------
 fs/nfsd/vfs.c       |   19 +++++--------------
 include/linux/fs.h  |    3 ++-
 4 files changed, 43 insertions(+), 34 deletions(-)

Index: linux-2.6/fs/ecryptfs/inode.c
===================================================================
--- linux-2.6.orig/fs/ecryptfs/inode.c	2008-05-05 11:29:32.000000000 +0200
+++ linux-2.6/fs/ecryptfs/inode.c	2008-05-05 11:29:32.000000000 +0200
@@ -555,25 +555,27 @@ ecryptfs_rename(struct inode *old_dir, s
 	int rc;
 	struct dentry *lower_old_dentry;
 	struct dentry *lower_new_dentry;
-	struct dentry *lower_old_dir_dentry;
-	struct dentry *lower_new_dir_dentry;
+	struct path lower_old_dir;
+	struct path lower_new_dir;
 
 	lower_old_dentry = ecryptfs_dentry_to_lower(old_dentry);
 	lower_new_dentry = ecryptfs_dentry_to_lower(new_dentry);
 	dget(lower_old_dentry);
 	dget(lower_new_dentry);
-	lower_old_dir_dentry = dget_parent(lower_old_dentry);
-	lower_new_dir_dentry = dget_parent(lower_new_dentry);
-	lock_rename(lower_old_dir_dentry, lower_new_dir_dentry);
-	rc = vfs_rename(lower_old_dir_dentry->d_inode, lower_old_dentry,
-			lower_new_dir_dentry->d_inode, lower_new_dentry);
+	lower_old_dir.mnt = ecryptfs_dentry_to_lower_mnt(old_dentry);
+	lower_old_dir.dentry = dget_parent(lower_old_dentry);
+	lower_new_dir.mnt = ecryptfs_dentry_to_lower_mnt(new_dentry);
+	lower_new_dir.dentry = dget_parent(lower_new_dentry);
+	lock_rename(lower_old_dir.dentry, lower_new_dir.dentry);
+	rc = path_rename(&lower_old_dir, lower_old_dentry,
+			 &lower_new_dir, lower_new_dentry);
 	if (rc)
 		goto out_lock;
-	fsstack_copy_attr_all(new_dir, lower_new_dir_dentry->d_inode, NULL);
+	fsstack_copy_attr_all(new_dir, lower_new_dir.dentry->d_inode, NULL);
 	if (new_dir != old_dir)
-		fsstack_copy_attr_all(old_dir, lower_old_dir_dentry->d_inode, NULL);
+		fsstack_copy_attr_all(old_dir, lower_old_dir.dentry->d_inode, NULL);
 out_lock:
-	unlock_rename(lower_old_dir_dentry, lower_new_dir_dentry);
+	unlock_rename(lower_old_dir.dentry, lower_new_dir.dentry);
 	dput(lower_new_dentry->d_parent);
 	dput(lower_old_dentry->d_parent);
 	dput(lower_new_dentry);
Index: linux-2.6/fs/namei.c
===================================================================
--- linux-2.6.orig/fs/namei.c	2008-05-05 11:29:32.000000000 +0200
+++ linux-2.6/fs/namei.c	2008-05-05 11:29:32.000000000 +0200
@@ -2729,8 +2729,8 @@ static int vfs_rename_other(struct inode
 	return error;
 }
 
-int vfs_rename(struct inode *old_dir, struct dentry *old_dentry,
-	       struct inode *new_dir, struct dentry *new_dentry)
+static int vfs_rename(struct inode *old_dir, struct dentry *old_dentry,
+		      struct inode *new_dir, struct dentry *new_dentry)
 {
 	int error;
 	int is_dir = S_ISDIR(old_dentry->d_inode->i_mode);
@@ -2772,6 +2772,27 @@ int vfs_rename(struct inode *old_dir, st
 	return error;
 }
 
+int path_rename(struct path *old_dir_path, struct dentry *old_dentry,
+		struct path *new_dir_path, struct dentry *new_dentry)
+{
+	int error;
+	struct vfsmount *mnt = old_dir_path->mnt;
+
+	BUG_ON(mnt != new_dir_path->mnt);
+
+	error = mnt_want_write(mnt);
+	if (!error) {
+		struct inode *old_dir = old_dir_path->dentry->d_inode;
+		struct inode *new_dir = new_dir_path->dentry->d_inode;
+
+		error = vfs_rename(old_dir, old_dentry, new_dir, new_dentry);
+		mnt_drop_write(mnt);
+	}
+
+	return error;
+}
+EXPORT_SYMBOL(path_rename);
+
 static int do_rename(int olddfd, const char *oldname,
 			int newdfd, const char *newname)
 {
@@ -2833,12 +2854,7 @@ static int do_rename(int olddfd, const c
 	if (new_dentry == trap)
 		goto exit5;
 
-	error = mnt_want_write(oldnd.path.mnt);
-	if (error)
-		goto exit5;
-	error = vfs_rename(old_dir->d_inode, old_dentry,
-				   new_dir->d_inode, new_dentry);
-	mnt_drop_write(oldnd.path.mnt);
+	error = path_rename(&oldnd.path, old_dentry, &newnd.path, new_dentry);
 exit5:
 	dput(new_dentry);
 exit4:
@@ -3029,6 +3045,5 @@ EXPORT_SYMBOL(unlock_rename);
 EXPORT_SYMBOL(vfs_follow_link);
 EXPORT_SYMBOL(generic_permission);
 EXPORT_SYMBOL(vfs_readlink);
-EXPORT_SYMBOL(vfs_rename);
 EXPORT_SYMBOL(dentry_unhash);
 EXPORT_SYMBOL(generic_readlink);
Index: linux-2.6/fs/nfsd/vfs.c
===================================================================
--- linux-2.6.orig/fs/nfsd/vfs.c	2008-05-05 11:29:32.000000000 +0200
+++ linux-2.6/fs/nfsd/vfs.c	2008-05-05 11:29:32.000000000 +0200
@@ -1650,8 +1650,9 @@ __be32
 nfsd_rename(struct svc_rqst *rqstp, struct svc_fh *ffhp, char *fname, int flen,
 			    struct svc_fh *tfhp, char *tname, int tlen)
 {
+	struct path 	old_dir_path;
+	struct path	new_dir_path;
 	struct dentry	*fdentry, *tdentry, *odentry, *ndentry, *trap;
-	struct inode	*fdir, *tdir;
 	__be32		err;
 	int		host_err;
 
@@ -1663,10 +1664,7 @@ nfsd_rename(struct svc_rqst *rqstp, stru
 		goto out;
 
 	fdentry = ffhp->fh_dentry;
-	fdir = fdentry->d_inode;
-
 	tdentry = tfhp->fh_dentry;
-	tdir = tdentry->d_inode;
 
 	err = (rqstp->rq_vers == 2) ? nfserr_acces : nfserr_xdev;
 	if (ffhp->fh_export != tfhp->fh_export)
@@ -1710,22 +1708,15 @@ nfsd_rename(struct svc_rqst *rqstp, stru
 			goto out_dput_new;
 	}
 
-	host_err = -EXDEV;
-	if (ffhp->fh_export->ex_path.mnt != tfhp->fh_export->ex_path.mnt)
-		goto out_dput_new;
-	host_err = mnt_want_write(ffhp->fh_export->ex_path.mnt);
-	if (host_err)
-		goto out_dput_new;
-
-	host_err = vfs_rename(fdir, odentry, tdir, ndentry);
+	fh_to_path(ffhp, &old_dir_path);
+	fh_to_path(tfhp, &new_dir_path);
+	host_err = path_rename(&old_dir_path, odentry, &new_dir_path, ndentry);
 	if (!host_err && EX_ISSYNC(tfhp->fh_export)) {
 		host_err = nfsd_sync_dir(tdentry);
 		if (!host_err)
 			host_err = nfsd_sync_dir(fdentry);
 	}
 
-	mnt_drop_write(ffhp->fh_export->ex_path.mnt);
-
  out_dput_new:
 	dput(ndentry);
  out_dput_old:
Index: linux-2.6/include/linux/fs.h
===================================================================
--- linux-2.6.orig/include/linux/fs.h	2008-05-05 11:29:32.000000000 +0200
+++ linux-2.6/include/linux/fs.h	2008-05-05 11:29:32.000000000 +0200
@@ -1131,7 +1131,8 @@ extern int path_symlink(struct path *, s
 extern int path_link(struct dentry *, struct path *, struct dentry *);
 extern int path_rmdir(struct path *, struct dentry *);
 extern int path_unlink(struct path *, struct dentry *);
-extern int vfs_rename(struct inode *, struct dentry *, struct inode *, struct dentry *);
+extern int path_rename(struct path *, struct dentry *,
+		       struct path *, struct dentry *);
 
 /*
  * VFS dentry helper functions.

--

[patch 08/10] vfs: add path_setattr()

[Miklos Szeredi]

[-- Attachment #1: path_setattr.patch --]
[-- Type: text/plain, Size: 9209 bytes --]

From: Miklos Szeredi <mszeredi@suse.cz>

Introduce path_setattr().  Stop exporting notify_change() to modules.

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---
 fs/attr.c           |   14 +++++++++++++-
 fs/ecryptfs/inode.c |   11 ++++++-----
 fs/nfsd/vfs.c       |   17 +++++++++++------
 fs/open.c           |   52 ++++++++++------------------------------------------
 fs/utimes.c         |    6 +-----
 include/linux/fs.h  |    1 +
 6 files changed, 42 insertions(+), 59 deletions(-)

Index: linux-2.6/fs/open.c
===================================================================
--- linux-2.6.orig/fs/open.c	2008-05-05 11:29:28.000000000 +0200
+++ linux-2.6/fs/open.c	2008-05-05 11:29:32.000000000 +0200
@@ -579,18 +579,13 @@ asmlinkage long sys_fchmod(unsigned int 
 
 	audit_inode(NULL, dentry);
 
-	err = mnt_want_write(file->f_path.mnt);
-	if (err)
-		goto out_putf;
 	mutex_lock(&inode->i_mutex);
 	if (mode == (mode_t) -1)
 		mode = inode->i_mode;
 	newattrs.ia_mode = (mode & S_IALLUGO) | (inode->i_mode & ~S_IALLUGO);
 	newattrs.ia_valid = ATTR_MODE | ATTR_CTIME;
-	err = notify_change(dentry, &newattrs);
+	err = path_setattr(&file->f_path, &newattrs);
 	mutex_unlock(&inode->i_mutex);
-	mnt_drop_write(file->f_path.mnt);
-out_putf:
 	fput(file);
 out:
 	return err;
@@ -609,18 +604,13 @@ asmlinkage long sys_fchmodat(int dfd, co
 		goto out;
 	inode = nd.path.dentry->d_inode;
 
-	error = mnt_want_write(nd.path.mnt);
-	if (error)
-		goto dput_and_out;
 	mutex_lock(&inode->i_mutex);
 	if (mode == (mode_t) -1)
 		mode = inode->i_mode;
 	newattrs.ia_mode = (mode & S_IALLUGO) | (inode->i_mode & ~S_IALLUGO);
 	newattrs.ia_valid = ATTR_MODE | ATTR_CTIME;
-	error = notify_change(nd.path.dentry, &newattrs);
+	error = path_setattr(&nd.path, &newattrs);
 	mutex_unlock(&inode->i_mutex);
-	mnt_drop_write(nd.path.mnt);
-dput_and_out:
 	path_put(&nd.path);
 out:
 	return error;
@@ -631,9 +621,9 @@ asmlinkage long sys_chmod(const char __u
 	return sys_fchmodat(AT_FDCWD, filename, mode);
 }
 
-static int chown_common(struct dentry * dentry, uid_t user, gid_t group)
+static int chown_common(struct path *path, uid_t user, gid_t group)
 {
-	struct inode *inode = dentry->d_inode;
+	struct inode *inode = path->dentry->d_inode;
 	int error;
 	struct iattr newattrs;
 
@@ -650,7 +640,7 @@ static int chown_common(struct dentry * 
 		newattrs.ia_valid |=
 			ATTR_KILL_SUID | ATTR_KILL_SGID | ATTR_KILL_PRIV;
 	mutex_lock(&inode->i_mutex);
-	error = notify_change(dentry, &newattrs);
+	error = path_setattr(path, &newattrs);
 	mutex_unlock(&inode->i_mutex);
 
 	return error;
@@ -664,12 +654,7 @@ asmlinkage long sys_chown(const char __u
 	error = user_path_walk(filename, &nd);
 	if (error)
 		goto out;
-	error = mnt_want_write(nd.path.mnt);
-	if (error)
-		goto out_release;
-	error = chown_common(nd.path.dentry, user, group);
-	mnt_drop_write(nd.path.mnt);
-out_release:
+	error = chown_common(&nd.path, user, group);
 	path_put(&nd.path);
 out:
 	return error;
@@ -689,12 +674,7 @@ asmlinkage long sys_fchownat(int dfd, co
 	error = __user_walk_fd(dfd, filename, follow, &nd);
 	if (error)
 		goto out;
-	error = mnt_want_write(nd.path.mnt);
-	if (error)
-		goto out_release;
-	error = chown_common(nd.path.dentry, user, group);
-	mnt_drop_write(nd.path.mnt);
-out_release:
+	error = chown_common(&nd.path, user, group);
 	path_put(&nd.path);
 out:
 	return error;
@@ -708,12 +688,7 @@ asmlinkage long sys_lchown(const char __
 	error = user_path_walk_link(filename, &nd);
 	if (error)
 		goto out;
-	error = mnt_want_write(nd.path.mnt);
-	if (error)
-		goto out_release;
-	error = chown_common(nd.path.dentry, user, group);
-	mnt_drop_write(nd.path.mnt);
-out_release:
+	error = chown_common(&nd.path, user, group);
 	path_put(&nd.path);
 out:
 	return error;
@@ -724,20 +699,13 @@ asmlinkage long sys_fchown(unsigned int 
 {
 	struct file * file;
 	int error = -EBADF;
-	struct dentry * dentry;
 
 	file = fget(fd);
 	if (!file)
 		goto out;
 
-	error = mnt_want_write(file->f_path.mnt);
-	if (error)
-		goto out_fput;
-	dentry = file->f_path.dentry;
-	audit_inode(NULL, dentry);
-	error = chown_common(dentry, user, group);
-	mnt_drop_write(file->f_path.mnt);
-out_fput:
+	audit_inode(NULL, file->f_path.dentry);
+	error = chown_common(&file->f_path, user, group);
 	fput(file);
 out:
 	return error;
Index: linux-2.6/fs/attr.c
===================================================================
--- linux-2.6.orig/fs/attr.c	2008-05-05 11:29:27.000000000 +0200
+++ linux-2.6/fs/attr.c	2008-05-05 11:29:32.000000000 +0200
@@ -14,6 +14,7 @@
 #include <linux/fcntl.h>
 #include <linux/quotaops.h>
 #include <linux/security.h>
+#include <linux/mount.h>
 
 /* Taken over from the old code... */
 
@@ -195,4 +196,15 @@ int notify_change(struct dentry * dentry
 	return error;
 }
 
-EXPORT_SYMBOL(notify_change);
+int path_setattr(struct path *path, struct iattr *attr)
+{
+	int error = mnt_want_write(path->mnt);
+
+	if (!error) {
+		error = notify_change(path->dentry, attr);
+		mnt_drop_write(path->mnt);
+	}
+
+	return error;
+}
+EXPORT_SYMBOL(path_setattr);
Index: linux-2.6/fs/nfsd/vfs.c
===================================================================
--- linux-2.6.orig/fs/nfsd/vfs.c	2008-05-05 11:29:32.000000000 +0200
+++ linux-2.6/fs/nfsd/vfs.c	2008-05-05 11:29:32.000000000 +0200
@@ -392,8 +392,11 @@ nfsd_setattr(struct svc_rqst *rqstp, str
 
 	err = nfserr_notsync;
 	if (!check_guard || guardtime == inode->i_ctime.tv_sec) {
+		struct path path;
+
 		fh_lock(fhp);
-		host_err = notify_change(dentry, iap);
+		fh_to_path(fhp, &path);
+		host_err = path_setattr(&path, iap);
 		err = nfserrno(host_err);
 		fh_unlock(fhp);
 	}
@@ -949,14 +952,16 @@ out:
 	return err;
 }
 
-static void kill_suid(struct dentry *dentry)
+static void kill_suid(struct svc_fh *fhp)
 {
+	struct path	path;
 	struct iattr	ia;
 	ia.ia_valid = ATTR_KILL_SUID | ATTR_KILL_SGID | ATTR_KILL_PRIV;
 
-	mutex_lock(&dentry->d_inode->i_mutex);
-	notify_change(dentry, &ia);
-	mutex_unlock(&dentry->d_inode->i_mutex);
+	mutex_lock(&path.dentry->d_inode->i_mutex);
+	fh_to_path(fhp, &path);
+	path_setattr(&path, &ia);
+	mutex_unlock(&path.dentry->d_inode->i_mutex);
 }
 
 static __be32
@@ -1014,7 +1019,7 @@ nfsd_vfs_write(struct svc_rqst *rqstp, s
 
 	/* clear setuid/setgid flag after write */
 	if (host_err >= 0 && (inode->i_mode & (S_ISUID | S_ISGID)))
-		kill_suid(dentry);
+		kill_suid(fhp);
 
 	if (host_err >= 0 && stable) {
 		static ino_t	last_ino;
Index: linux-2.6/include/linux/fs.h
===================================================================
--- linux-2.6.orig/include/linux/fs.h	2008-05-05 11:29:32.000000000 +0200
+++ linux-2.6/include/linux/fs.h	2008-05-05 11:29:32.000000000 +0200
@@ -1761,6 +1761,7 @@ extern int do_remount_sb(struct super_bl
 extern sector_t bmap(struct inode *, sector_t);
 #endif
 extern int notify_change(struct dentry *, struct iattr *);
+extern int path_setattr(struct path *, struct iattr *);
 extern int permission(struct inode *, int, struct nameidata *);
 extern int generic_permission(struct inode *, int,
 		int (*check_acl)(struct inode *, int));
Index: linux-2.6/fs/ecryptfs/inode.c
===================================================================
--- linux-2.6.orig/fs/ecryptfs/inode.c	2008-05-05 11:29:32.000000000 +0200
+++ linux-2.6/fs/ecryptfs/inode.c	2008-05-05 11:29:32.000000000 +0200
@@ -843,7 +843,7 @@ ecryptfs_permission(struct inode *inode,
 static int ecryptfs_setattr(struct dentry *dentry, struct iattr *ia)
 {
 	int rc = 0;
-	struct dentry *lower_dentry;
+	struct path lower_path;
 	struct inode *inode;
 	struct inode *lower_inode;
 	struct ecryptfs_crypt_stat *crypt_stat;
@@ -853,7 +853,8 @@ static int ecryptfs_setattr(struct dentr
 		ecryptfs_init_crypt_stat(crypt_stat);
 	inode = dentry->d_inode;
 	lower_inode = ecryptfs_inode_to_lower(inode);
-	lower_dentry = ecryptfs_dentry_to_lower(dentry);
+	lower_path.mnt = ecryptfs_dentry_to_lower_mnt(dentry);
+	lower_path.dentry = ecryptfs_dentry_to_lower(dentry);
 	mutex_lock(&crypt_stat->cs_mutex);
 	if (S_ISDIR(dentry->d_inode->i_mode))
 		crypt_stat->flags &= ~(ECRYPTFS_ENCRYPTED);
@@ -904,9 +905,9 @@ static int ecryptfs_setattr(struct dentr
 	if (ia->ia_valid & (ATTR_KILL_SUID | ATTR_KILL_SGID))
 		ia->ia_valid &= ~ATTR_MODE;
 
-	mutex_lock(&lower_dentry->d_inode->i_mutex);
-	rc = notify_change(lower_dentry, ia);
-	mutex_unlock(&lower_dentry->d_inode->i_mutex);
+	mutex_lock(&lower_path.dentry->d_inode->i_mutex);
+	rc = path_setattr(&lower_path, ia);
+	mutex_unlock(&lower_path.dentry->d_inode->i_mutex);
 out:
 	fsstack_copy_attr_all(inode, lower_inode, NULL);
 	return rc;
Index: linux-2.6/fs/utimes.c
===================================================================
--- linux-2.6.orig/fs/utimes.c	2008-05-05 11:29:28.000000000 +0200
+++ linux-2.6/fs/utimes.c	2008-05-05 11:29:32.000000000 +0200
@@ -78,11 +78,7 @@ static int utimes_common(struct path *pa
 		}
 	}
 	mutex_lock(&path->dentry->d_inode->i_mutex);
-	error = mnt_want_write(path->mnt);
-	if (!error) {
-		error = notify_change(path->dentry, &newattrs);
-		mnt_drop_write(path->mnt);
-	}
+	error = path_setattr(path, &newattrs);
 	mutex_unlock(&path->dentry->d_inode->i_mutex);
 
 	return error;

--

[patch 09/10] vfs: add path_setxattr()

[Miklos Szeredi]

[-- Attachment #1: path_setxattr.patch --]
[-- Type: text/plain, Size: 6486 bytes --]

From: Miklos Szeredi <mszeredi@suse.cz>

Introduce path_setxattr().  Make vfs_setxattr() static.

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---
 fs/nfsd/vfs.c         |   18 ++++++++++--------
 fs/xattr.c            |   44 ++++++++++++++++++++++----------------------
 include/linux/xattr.h |    3 ++-
 3 files changed, 34 insertions(+), 31 deletions(-)

Index: linux-2.6/fs/nfsd/vfs.c
===================================================================
--- linux-2.6.orig/fs/nfsd/vfs.c	2008-05-05 11:29:32.000000000 +0200
+++ linux-2.6/fs/nfsd/vfs.c	2008-05-05 11:29:33.000000000 +0200
@@ -434,7 +434,7 @@ static ssize_t nfsd_getxattr(struct dent
 
 #if defined(CONFIG_NFSD_V4)
 static int
-set_nfsv4_acl_one(struct dentry *dentry, struct posix_acl *pacl, char *key)
+set_nfsv4_acl_one(struct path *path, struct posix_acl *pacl, char *key)
 {
 	int len;
 	size_t buflen;
@@ -453,7 +453,7 @@ set_nfsv4_acl_one(struct dentry *dentry,
 		goto out;
 	}
 
-	error = vfs_setxattr(dentry, key, buf, len, 0);
+	error = path_setxattr(path, key, buf, len, 0);
 out:
 	kfree(buf);
 	return error;
@@ -465,7 +465,7 @@ nfsd4_set_nfs4_acl(struct svc_rqst *rqst
 {
 	__be32 error;
 	int host_error;
-	struct dentry *dentry;
+	struct path path;
 	struct inode *inode;
 	struct posix_acl *pacl = NULL, *dpacl = NULL;
 	unsigned int flags = 0;
@@ -475,8 +475,8 @@ nfsd4_set_nfs4_acl(struct svc_rqst *rqst
 	if (error)
 		return error;
 
-	dentry = fhp->fh_dentry;
-	inode = dentry->d_inode;
+	fh_to_path(fhp, &path);
+	inode = path.dentry->d_inode;
 	if (S_ISDIR(inode->i_mode))
 		flags = NFS4_ACL_DIR;
 
@@ -486,12 +486,12 @@ nfsd4_set_nfs4_acl(struct svc_rqst *rqst
 	} else if (host_error < 0)
 		goto out_nfserr;
 
-	host_error = set_nfsv4_acl_one(dentry, pacl, POSIX_ACL_XATTR_ACCESS);
+	host_error = set_nfsv4_acl_one(&path, pacl, POSIX_ACL_XATTR_ACCESS);
 	if (host_error < 0)
 		goto out_release;
 
 	if (S_ISDIR(inode->i_mode))
-		host_error = set_nfsv4_acl_one(dentry, dpacl, POSIX_ACL_XATTR_DEFAULT);
+		host_error = set_nfsv4_acl_one(&path, dpacl, POSIX_ACL_XATTR_DEFAULT);
 
 out_release:
 	posix_acl_release(pacl);
@@ -2038,6 +2038,7 @@ nfsd_get_posix_acl(struct svc_fh *fhp, i
 int
 nfsd_set_posix_acl(struct svc_fh *fhp, int type, struct posix_acl *acl)
 {
+	struct path path;
 	struct inode *inode = fhp->fh_dentry->d_inode;
 	char *name;
 	void *value = NULL;
@@ -2073,8 +2074,9 @@ nfsd_set_posix_acl(struct svc_fh *fhp, i
 	error = mnt_want_write(fhp->fh_export->ex_path.mnt);
 	if (error)
 		goto getout;
+	fh_to_path(fhp, &path);
 	if (size)
-		error = vfs_setxattr(fhp->fh_dentry, name, value, size, 0);
+		error = path_setxattr(&path, name, value, size, 0);
 	else {
 		if (!S_ISDIR(inode->i_mode) && type == ACL_TYPE_DEFAULT)
 			error = 0;
Index: linux-2.6/fs/xattr.c
===================================================================
--- linux-2.6.orig/fs/xattr.c	2008-05-05 11:28:37.000000000 +0200
+++ linux-2.6/fs/xattr.c	2008-05-05 11:29:33.000000000 +0200
@@ -66,7 +66,7 @@ xattr_permission(struct inode *inode, co
 	return permission(inode, mask, NULL);
 }
 
-int
+static int
 vfs_setxattr(struct dentry *dentry, const char *name, const void *value,
 		size_t size, int flags)
 {
@@ -101,7 +101,20 @@ out:
 	mutex_unlock(&inode->i_mutex);
 	return error;
 }
-EXPORT_SYMBOL_GPL(vfs_setxattr);
+
+int path_setxattr(struct path *path, const char *name, const void *value,
+		   size_t size, int flags)
+{
+	int error = mnt_want_write(path->mnt);
+
+	if (!error) {
+		error = vfs_setxattr(path->dentry, name, value, size, flags);
+		mnt_drop_write(path->mnt);
+	}
+
+	return error;
+}
+EXPORT_SYMBOL(path_setxattr);
 
 ssize_t
 xattr_getsecurity(struct inode *inode, const char *name, void *value,
@@ -218,7 +231,7 @@ EXPORT_SYMBOL_GPL(vfs_removexattr);
  * Extended attribute SET operations
  */
 static long
-setxattr(struct dentry *d, const char __user *name, const void __user *value,
+setxattr(struct path *path, const char __user *name, const void __user *value,
 	 size_t size, int flags)
 {
 	int error;
@@ -246,7 +259,7 @@ setxattr(struct dentry *d, const char __
 		}
 	}
 
-	error = vfs_setxattr(d, kname, kvalue, size, flags);
+	error = path_setxattr(path, kname, kvalue, size, flags);
 	kfree(kvalue);
 	return error;
 }
@@ -261,11 +274,8 @@ sys_setxattr(const char __user *path, co
 	error = user_path_walk(path, &nd);
 	if (error)
 		return error;
-	error = mnt_want_write(nd.path.mnt);
-	if (!error) {
-		error = setxattr(nd.path.dentry, name, value, size, flags);
-		mnt_drop_write(nd.path.mnt);
-	}
+
+	error = setxattr(&nd.path, name, value, size, flags);
 	path_put(&nd.path);
 	return error;
 }
@@ -280,11 +290,7 @@ sys_lsetxattr(const char __user *path, c
 	error = user_path_walk_link(path, &nd);
 	if (error)
 		return error;
-	error = mnt_want_write(nd.path.mnt);
-	if (!error) {
-		error = setxattr(nd.path.dentry, name, value, size, flags);
-		mnt_drop_write(nd.path.mnt);
-	}
+	error = setxattr(&nd.path, name, value, size, flags);
 	path_put(&nd.path);
 	return error;
 }
@@ -294,19 +300,13 @@ sys_fsetxattr(int fd, const char __user 
 	      size_t size, int flags)
 {
 	struct file *f;
-	struct dentry *dentry;
 	int error = -EBADF;
 
 	f = fget(fd);
 	if (!f)
 		return error;
-	dentry = f->f_path.dentry;
-	audit_inode(NULL, dentry);
-	error = mnt_want_write(f->f_path.mnt);
-	if (!error) {
-		error = setxattr(dentry, name, value, size, flags);
-		mnt_drop_write(f->f_path.mnt);
-	}
+	audit_inode(NULL, f->f_path.dentry);
+	error = setxattr(&f->f_path, name, value, size, flags);
 	fput(f);
 	return error;
 }
Index: linux-2.6/include/linux/xattr.h
===================================================================
--- linux-2.6.orig/include/linux/xattr.h	2008-05-05 11:28:37.000000000 +0200
+++ linux-2.6/include/linux/xattr.h	2008-05-05 11:29:33.000000000 +0200
@@ -35,6 +35,7 @@
 
 struct inode;
 struct dentry;
+struct path;
 
 struct xattr_handler {
 	char *prefix;
@@ -49,7 +50,7 @@ struct xattr_handler {
 ssize_t xattr_getsecurity(struct inode *, const char *, void *, size_t);
 ssize_t vfs_getxattr(struct dentry *, const char *, void *, size_t);
 ssize_t vfs_listxattr(struct dentry *d, char *list, size_t size);
-int vfs_setxattr(struct dentry *, const char *, const void *, size_t, int);
+int path_setxattr(struct path *, const char *, const void *, size_t, int);
 int vfs_removexattr(struct dentry *, const char *);
 
 ssize_t generic_getxattr(struct dentry *dentry, const char *name, void *buffer, size_t size);

--

[patch 10/10] vfs: add path_removexattr()

[Miklos Szeredi]

[-- Attachment #1: path_removexattr.patch --]
[-- Type: text/plain, Size: 4607 bytes --]

From: Miklos Szeredi <mszeredi@suse.cz>

Introduce path_removexattr().  Make vfs_removexattr() static.

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---
 fs/nfsd/vfs.c         |    6 +-----
 fs/xattr.c            |   41 +++++++++++++++++++----------------------
 include/linux/xattr.h |    2 +-
 3 files changed, 21 insertions(+), 28 deletions(-)

Index: linux-2.6/fs/nfsd/vfs.c
===================================================================
--- linux-2.6.orig/fs/nfsd/vfs.c	2008-05-05 11:29:33.000000000 +0200
+++ linux-2.6/fs/nfsd/vfs.c	2008-05-05 11:29:33.000000000 +0200
@@ -2071,9 +2071,6 @@ nfsd_set_posix_acl(struct svc_fh *fhp, i
 	} else
 		size = 0;
 
-	error = mnt_want_write(fhp->fh_export->ex_path.mnt);
-	if (error)
-		goto getout;
 	fh_to_path(fhp, &path);
 	if (size)
 		error = path_setxattr(&path, name, value, size, 0);
@@ -2081,12 +2078,11 @@ nfsd_set_posix_acl(struct svc_fh *fhp, i
 		if (!S_ISDIR(inode->i_mode) && type == ACL_TYPE_DEFAULT)
 			error = 0;
 		else {
-			error = vfs_removexattr(fhp->fh_dentry, name);
+			error = path_removexattr(&path, name);
 			if (error == -ENODATA)
 				error = 0;
 		}
 	}
-	mnt_drop_write(fhp->fh_export->ex_path.mnt);
 
 getout:
 	kfree(value);
Index: linux-2.6/fs/xattr.c
===================================================================
--- linux-2.6.orig/fs/xattr.c	2008-05-05 11:29:33.000000000 +0200
+++ linux-2.6/fs/xattr.c	2008-05-05 11:29:33.000000000 +0200
@@ -199,7 +199,7 @@ vfs_listxattr(struct dentry *d, char *li
 }
 EXPORT_SYMBOL_GPL(vfs_listxattr);
 
-int
+static int
 vfs_removexattr(struct dentry *dentry, const char *name)
 {
 	struct inode *inode = dentry->d_inode;
@@ -224,8 +224,19 @@ vfs_removexattr(struct dentry *dentry, c
 		fsnotify_xattr(dentry);
 	return error;
 }
-EXPORT_SYMBOL_GPL(vfs_removexattr);
 
+int path_removexattr(struct path *path, const char *name)
+{
+	int error = mnt_want_write(path->mnt);
+
+	if (!error) {
+		error = vfs_removexattr(path->dentry, name);
+		mnt_drop_write(path->mnt);
+	}
+
+	return error;
+}
+EXPORT_SYMBOL(path_removexattr);
 
 /*
  * Extended attribute SET operations
@@ -471,7 +482,7 @@ sys_flistxattr(int fd, char __user *list
  * Extended attribute REMOVE operations
  */
 static long
-removexattr(struct dentry *d, const char __user *name)
+removexattr(struct path *path, const char __user *name)
 {
 	int error;
 	char kname[XATTR_NAME_MAX + 1];
@@ -482,7 +493,7 @@ removexattr(struct dentry *d, const char
 	if (error < 0)
 		return error;
 
-	return vfs_removexattr(d, kname);
+	return path_removexattr(path, kname);
 }
 
 asmlinkage long
@@ -494,11 +505,7 @@ sys_removexattr(const char __user *path,
 	error = user_path_walk(path, &nd);
 	if (error)
 		return error;
-	error = mnt_want_write(nd.path.mnt);
-	if (!error) {
-		error = removexattr(nd.path.dentry, name);
-		mnt_drop_write(nd.path.mnt);
-	}
+	error = removexattr(&nd.path, name);
 	path_put(&nd.path);
 	return error;
 }
@@ -512,11 +519,7 @@ sys_lremovexattr(const char __user *path
 	error = user_path_walk_link(path, &nd);
 	if (error)
 		return error;
-	error = mnt_want_write(nd.path.mnt);
-	if (!error) {
-		error = removexattr(nd.path.dentry, name);
-		mnt_drop_write(nd.path.mnt);
-	}
+	error = removexattr(&nd.path, name);
 	path_put(&nd.path);
 	return error;
 }
@@ -525,19 +528,13 @@ asmlinkage long
 sys_fremovexattr(int fd, const char __user *name)
 {
 	struct file *f;
-	struct dentry *dentry;
 	int error = -EBADF;
 
 	f = fget(fd);
 	if (!f)
 		return error;
-	dentry = f->f_path.dentry;
-	audit_inode(NULL, dentry);
-	error = mnt_want_write(f->f_path.mnt);
-	if (!error) {
-		error = removexattr(dentry, name);
-		mnt_drop_write(f->f_path.mnt);
-	}
+	audit_inode(NULL, f->f_path.dentry);
+	error = removexattr(&f->f_path, name);
 	fput(f);
 	return error;
 }
Index: linux-2.6/include/linux/xattr.h
===================================================================
--- linux-2.6.orig/include/linux/xattr.h	2008-05-05 11:29:33.000000000 +0200
+++ linux-2.6/include/linux/xattr.h	2008-05-05 11:29:33.000000000 +0200
@@ -51,7 +51,7 @@ ssize_t xattr_getsecurity(struct inode *
 ssize_t vfs_getxattr(struct dentry *, const char *, void *, size_t);
 ssize_t vfs_listxattr(struct dentry *d, char *list, size_t size);
 int path_setxattr(struct path *, const char *, const void *, size_t, int);
-int vfs_removexattr(struct dentry *, const char *);
+int path_removexattr(struct path *, const char *);
 
 ssize_t generic_getxattr(struct dentry *dentry, const char *name, void *buffer, size_t size);
 ssize_t generic_listxattr(struct dentry *dentry, char *buffer, size_t buffer_size);

--

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

[Miklos Szeredi]

See first patch for description.  This is against vfs-2.6 + two
patches I posted recently.

It's not for -mm yet, as it needs conversion of unionfs (will that be
done by me, I wonder?)

Thanks,
Miklos

--

[patch 01/10] vfs: add path_create() and path_mknod()

[Miklos Szeredi]

[-- Attachment #1: path_create.patch --]
[-- Type: text/plain, Size: 11638 bytes --]

From: Miklos Szeredi <mszeredi@suse.cz>

R/o bind mounts require operations which modify the filesystem to be
wrapped in mnt_want_write()/mnt_drop_write().  Create helpers which do
this, so callers won't need to bother, and more importantly, cannot
forget!  Call these path_*, analogous to vfs_*.  Where there are no
callers of vfs_* left, make them static.

This series is a cleanup, as well as fixing several places (mostly in
nfsd) where mnt_{want,drop}_write() were missing.

It will also help with merging You Know What(*) security module, which
needs to know the path within the namespace, and not just within the
filesystem.  These helpers will allow the security hooks to be in a
common place, and need not be repeated in all callers.

(*) AppArmor is a swear word in VFS circles, for some reason or other.

This patch:

Introduce path_create() and path_mknod().  Make vfs_create() and
vfs_mknod() static.

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---
 fs/ecryptfs/inode.c |   21 +++++++-------
 fs/namei.c          |   75 +++++++++++++++++++++++++++-------------------------
 fs/nfsd/vfs.c       |   19 ++++++-------
 include/linux/fs.h  |    4 +-
 ipc/mqueue.c        |    6 +++-
 net/unix/af_unix.c  |    6 ----
 6 files changed, 68 insertions(+), 63 deletions(-)

Index: vfs-2.6/fs/namei.c
===================================================================
--- vfs-2.6.orig/fs/namei.c	2008-04-02 21:10:14.000000000 +0200
+++ vfs-2.6/fs/namei.c	2008-04-02 21:10:17.000000000 +0200
@@ -1574,7 +1574,7 @@ void unlock_rename(struct dentry *p1, st
 	}
 }
 
-int vfs_create(struct inode *dir, struct dentry *dentry, int mode,
+static int vfs_create(struct inode *dir, struct dentry *dentry, int mode,
 		struct nameidata *nd)
 {
 	int error = may_create(dir, dentry, nd);
@@ -1596,6 +1596,20 @@ int vfs_create(struct inode *dir, struct
 	return error;
 }
 
+int path_create(struct path *dir_path, struct dentry *dentry, int mode,
+		struct nameidata *nd)
+{
+	int error = mnt_want_write(dir_path->mnt);
+
+	if (!error) {
+		error = vfs_create(dir_path->dentry->d_inode, dentry, mode, nd);
+		mnt_drop_write(dir_path->mnt);
+	}
+
+	return error;
+}
+EXPORT_SYMBOL(path_create);
+
 int may_open(struct nameidata *nd, int acc_mode, int flag)
 {
 	struct dentry *dentry = nd->path.dentry;
@@ -2015,7 +2029,7 @@ fail:
 }
 EXPORT_SYMBOL_GPL(lookup_create);
 
-int vfs_mknod(struct inode *dir, struct dentry *dentry, int mode, dev_t dev)
+static int vfs_mknod(struct inode *dir, struct dentry *dentry, int mode, dev_t dev)
 {
 	int error = may_create(dir, dentry, NULL);
 
@@ -2039,22 +2053,19 @@ int vfs_mknod(struct inode *dir, struct 
 	return error;
 }
 
-static int may_mknod(mode_t mode)
+int path_mknod(struct path *dir_path, struct dentry *dentry, int mode,
+	       dev_t dev)
 {
-	switch (mode & S_IFMT) {
-	case S_IFREG:
-	case S_IFCHR:
-	case S_IFBLK:
-	case S_IFIFO:
-	case S_IFSOCK:
-	case 0: /* zero mode translates to S_IFREG */
-		return 0;
-	case S_IFDIR:
-		return -EPERM;
-	default:
-		return -EINVAL;
+	int error = mnt_want_write(dir_path->mnt);
+
+	if (!error) {
+		error = vfs_mknod(dir_path->dentry->d_inode, dentry, mode, dev);
+		mnt_drop_write(dir_path->mnt);
 	}
+
+	return error;
 }
+EXPORT_SYMBOL(path_mknod);
 
 asmlinkage long sys_mknodat(int dfd, const char __user *filename, int mode,
 				unsigned dev)
@@ -2080,26 +2091,22 @@ asmlinkage long sys_mknodat(int dfd, con
 	}
 	if (!IS_POSIXACL(nd.path.dentry->d_inode))
 		mode &= ~current->fs->umask;
-	error = may_mknod(mode);
-	if (error)
-		goto out_dput;
-	error = mnt_want_write(nd.path.mnt);
-	if (error)
-		goto out_dput;
 	switch (mode & S_IFMT) {
-		case 0: case S_IFREG:
-			error = vfs_create(nd.path.dentry->d_inode,dentry,mode,&nd);
-			break;
-		case S_IFCHR: case S_IFBLK:
-			error = vfs_mknod(nd.path.dentry->d_inode,dentry,mode,
-					new_decode_dev(dev));
-			break;
-		case S_IFIFO: case S_IFSOCK:
-			error = vfs_mknod(nd.path.dentry->d_inode,dentry,mode,0);
-			break;
+	case 0: case S_IFREG:
+		error = path_create(&nd.path, dentry, mode, &nd);
+		break;
+	case S_IFCHR: case S_IFBLK:
+		error = path_mknod(&nd.path, dentry, mode, new_decode_dev(dev));
+		break;
+	case S_IFIFO: case S_IFSOCK:
+		error = path_mknod(&nd.path, dentry, mode, 0);
+		break;
+	case S_IFDIR:
+		error = -EPERM;
+		break;
+	default:
+		error = -EINVAL;
 	}
-	mnt_drop_write(nd.path.mnt);
-out_dput:
 	dput(dentry);
 out_unlock:
 	mutex_unlock(&nd.path.dentry->d_inode->i_mutex);
@@ -2966,11 +2973,9 @@ EXPORT_SYMBOL(permission);
 EXPORT_SYMBOL(vfs_permission);
 EXPORT_SYMBOL(file_permission);
 EXPORT_SYMBOL(unlock_rename);
-EXPORT_SYMBOL(vfs_create);
 EXPORT_SYMBOL(vfs_follow_link);
 EXPORT_SYMBOL(vfs_link);
 EXPORT_SYMBOL(vfs_mkdir);
-EXPORT_SYMBOL(vfs_mknod);
 EXPORT_SYMBOL(generic_permission);
 EXPORT_SYMBOL(vfs_readlink);
 EXPORT_SYMBOL(vfs_rename);
Index: vfs-2.6/include/linux/fs.h
===================================================================
--- vfs-2.6.orig/include/linux/fs.h	2008-04-02 21:10:14.000000000 +0200
+++ vfs-2.6/include/linux/fs.h	2008-04-02 21:10:17.000000000 +0200
@@ -1123,9 +1123,9 @@ extern void unlock_super(struct super_bl
  * VFS helper functions..
  */
 extern int vfs_permission(struct nameidata *, int);
-extern int vfs_create(struct inode *, struct dentry *, int, struct nameidata *);
+extern int path_create(struct path *, struct dentry *, int, struct nameidata *);
 extern int vfs_mkdir(struct inode *, struct dentry *, int);
-extern int vfs_mknod(struct inode *, struct dentry *, int, dev_t);
+extern int path_mknod(struct path *, struct dentry *, int, dev_t);
 extern int vfs_symlink(struct inode *, struct dentry *, const char *, int);
 extern int vfs_link(struct dentry *, struct inode *, struct dentry *);
 extern int vfs_rmdir(struct inode *, struct dentry *);
Index: vfs-2.6/fs/ecryptfs/inode.c
===================================================================
--- vfs-2.6.orig/fs/ecryptfs/inode.c	2008-04-02 21:10:15.000000000 +0200
+++ vfs-2.6/fs/ecryptfs/inode.c	2008-04-02 21:10:17.000000000 +0200
@@ -67,7 +67,7 @@ static void unlock_dir(struct dentry *di
  * Returns zero on success; non-zero on error condition
  */
 static int
-ecryptfs_create_underlying_file(struct inode *lower_dir_inode,
+ecryptfs_create_underlying_file(struct dentry *lower_dir_dentry,
 				struct dentry *dentry, int mode,
 				struct nameidata *nd)
 {
@@ -79,9 +79,9 @@ ecryptfs_create_underlying_file(struct i
 
 	dentry_save = nd->path.dentry;
 	vfsmount_save = nd->path.mnt;
-	nd->path.dentry = lower_dentry;
+	nd->path.dentry = lower_dir_dentry;
 	nd->path.mnt = lower_mnt;
-	rc = vfs_create(lower_dir_inode, lower_dentry, mode, nd);
+	rc = path_create(&nd->path, lower_dentry, mode, nd);
 	nd->path.dentry = dentry_save;
 	nd->path.mnt = vfsmount_save;
 	return rc;
@@ -117,7 +117,7 @@ ecryptfs_do_create(struct inode *directo
 		rc = PTR_ERR(lower_dir_dentry);
 		goto out;
 	}
-	rc = ecryptfs_create_underlying_file(lower_dir_dentry->d_inode,
+	rc = ecryptfs_create_underlying_file(lower_dir_dentry,
 					     ecryptfs_dentry, mode, nd);
 	if (rc) {
 		printk(KERN_ERR "%s: Failure to create dentry in lower fs; "
@@ -535,20 +535,21 @@ ecryptfs_mknod(struct inode *dir, struct
 {
 	int rc;
 	struct dentry *lower_dentry;
-	struct dentry *lower_dir_dentry;
+	struct path lower_dir;
 
 	lower_dentry = ecryptfs_dentry_to_lower(dentry);
-	lower_dir_dentry = lock_parent(lower_dentry);
-	rc = vfs_mknod(lower_dir_dentry->d_inode, lower_dentry, mode, dev);
+	lower_dir.mnt = ecryptfs_dentry_to_lower_mnt(dentry);
+	lower_dir.dentry = lock_parent(lower_dentry);
+	rc = path_mknod(&lower_dir, lower_dentry, mode, dev);
 	if (rc || !lower_dentry->d_inode)
 		goto out;
 	rc = ecryptfs_interpose(lower_dentry, dentry, dir->i_sb, 0);
 	if (rc)
 		goto out;
-	fsstack_copy_attr_times(dir, lower_dir_dentry->d_inode);
-	fsstack_copy_inode_size(dir, lower_dir_dentry->d_inode);
+	fsstack_copy_attr_times(dir, lower_dir.dentry->d_inode);
+	fsstack_copy_inode_size(dir, lower_dir.dentry->d_inode);
 out:
-	unlock_dir(lower_dir_dentry);
+	unlock_dir(lower_dir.dentry);
 	if (!dentry->d_inode)
 		d_drop(dentry);
 	return rc;
Index: vfs-2.6/fs/nfsd/vfs.c
===================================================================
--- vfs-2.6.orig/fs/nfsd/vfs.c	2008-04-02 21:10:14.000000000 +0200
+++ vfs-2.6/fs/nfsd/vfs.c	2008-04-02 21:10:17.000000000 +0200
@@ -1185,6 +1185,7 @@ nfsd_create(struct svc_rqst *rqstp, stru
 		char *fname, int flen, struct iattr *iap,
 		int type, dev_t rdev, struct svc_fh *resfhp)
 {
+	struct path 	dir_path;
 	struct dentry	*dentry, *dchild = NULL;
 	struct inode	*dirp;
 	__be32		err;
@@ -1249,13 +1250,12 @@ nfsd_create(struct svc_rqst *rqstp, stru
 		iap->ia_mode = 0;
 	iap->ia_mode = (iap->ia_mode & S_IALLUGO) | type;
 
-	/*
-	 * Get the dir op function pointer.
-	 */
+	dir_path.mnt = fhp->fh_export->ex_path.mnt;
+	dir_path.dentry = dentry;
 	err = 0;
 	switch (type) {
 	case S_IFREG:
-		host_err = vfs_create(dirp, dchild, iap->ia_mode, NULL);
+		host_err = path_create(&dir_path, dchild, iap->ia_mode, NULL);
 		break;
 	case S_IFDIR:
 		host_err = vfs_mkdir(dirp, dchild, iap->ia_mode);
@@ -1264,11 +1264,7 @@ nfsd_create(struct svc_rqst *rqstp, stru
 	case S_IFBLK:
 	case S_IFIFO:
 	case S_IFSOCK:
-		host_err = mnt_want_write(fhp->fh_export->ex_path.mnt);
-		if (host_err)
-			break;
-		host_err = vfs_mknod(dirp, dchild, iap->ia_mode, rdev);
-		mnt_drop_write(fhp->fh_export->ex_path.mnt);
+		host_err = path_mknod(&dir_path, dchild, iap->ia_mode, rdev);
 		break;
 	default:
 	        printk("nfsd: bad file type %o in nfsd_create\n", type);
@@ -1311,6 +1307,7 @@ nfsd_create_v3(struct svc_rqst *rqstp, s
 		struct svc_fh *resfhp, int createmode, u32 *verifier,
 	        int *truncp, int *created)
 {
+	struct path 	dir_path;
 	struct dentry	*dentry, *dchild = NULL;
 	struct inode	*dirp;
 	__be32		err;
@@ -1397,7 +1394,9 @@ nfsd_create_v3(struct svc_rqst *rqstp, s
 		goto out;
 	}
 
-	host_err = vfs_create(dirp, dchild, iap->ia_mode, NULL);
+	dir_path.mnt = fhp->fh_export->ex_path.mnt;
+	dir_path.dentry = dentry;
+	host_err = path_create(&dir_path, dchild, iap->ia_mode, NULL);
 	if (host_err < 0)
 		goto out_nfserr;
 	if (created)
Index: vfs-2.6/ipc/mqueue.c
===================================================================
--- vfs-2.6.orig/ipc/mqueue.c	2008-04-02 21:10:14.000000000 +0200
+++ vfs-2.6/ipc/mqueue.c	2008-04-02 21:10:17.000000000 +0200
@@ -599,6 +599,7 @@ static struct file *do_create(struct den
 {
 	struct mq_attr attr;
 	struct file *result;
+	struct path dir_path;
 	int ret;
 
 	if (u_attr) {
@@ -616,7 +617,10 @@ static struct file *do_create(struct den
 	ret = mnt_want_write(mqueue_mnt);
 	if (ret)
 		goto out;
-	ret = vfs_create(dir->d_inode, dentry, mode, NULL);
+
+	dir_path.mnt = mqueue_mnt;
+	dir_path.dentry = dir;
+	ret = path_create(&dir_path, dentry, mode, NULL);
 	dentry->d_fsdata = NULL;
 	if (ret)
 		goto out_drop_write;
Index: vfs-2.6/net/unix/af_unix.c
===================================================================
--- vfs-2.6.orig/net/unix/af_unix.c	2008-04-02 21:10:14.000000000 +0200
+++ vfs-2.6/net/unix/af_unix.c	2008-04-02 21:10:17.000000000 +0200
@@ -819,11 +819,7 @@ static int unix_bind(struct socket *sock
 		 */
 		mode = S_IFSOCK |
 		       (SOCK_INODE(sock)->i_mode & ~current->fs->umask);
-		err = mnt_want_write(nd.path.mnt);
-		if (err)
-			goto out_mknod_dput;
-		err = vfs_mknod(nd.path.dentry->d_inode, dentry, mode, 0);
-		mnt_drop_write(nd.path.mnt);
+		err = path_mknod(&nd.path, dentry, mode, 0);
 		if (err)
 			goto out_mknod_dput;
 		mutex_unlock(&nd.path.dentry->d_inode->i_mutex);

--

Re: [patch 01/10] vfs: add path_create() and path_mknod()

[Al Viro]

On Wed, Apr 02, 2008 at 10:12:48PM +0200, Miklos Szeredi wrote:
> From: Miklos Szeredi <mszeredi@suse.cz>
> 
> R/o bind mounts require operations which modify the filesystem to be
> wrapped in mnt_want_write()/mnt_drop_write().  Create helpers which do
> this, so callers won't need to bother, and more importantly, cannot
> forget!  Call these path_*, analogous to vfs_*.  Where there are no
> callers of vfs_* left, make them static.
>
> This series is a cleanup, as well as fixing several places (mostly in
> nfsd) where mnt_{want,drop}_write() were missing.
> 
> It will also help with merging You Know What(*) security module, which
> needs to know the path within the namespace, and not just within the
> filesystem.  These helpers will allow the security hooks to be in a
> common place, and need not be repeated in all callers.

Rot.

Places in nfsd must be fixed, LSM hooks *moved* *to* *callers*.  And
really, by that point I absolutely do not give a damn for these clowns.
"Help with merging" implies that they can't be arsed to do _anything_
with their code.  Just as they could not be arsed to react to any
comments (including civil ones) in any manner except "wait for a month
and repost without changes".  Sod them.

And no, "make static where all (two of) current callers have vfsmount"
is non-starter.  path_...() is (at most) a convenience helper, not a
fundamental interface - simply because new callers should not be
forced into inventing a fake vfsmount just to use that.

I'll look into missing mnt_..._write() in nfsd and fix that.  The rest...
Sorry.

Re: [patch 01/10] vfs: add path_create() and path_mknod()

[Miklos Szeredi]

> > R/o bind mounts require operations which modify the filesystem to be
> > wrapped in mnt_want_write()/mnt_drop_write().  Create helpers which do
> > this, so callers won't need to bother, and more importantly, cannot
> > forget!  Call these path_*, analogous to vfs_*.  Where there are no
> > callers of vfs_* left, make them static.
> >
> > This series is a cleanup, as well as fixing several places (mostly in
> > nfsd) where mnt_{want,drop}_write() were missing.
> > 
> > It will also help with merging You Know What(*) security module, which
> > needs to know the path within the namespace, and not just within the
> > filesystem.  These helpers will allow the security hooks to be in a
> > common place, and need not be repeated in all callers.
> 
> Rot.
> 
> Places in nfsd must be fixed, LSM hooks *moved* *to* *callers*.  And
> really, by that point I absolutely do not give a damn for these clowns.
> "Help with merging" implies that they can't be arsed to do _anything_
> with their code.  Just as they could not be arsed to react to any
> comments (including civil ones) in any manner except "wait for a month
> and repost without changes".  Sod them.

Al, calm down.  I've been asked to help with merging this code, and if
there are concerns with it, I'll help fix them.  If you had bad
experience with it in the past, I'm sorry.  But let that not make this
any harder that in need to be.

> And no, "make static where all (two of) current callers have vfsmount"
> is non-starter.  path_...() is (at most) a convenience helper, not a
> fundamental interface - simply because new callers should not be
> forced into inventing a fake vfsmount just to use that.

It is an interface with at least 3 callers at the moment: syscalls,
nfsd and ecryptfs and unionfs in the future. It is an interface
because all external accesses to the filesystem *are* done through the
namespace and not directly on filesystem internals.

Such direct access would be conceivable, but I don't think we should
base the design on theoretically possible uses.  If those uses appear,
we can change the interface to fit that.

This "move everything to callers" thing is wrong because it just
results in bloat and bugs, none of which is desirable in the kernel.

Miklos

Re: [patch 01/10] vfs: add path_create() and path_mknod()

[Al Viro]

On Wed, Apr 02, 2008 at 11:11:39PM +0200, Miklos Szeredi wrote:

> It is an interface with at least 3 callers at the moment: syscalls,
> nfsd and ecryptfs and unionfs in the future. It is an interface
> because all external accesses to the filesystem *are* done through the
> namespace and not directly on filesystem internals.
> 
> Such direct access would be conceivable, but I don't think we should
> base the design on theoretically possible uses.  If those uses appear,
> we can change the interface to fit that.
> 
> This "move everything to callers" thing is wrong because it just
> results in bloat and bugs, none of which is desirable in the kernel.

I disagree.  First of all, clear separation between operations on
_filesystem_, which should all be namespace-agnostic and things
that depend on vfsmount is a Good Thing(tm).  Think of that as
of separation between server (superblock and everything related
to it, starting with dentry tree) and clients; mixing those is a
bloody bad idea.

What's more, I'm not at all convinced that for nfsd it's the right
set of checks, to start with.  The same goes for future users.

As for ecryptfs, looking at their "lower_mnt" thing...  I'd say that
it's a nonsense.  For one thing, duplicating a reference into ever
dentry out there (and it's simply duplicated) makes no sense whatsoever.
For another...  I'm not at all sure that remount of the underlying
vfsmount r/o *should* take that sucker read-only.  And if it should,
it's clearly an action that should have a visible effect on superblock
flags of ecryptfs.

	Incidentally, looking at ecryptfs open(), WTF is protecting
crypt_stat->flags?  We take crypt_stat->cs_mutex, do nothing but
check-and-modify of ->flags under it, then drop and several lines
later do crypt_stat->flags &= ~(ECRYPTFS_ENCRYPTED); with no ->cs_mutex
held...

Re: [patch 01/10] vfs: add path_create() and path_mknod()

[Trond Myklebust]

On Wed, 2008-04-02 at 22:48 +0100, Al Viro wrote:
> I disagree.  First of all, clear separation between operations on
> _filesystem_, which should all be namespace-agnostic and things
> that depend on vfsmount is a Good Thing(tm).  Think of that as
> of separation between server (superblock and everything related
> to it, starting with dentry tree) and clients; mixing those is a
> bloody bad idea.

Speaking of which: is there any reason why we can't get rid of the
vfsmount reference in struct file?

Most file operations, don't involve namespace traversal at all: aside
from fchdir(), and the *at() functions (all of which take file
descriptors, not pointers to struct file) the only function of that
vfsmount reference appears to be to prevent the superblock from going
away.

Cheers
  Trond

Re: [patch 01/10] vfs: add path_create() and path_mknod()

[Al Viro]

On Wed, Apr 02, 2008 at 06:21:30PM -0400, Trond Myklebust wrote:
> On Wed, 2008-04-02 at 22:48 +0100, Al Viro wrote:
> > I disagree.  First of all, clear separation between operations on
> > _filesystem_, which should all be namespace-agnostic and things
> > that depend on vfsmount is a Good Thing(tm).  Think of that as
> > of separation between server (superblock and everything related
> > to it, starting with dentry tree) and clients; mixing those is a
> > bloody bad idea.
> 
> Speaking of which: is there any reason why we can't get rid of the
> vfsmount reference in struct file?
> 
> Most file operations, don't involve namespace traversal at all: aside
> from fchdir(), and the *at() functions (all of which take file
> descriptors, not pointers to struct file) the only function of that
> vfsmount reference appears to be to prevent the superblock from going
> away.

Huh?  Are you proposing to move that to descriptor table, of all things?
Not to mention SCM_RIGHTS datagrams and hell knows what else...

Re: [patch 01/10] vfs: add path_create() and path_mknod()

[Trond Myklebust]

On Wed, 2008-04-02 at 23:36 +0100, Al Viro wrote:
> On Wed, Apr 02, 2008 at 06:21:30PM -0400, Trond Myklebust wrote:
> > On Wed, 2008-04-02 at 22:48 +0100, Al Viro wrote:
> > > I disagree.  First of all, clear separation between operations on
> > > _filesystem_, which should all be namespace-agnostic and things
> > > that depend on vfsmount is a Good Thing(tm).  Think of that as
> > > of separation between server (superblock and everything related
> > > to it, starting with dentry tree) and clients; mixing those is a
> > > bloody bad idea.
> > 
> > Speaking of which: is there any reason why we can't get rid of the
> > vfsmount reference in struct file?
> > 
> > Most file operations, don't involve namespace traversal at all: aside
> > from fchdir(), and the *at() functions (all of which take file
> > descriptors, not pointers to struct file) the only function of that
> > vfsmount reference appears to be to prevent the superblock from going
> > away.
> 
> Huh?  Are you proposing to move that to descriptor table, of all things?
> Not to mention SCM_RIGHTS datagrams and hell knows what else...

I'm just suggesting splitting out the namespace-specific part of struct
file into a separate structure that would be private to the VFS.
Something like

struct file_descriptor {
	struct file *file;
	struct vfsmount *mnt;
	atomic_t refcount;
};

and then having the 'struct file' hold a reference to the superblock
instead of holding a reference to the vfsmount.

Why would that be problematic for SCM_RIGHTS? We don't allow people to
send arbitrary references to 'struct file' using SCM_RIGHTS now; they
have to send descriptors.

Re: [patch 01/10] vfs: add path_create() and path_mknod()

[Al Viro]

On Wed, Apr 02, 2008 at 07:19:58PM -0400, Trond Myklebust wrote:

> I'm just suggesting splitting out the namespace-specific part of struct
> file into a separate structure that would be private to the VFS.
> Something like
> 
> struct file_descriptor {
> 	struct file *file;
> 	struct vfsmount *mnt;
> 	atomic_t refcount;
> };
> 
> and then having the 'struct file' hold a reference to the superblock
> instead of holding a reference to the vfsmount.
> 
> Why would that be problematic for SCM_RIGHTS? We don't allow people to
> send arbitrary references to 'struct file' using SCM_RIGHTS now; they
> have to send descriptors.

HUH?  Descriptor is a number.  There is no struct file_descriptor,
let alone refcounting for such.  There is a table, indexed by number
and containing references to struct file.

If you want to shove pointer to vfsmount in there (what for?  to waste
some memory and make SMP protection on access more interesting?), you
could do that, but IMO it's too ugly to consider.

Anyway, what the hell for?  It's more complex and buys you nothing
useful.

Re: [patch 01/10] vfs: add path_create() and path_mknod()

[Al Viro]

On Thu, Apr 03, 2008 at 12:40:43AM +0100, Al Viro wrote:
> Anyway, what the hell for?  It's more complex and buys you nothing
> useful.

BTW, what it gives you is an extra headache for scenarios like

task A:
	write(fd, ...)
task B:	/* shares descriptor table with A */
	close(fd)
task C: umount(<mountpoint of tree where fd is>)
...
task A: still writing

At the very least, you want "that thing is still busy" on normal umount -
we are still in the middle of write(2) and hell knows how long it's going
to last.  So you need to play with refcount of vfsmount in a very nasty
way, for all your pains.  Again, what for?

Re: [patch 01/10] vfs: add path_create() and path_mknod()

[Trond Myklebust]

On Thu, 2008-04-03 at 00:47 +0100, Al Viro wrote:
> On Thu, Apr 03, 2008 at 12:40:43AM +0100, Al Viro wrote:
> > Anyway, what the hell for?  It's more complex and buys you nothing
> > useful.
> 
> BTW, what it gives you is an extra headache for scenarios like
> 
> task A:
> 	write(fd, ...)
> task B:	/* shares descriptor table with A */
> 	close(fd)
> task C: umount(<mountpoint of tree where fd is>)
> ...
> task A: still writing
> 
> At the very least, you want "that thing is still busy" on normal umount -
> we are still in the middle of write(2) and hell knows how long it's going
> to last.  So you need to play with refcount of vfsmount in a very nasty
> way, for all your pains. 

We already call fget_light()/fput_light() around the whole call to
vfs_write(). Substituting a call to something which takes a reference to
the new structure is trivial.

Once the write() syscall is done, and the file is closed, why should we
refuse the umount request? There should be nothing more going on that
depends on the namespace.

>  Again, what for?

It allows you to get rid of the vfsmount 'argument' when opening a file,
which again lowers the barrier for stacking filesystems.

As far as the filesystems themselves are concerned, the effect is to
enforce your assertion that file operations should not depend on the
namespace.

Re: [patch 01/10] vfs: add path_create() and path_mknod()

[Erez Zadok]

In message <1207183329.20254.49.camel@heimdal.trondhjem.org>, Trond Myklebust writes:
> 
> On Thu, 2008-04-03 at 00:47 +0100, Al Viro wrote:
[...]
> >  Again, what for?
> 
> It allows you to get rid of the vfsmount 'argument' when opening a file,
> which again lowers the barrier for stacking filesystems.
> 
> As far as the filesystems themselves are concerned, the effect is to
> enforce your assertion that file operations should not depend on the
> namespace.

I'd be delighted the day I won't have to deal with vfsmounts AT ALL in
any stacked f/s.

Erez.

Re: [patch 01/10] vfs: add path_create() and path_mknod()

[Al Viro]

On Wed, Apr 02, 2008 at 08:47:06PM -0400, Erez Zadok wrote:
> In message <1207183329.20254.49.camel@heimdal.trondhjem.org>, Trond Myklebust writes:
> > 
> > On Thu, 2008-04-03 at 00:47 +0100, Al Viro wrote:
> [...]
> > >  Again, what for?
> > 
> > It allows you to get rid of the vfsmount 'argument' when opening a file,
> > which again lowers the barrier for stacking filesystems.
> > 
> > As far as the filesystems themselves are concerned, the effect is to
> > enforce your assertion that file operations should not depend on the
> > namespace.
> 
> I'd be delighted the day I won't have to deal with vfsmounts AT ALL in
> any stacked f/s.

vfsmounts or the fact that any fs may be mounted in many places?

Re: [patch 01/10] vfs: add path_create() and path_mknod()

[Erez Zadok]

In message <20080403010016.GU9785@ZenIV.linux.org.uk>, Al Viro writes:
> On Wed, Apr 02, 2008 at 08:47:06PM -0400, Erez Zadok wrote:
> > In message <1207183329.20254.49.camel@heimdal.trondhjem.org>, Trond Myklebust writes:
> > > 
> > > On Thu, 2008-04-03 at 00:47 +0100, Al Viro wrote:
> > [...]
> > > >  Again, what for?
> > > 
> > > It allows you to get rid of the vfsmount 'argument' when opening a file,
> > > which again lowers the barrier for stacking filesystems.
> > > 
> > > As far as the filesystems themselves are concerned, the effect is to
> > > enforce your assertion that file operations should not depend on the
> > > namespace.
> > 
> > I'd be delighted the day I won't have to deal with vfsmounts AT ALL in
> > any stacked f/s.
> 
> vfsmounts or the fact that any fs may be mounted in many places?

I think the former.  The fact that any f/s can be mounted in many places,
should be fine for a stacked f/s (topology changes as we discussed before
are a different problem).

Since you've hinted of major vfs changes post-25, here's my take.

Right now I (and to a similar extent ecryptfs too) needs to keep track of
vfsmounts for various reasons:

- to grab a reference so the lower filesystems/mounts won't disappear on me
- to pass it to dentry_open (opening the lower file)
- some fs ops pass a vfsmount (umount_begin, show_options)
- some fs ops or vfs helpers require a nameidata or struct path which embed
  a vfsmount inside
- sometimes it's ok to pass NULL for those things, sometimes it's not ok

Often it also appears that a vfsmount and a dentry tuple are needed
together, hence struct path.  So we recently started using struct path in a
few places, and using path_get/put.  The need to deal with <dentry,vfsmount>
pairs by hand can lead to interesting bugs, such as the recent ecryptfs
bugfix which had to swap the order of a dput() and mntput() sequence.

The fact that several vfs ops and helpers take different namespace-related
structures (dentry, nameidata, vfsmount, path), is confusing and hard to
track.  Is there a way to reduce the number of those structures to 1-2 at
most?

For a stackable f/s, it's important the the VFS ops *and* the VFS helpers
(vfs_*, path_*, dentry_open, etc.) use the same namespace structures: then
there's more symmetry b/t the objects passed to a stacked f/s, and the
objects it has to pass to VFS helpers (and FWIW, every such object should
ideally leave a "void *private" field for f/s specific extensibility).

In the longer run, is there a way that a stackable f/s could traverse the
namespace below it w/o knowing or caring how they are composed (assorted r-w
and r-o bind mounts and such)?

Cheers,
Erez.

Re: [patch 01/10] vfs: add path_create() and path_mknod()

[Al Viro]

On Wed, Apr 02, 2008 at 09:37:01PM -0400, Erez Zadok wrote:
> Since you've hinted of major vfs changes post-25, here's my take.
> 
> Right now I (and to a similar extent ecryptfs too) needs to keep track of
> vfsmounts for various reasons:
> 
> - to grab a reference so the lower filesystems/mounts won't disappear on me

Umm...  Strictly speaking that's not true; you can grab an active reference
to superblock and then superblock will stay.  vfsmount is usually more
convenient, but that's it.

> - to pass it to dentry_open (opening the lower file)
> - some fs ops pass a vfsmount (umount_begin, show_options)

Thank Trond for the former, BTW ;-)  Both methods will be back to sanity;
for umount_begin() the only obstacle is cifs mess, where we do unconditional
(and bogus) work at each umount(2).  With that fixed, we'll be back to
calling it only for forced umount and passing it only superblock.
->show_options() can and should revert immediately after 2.6.25; thanks
for pointing that one out.

> - some fs ops or vfs helpers require a nameidata or struct path which embed
>   a vfsmount inside

*ONLY* ->follow_link().  Which has to, by definition...  The rest will be
gone by .26.

> - sometimes it's ok to pass NULL for those things, sometimes it's not ok

See above.  This crap will be gone.  For ->follow_link() nobody is allowed
to pass NULL as nameidata, period.

> In the longer run, is there a way that a stackable f/s could traverse the
> namespace below it w/o knowing or caring how they are composed (assorted r-w
> and r-o bind mounts and such)?

Eh?  Explain, please...

Re: [patch 01/10] vfs: add path_create() and path_mknod()

[Erez Zadok]

In message <20080403014622.GW9785@ZenIV.linux.org.uk>, Al Viro writes:
> On Wed, Apr 02, 2008 at 09:37:01PM -0400, Erez Zadok wrote:
> > Since you've hinted of major vfs changes post-25, here's my take.
> > 
> > Right now I (and to a similar extent ecryptfs too) needs to keep track of
> > vfsmounts for various reasons:
> > 
> > - to grab a reference so the lower filesystems/mounts won't disappear on me
> 
> Umm...  Strictly speaking that's not true; you can grab an active reference
> to superblock and then superblock will stay.  vfsmount is usually more
> convenient, but that's it.

Yes, I do grab both vfsmount and superblock refs.  I found out that grabbing
vfsmount refs wasn't enough to prevent "umount -l" from detaching the f/s on
which I'm stacked on.  So now at mount time (or branch management time), I
grab those super-refs, as I have them after a successful path_lookup.  And,
since I keep a list of the branches I'm stacked on, I know precisely which
superblocks' references I need to release when unionfs is unmounted.

But what do I do if I descend into another lower superblock while looking up
a lower directory?  How do keep track of the superblock refs now?  I'd
basically have to memorize the hierarchy of mounted superblocks somehow?
How would I know when to release those refs?  (hmm, maybe I can rely on
d_mounted or the like?)

> > - sometimes it's ok to pass NULL for those things, sometimes it's not ok
> 
> See above.  This crap will be gone.  For ->follow_link() nobody is allowed
> to pass NULL as nameidata, period.

There's been talk in the past of splitting nameidata into intent structure
and all the rest.  Is that also part of your plan for 26?  Intents are
indeed very useful in ->lookup; the rest I can do without.

> > In the longer run, is there a way that a stackable f/s could traverse the
> > namespace below it w/o knowing or caring how they are composed (assorted r-w
> > and r-o bind mounts and such)?
> 
> Eh?  Explain, please...

So, in ->lookup, I essentially have to do a lookup on the corresponding
lower objects.  I get a nameidata, I have to create my own nameidata, and
pass it to lower ->lookup calls.  Can the API be changed so I wouldn't need
to get or pass a nameidata? (maybe just intent struct).

Also in ->lookup I call lookup_one_len, which is nice b/c it doesn't involve
vfsmounts or nameidata.  But lookup_one_len doesn't use the same prototype
as ->lookup, so there's some asymmetry here (I often like to see that a
stacked op passed to the lower f/s uses the same API as what was passed to
the op in the first place).

Ironically, since lookup_one_len doesn't involve vfsmounts, but I need them
for other reasons, I'm forced to live with NULL vfsmounts in some cases, or
refer to the lower vfsmounts I already had for my root dentry (that makes
transparently descending into a different vfsmount challenging, if not
inconsistent).

For many fs ops, the prototype of the ->op has a perfectly symmetric vfs_op()
helper.  For example, ->mkdir(inode,dentry,mode) and
vfs_mkdir(inode,dentry,mode).  But nothing as simple for lookup, one of the
most complex ops in unionfs.

BTW, to be honest, some of extra complications in unionfs (and
unionfs_lookup) have to do with .wh.* files (whiteouts).  But I hope that'll
get simpler once we have native WH support in the all the major filesystems.

Cheers,
Erez.

Re: [patch 01/10] vfs: add path_create() and path_mknod()

[Al Viro]

On Wed, Apr 02, 2008 at 10:21:24PM -0400, Erez Zadok wrote:
> Yes, I do grab both vfsmount and superblock refs.  I found out that grabbing
> vfsmount refs wasn't enough to prevent "umount -l" from detaching the f/s on
> which I'm stacked on.  So now at mount time (or branch management time), I
> grab those super-refs, as I have them after a successful path_lookup.  And,
> since I keep a list of the branches I'm stacked on, I know precisely which
> superblocks' references I need to release when unionfs is unmounted.

How the devil would holding a superblock prevent umount -l?
 
> But what do I do if I descend into another lower superblock while looking up
> a lower directory?  How do keep track of the superblock refs now?  I'd
> basically have to memorize the hierarchy of mounted superblocks somehow?
> How would I know when to release those refs?  (hmm, maybe I can rely on
> d_mounted or the like?)
>
> > > - sometimes it's ok to pass NULL for those things, sometimes it's not ok
> > 
> > See above.  This crap will be gone.  For ->follow_link() nobody is allowed
> > to pass NULL as nameidata, period.
> 
> There's been talk in the past of splitting nameidata into intent structure
> and all the rest.  Is that also part of your plan for 26?  Intents are
> indeed very useful in ->lookup; the rest I can do without.

intents will die.  There'll be a method for final step of lookup + open,
but that's it (and it'll take preallocated struct file as one of the
arguments).  Please, explain what you want to do with intents, because
as far as I'm concerned these had been a mistake for a lot of reasons.

> Ironically, since lookup_one_len doesn't involve vfsmounts, but I need them
> for other reasons, I'm forced to live with NULL vfsmounts in some cases, or
> refer to the lower vfsmounts I already had for my root dentry (that makes
> transparently descending into a different vfsmount challenging, if not
> inconsistent).

Details, please.  If you just want a snapshot of vfsmount tree, then by
all means take a bloody snapshot.  collect_mounts() is there for purpose.
If you want mount/umount/etc. changes affect what you have, then I really
would like to see the semantics you want.  Some variation on shared-subtree
might be close to that...

Re: [patch 01/10] vfs: add path_create() and path_mknod()

[Erez Zadok]

In message <20080403023239.GY9785@ZenIV.linux.org.uk>, Al Viro writes:
> On Wed, Apr 02, 2008 at 10:21:24PM -0400, Erez Zadok wrote:
> > Yes, I do grab both vfsmount and superblock refs.  I found out that grabbing
> > vfsmount refs wasn't enough to prevent "umount -l" from detaching the f/s on
> > which I'm stacked on.  So now at mount time (or branch management time), I
> > grab those super-refs, as I have them after a successful path_lookup.  And,
> > since I keep a list of the branches I'm stacked on, I know precisely which
> > superblocks' references I need to release when unionfs is unmounted.
> 
> How the devil would holding a superblock prevent umount -l?

It doesn't prevent umount -l; but it prevents the lower superblock from
being kfree()d if there were no references left to it, so I don't try to
accessed freed memory (and get 6b6b6b6b oopses :-)

> > But what do I do if I descend into another lower superblock while looking up
> > a lower directory?  How do keep track of the superblock refs now?  I'd
> > basically have to memorize the hierarchy of mounted superblocks somehow?
> > How would I know when to release those refs?  (hmm, maybe I can rely on
> > d_mounted or the like?)
> >
> > > > - sometimes it's ok to pass NULL for those things, sometimes it's not ok
> > > 
> > > See above.  This crap will be gone.  For ->follow_link() nobody is allowed
> > > to pass NULL as nameidata, period.
> > 
> > There's been talk in the past of splitting nameidata into intent structure
> > and all the rest.  Is that also part of your plan for 26?  Intents are
> > indeed very useful in ->lookup; the rest I can do without.
> 
> intents will die.  There'll be a method for final step of lookup + open,
> but that's it (and it'll take preallocated struct file as one of the
> arguments).

How much of nameidata will still need to be passed to f/s ops?  How many vfs
helpers will still require a nameidata?  Hopefully as few as possible.

Is this preallocated struct file a replacement to the nd->intent.open.file
that could have returned an allocated struct file?  (I could never really
tell what's the right thing to do with that field.)

> Please, explain what you want to do with intents, because
> as far as I'm concerned these had been a mistake for a lot of reasons.

I said above "intents are indeed very useful" only because they were:

1. required to be passed to vfs_* methods, which made it hard if the ->op I
   was in didn't have an intent (I have to create temp intents for those).

2. apparently heavily used in nfs4, to a point where if I stacked on nfs4
   and didn't pass a correctly formatted nameidata, I'd get an oops deep
   inside nfs4 code.  (I think some of those nfs4 requirements went away,
   not sure what's left.).

> > Ironically, since lookup_one_len doesn't involve vfsmounts, but I need them
> > for other reasons, I'm forced to live with NULL vfsmounts in some cases, or
> > refer to the lower vfsmounts I already had for my root dentry (that makes
> > transparently descending into a different vfsmount challenging, if not
> > inconsistent).
> 
> Details, please.  If you just want a snapshot of vfsmount tree, then by
> all means take a bloody snapshot.  collect_mounts() is there for purpose.
> If you want mount/umount/etc. changes affect what you have, then I really
> would like to see the semantics you want.  Some variation on shared-subtree
> might be close to that...

I'll have to take a closer look at vfsmount tree snapshotting and
collect_mounts() before I can say how useful they'd be.  But if you recall
my questions at LSF, I asked whether it was possible for me to create a
readonly directory tree (e.g., r-o bind mounts) or some form of an immutable
namespace that no one can modify underneath me.  You said that r-o bind
mounts were not intended for that.

Right now I'm allowing users to modify lower branches, with all the pros and
cons that it has.  But even if I wanted to prevent users from touching any
lower files below a certain directory, while allowing only unionfs to modify
those files, it doesn't appear that there's something available I could use.


I have two other questions/requests:

1. The less I have to use or know about vfsmounts and nameidata/intents, the
   better.  But whatever API changes you make, please consider the symmetry
   between the f/s ->op I'm called with, and the vfs helpers I might use in
   the ->op to pass through to the lower f/s.  Ideally the prototypes of the
   ->op and vfs helpers be identical, so I don't have to work too hard to
   locate the lower objects, or worse, having to make them up temporarily.

2. You mentioned that all this work is scheduled for 26.  25 is nearing
   release.  Do you have code already that I can experiment with?  A preview
   of things to come?  Maybe an example or two of how a filesystem
   (stackable, nfsd, or otherwise) should lookup and open arbitrary files?
   What you mentioned in this discussion thread sounds promising and
   exciting, but may take me a while to apply to my tree (longer than the
   usual merge window, which reportedly will shrink even further, now that
   we have linux-next :-).  So the more lead time, the better, please.

Thanks,
Erez.

Re: [patch 01/10] vfs: add path_create() and path_mknod()

[Miklos Szeredi]

> In message <20080403023239.GY9785@ZenIV.linux.org.uk>, Al Viro writes:
> > On Wed, Apr 02, 2008 at 10:21:24PM -0400, Erez Zadok wrote:
> > > Yes, I do grab both vfsmount and superblock refs.  I found out that grabbing
> > > vfsmount refs wasn't enough to prevent "umount -l" from detaching the f/s on
> > > which I'm stacked on.  So now at mount time (or branch management time), I
> > > grab those super-refs, as I have them after a successful path_lookup.  And,
> > > since I keep a list of the branches I'm stacked on, I know precisely which
> > > superblocks' references I need to release when unionfs is unmounted.
> > 
> > How the devil would holding a superblock prevent umount -l?
> 
> It doesn't prevent umount -l; but it prevents the lower superblock from
> being kfree()d if there were no references left to it, so I don't try to
> accessed freed memory (and get 6b6b6b6b oopses :-)

Having a ref on the vfsmount should be enough, regardless of whether
the mount is attached or not.  There's nothing special about a
detached mount, that anything outside namespace.c should care about.

Miklos

Re: [patch 01/10] vfs: add path_create() and path_mknod()

[Al Viro]

On Wed, Apr 02, 2008 at 08:42:09PM -0400, Trond Myklebust wrote:

> > At the very least, you want "that thing is still busy" on normal umount -
> > we are still in the middle of write(2) and hell knows how long it's going
> > to last.  So you need to play with refcount of vfsmount in a very nasty
> > way, for all your pains. 
> 
> We already call fget_light()/fput_light() around the whole call to
> vfs_write(). Substituting a call to something which takes a reference to
> the new structure is trivial.

Huh?  So you want an extra layer of indirection?  descriptor table -> that
one -> struct file?  And refcounting these puppies?
 
> It allows you to get rid of the vfsmount 'argument' when opening a file,
> which again lowers the barrier for stacking filesystems.

I don't see how that would fix the fundamental breakage in those, but
anyway... (and yes, ecryptfs has interesting issues, but the look of it).

> As far as the filesystems themselves are concerned, the effect is to
> enforce your assertion that file operations should not depend on the
> namespace.

I really doubt that it's worth doing in this area...  "Don't use ->f_vfsmnt
in fs code" is easily enforced and struct file is really used outside of
filesystem code in fs-independent ways.

IOW, I don't believe that it's worth introducing a new layer between descriptor
table and files.  BTW, that'll complicate union-mount handling (real ones, not
unionfs under different name) and quite a few other things...

Re: [patch 01/10] vfs: add path_create() and path_mknod()

[Miklos Szeredi]

> > It is an interface with at least 3 callers at the moment: syscalls,
> > nfsd and ecryptfs and unionfs in the future. It is an interface
> > because all external accesses to the filesystem *are* done through the
> > namespace and not directly on filesystem internals.
> > 
> > Such direct access would be conceivable, but I don't think we should
> > base the design on theoretically possible uses.  If those uses appear,
> > we can change the interface to fit that.
> > 
> > This "move everything to callers" thing is wrong because it just
> > results in bloat and bugs, none of which is desirable in the kernel.
> 
> I disagree.  First of all, clear separation between operations on
> _filesystem_, which should all be namespace-agnostic and things
> that depend on vfsmount is a Good Thing(tm).  Think of that as
> of separation between server (superblock and everything related
> to it, starting with dentry tree) and clients; mixing those is a
> bloody bad idea.

Separation: yes.  Exporting it as an interface: I'm not convinced.

If we ever want to rely on r/o mounts doing what they're supposed to
do, ie. not allowing modifications to the filesystem, we'd better
expose an interface that does just that.  Relying on callers won't
work (as demonstrated in practice).

Wasn't one of the points behind the r/o bind patchset is to make
remounting read-only race free?  If all mounts of a super block are
read-only, then the super block itself can be safely marked read-only.

If we allow external entities such as filesystem stacks to bypass this
rule and access the filesystem directly, then the whole thing is
basically worthless.

> What's more, I'm not at all convinced that for nfsd it's the right
> set of checks, to start with.

Well, what are the right checks then?  From the r/o consistency pov,
these are the right checks.  From NFS' pov there may be places where
we might want to take a write-ref spanning several operations.  But
that's OK, the checks are cheap (if not, we have other problems).

>  The same goes for future users.

There's already one place in that interface where users need to know
the vfsmount which they are operating on: opening a file (and hence
subsequent I/O on that file).  Besides my other points, what is the
sense in an interface, half of which operates on the namespace, and
the other half on the filesystem?

> As for ecryptfs, looking at their "lower_mnt" thing...  I'd say that
> it's a nonsense.  For one thing, duplicating a reference into ever
> dentry out there (and it's simply duplicated) makes no sense whatsoever.
> For another...  I'm not at all sure that remount of the underlying
> vfsmount r/o *should* take that sucker read-only.  And if it should,
> it's clearly an action that should have a visible effect on superblock
> flags of ecryptfs.

If it wants to handle that case nicely, it can monitor /proc/mounts
and reflect it in it's superblock flags.  And it can take a write-ref
on the underlying fs if it has outstanding dirtyness.  But we should
not _rely_ on ecryptfs to ensure that it's never writing to a
read-only mount.

Miklos

Re: [patch 01/10] vfs: add path_create() and path_mknod()

[Erez Zadok]

In message <E1JhJwR-0008Vv-2x@pomaz-ex.szeredi.hu>, Miklos Szeredi writes:
[...]
> If it wants to handle that case nicely, it can monitor /proc/mounts
> and reflect it in it's superblock flags.  And it can take a write-ref
> on the underlying fs if it has outstanding dirtyness.  But we should
> not _rely_ on ecryptfs to ensure that it's never writing to a
> read-only mount.

I'm all for reducing the need for stackable f/s such as ecryptfs from having
to remember to manage the write counts.  A stackable filesystem has to
emulate some parts of the VFS when it calls into a lower f/s: in that case,
having useful VFS helpers that do as much of the work as possible, saves a
lot of hassle.

> Miklos

Cheers,
Erez.

Re: [patch 01/10] vfs: add path_create() and path_mknod()

[Stephen Smalley]

On Wed, 2008-04-02 at 21:54 +0100, Al Viro wrote:
> On Wed, Apr 02, 2008 at 10:12:48PM +0200, Miklos Szeredi wrote:
> > From: Miklos Szeredi <mszeredi@suse.cz>
> > 
> > R/o bind mounts require operations which modify the filesystem to be
> > wrapped in mnt_want_write()/mnt_drop_write().  Create helpers which do
> > this, so callers won't need to bother, and more importantly, cannot
> > forget!  Call these path_*, analogous to vfs_*.  Where there are no
> > callers of vfs_* left, make them static.
> >
> > This series is a cleanup, as well as fixing several places (mostly in
> > nfsd) where mnt_{want,drop}_write() were missing.
> > 
> > It will also help with merging You Know What(*) security module, which
> > needs to know the path within the namespace, and not just within the
> > filesystem.  These helpers will allow the security hooks to be in a
> > common place, and need not be repeated in all callers.
> 
> Rot.
> 
> Places in nfsd must be fixed, LSM hooks *moved* *to* *callers*.

Please don't move the existing security_inode hooks - they are at
exactly the right location for SELinux and likely other security modules
too - after the DAC checks (so no noise in the audit logs from accesses
that would be denied by DAC anyway), and right before the inode method
is invoked (so easy to see that the permission check is always invoked
before access).  Moving the existing hooks will break the first property
unless the DAC checks are also moved and will make it harder to check
and maintain the second property.

>   And
> really, by that point I absolutely do not give a damn for these clowns.
> "Help with merging" implies that they can't be arsed to do _anything_
> with their code.  Just as they could not be arsed to react to any
> comments (including civil ones) in any manner except "wait for a month
> and repost without changes".  Sod them.
> 
> And no, "make static where all (two of) current callers have vfsmount"
> is non-starter.  path_...() is (at most) a convenience helper, not a
> fundamental interface - simply because new callers should not be
> forced into inventing a fake vfsmount just to use that.
> 
> I'll look into missing mnt_..._write() in nfsd and fix that.  The rest...
> Sorry.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
-- 
Stephen Smalley
National Security Agency

Re: [patch 01/10] vfs: add path_create() and path_mknod()

[Dave Hansen]

On Wed, 2008-04-02 at 22:12 +0200, Miklos Szeredi wrote:
> This series is a cleanup, as well as fixing several places (mostly in
> nfsd) where mnt_{want,drop}_write() were missing.

Could you elaborate on these?

-- Dave

Re: [patch 01/10] vfs: add path_create() and path_mknod()

[Dave Hansen]

I think these definitely all clean the code up.  They might add a couple
of lines, net, but they certainly make it easier to read.  As you noted,
they also make it much harder to screw up the locking, which is even
better.  Just looking at it from an aesthetic point of view, it looks
good to have these helpers around.

-- Dave