[patch 0/8] vfs: more cleanups and fixes

[patch 0/8] vfs: more cleanups and fixes

[Miklos Szeredi]

Various cleanups and fixes.

This series is based on the vfs-cleanups(*) tree.  If no problems are
found, I'll commit the patches to that tree.

Thanks,
Miklos

(*) git://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/vfs.git vfs-cleanups

--

[patch 1/8] vfs: dcache cleanups

[Miklos Szeredi]

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

From: Miklos Szeredi <mszeredi@suse.cz>

d_path() cleanups and fix the following sparse warnings:

fs/dcache.c:2183:19: warning: symbol 'filp_cachep' was not declared. Should it be static?
fs/dcache.c:115:3: warning: context imbalance in 'dentry_iput' - unexpected unlock
fs/dcache.c:188:2: warning: context imbalance in 'dput' - different lock contexts for basic block
fs/dcache.c:400:2: warning: context imbalance in 'prune_one_dentry' - different lock contexts for basic block
fs/dcache.c:431:22: warning: context imbalance in 'prune_dcache' - different lock contexts for basic block
fs/dcache.c:563:2: warning: context imbalance in 'shrink_dcache_sb' - different lock contexts for basic block
fs/dcache.c:1385:6: warning: context imbalance in 'd_delete' - wrong count at exit
fs/dcache.c:1636:2: warning: context imbalance in '__d_unalias' - unexpected unlock
fs/dcache.c:1735:2: warning: context imbalance in 'd_materialise_unique' - different lock contexts for basic block

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---
 fs/dcache.c |   51 +++++++++++++++++++++++++--------------------------
 1 file changed, 25 insertions(+), 26 deletions(-)

Index: linux-2.6/fs/dcache.c
===================================================================
--- linux-2.6.orig/fs/dcache.c	2008-05-29 12:46:20.000000000 +0200
+++ linux-2.6/fs/dcache.c	2008-05-29 12:46:22.000000000 +0200
@@ -17,6 +17,7 @@
 #include <linux/syscalls.h>
 #include <linux/string.h>
 #include <linux/mm.h>
+#include <linux/fdtable.h>
 #include <linux/fs.h>
 #include <linux/fsnotify.h>
 #include <linux/slab.h>
@@ -106,9 +107,10 @@ static void dentry_lru_remove(struct den
 /*
  * Release the dentry's inode, using the filesystem
  * d_iput() operation if defined.
- * Called with dcache_lock and per dentry lock held, drops both.
  */
 static void dentry_iput(struct dentry * dentry)
+	__releases(dentry->d_lock)
+	__releases(dcache_lock)
 {
 	struct inode *inode = dentry->d_inode;
 	if (inode) {
@@ -132,12 +134,13 @@ static void dentry_iput(struct dentry * 
  * d_kill - kill dentry and return parent
  * @dentry: dentry to kill
  *
- * Called with dcache_lock and d_lock, releases both.  The dentry must
- * already be unhashed and removed from the LRU.
+ * The dentry must already be unhashed and removed from the LRU.
  *
  * If this is the root of the dentry tree, return NULL.
  */
 static struct dentry *d_kill(struct dentry *dentry)
+	__releases(dentry->d_lock)
+	__releases(dcache_lock)
 {
 	struct dentry *parent;
 
@@ -383,11 +386,11 @@ restart:
  * Try to prune ancestors as well.  This is necessary to prevent
  * quadratic behavior of shrink_dcache_parent(), but is also expected
  * to be beneficial in reducing dentry cache fragmentation.
- *
- * Called with dcache_lock, drops it and then regains.
- * Called with dentry->d_lock held, drops it.
  */
 static void prune_one_dentry(struct dentry * dentry)
+	__releases(dentry->d_lock)
+	__releases(dcache_lock)
+	__acquires(dcache_lock)
 {
 	__d_drop(dentry);
 	dentry = d_kill(dentry);
@@ -1604,10 +1607,9 @@ static int d_isparent(struct dentry *p1,
  *
  * Note: If ever the locking in lock_rename() changes, then please
  * remember to update this too...
- *
- * On return, dcache_lock will have been unlocked.
  */
 static struct dentry *__d_unalias(struct dentry *dentry, struct dentry *alias)
+	__releases(dcache_lock)
 {
 	struct mutex *m1 = NULL, *m2 = NULL;
 	struct dentry *ret;
@@ -1743,7 +1745,6 @@ out_nolock:
 shouldnt_be_hashed:
 	spin_unlock(&dcache_lock);
 	BUG();
-	goto shouldnt_be_hashed;
 }
 
 static int prepend(char **buffer, int *buflen, const char *str,
@@ -1758,7 +1759,7 @@ static int prepend(char **buffer, int *b
 }
 
 /**
- * d_path - return the path of a dentry
+ * __d_path - return the path of a dentry
  * @path: the dentry/vfsmount to report
  * @root: root vfsmnt/dentry (may be modified by this function)
  * @buffer: buffer to return value in
@@ -1779,8 +1780,9 @@ char *__d_path(const struct path *path, 
 {
 	struct dentry *dentry = path->dentry;
 	struct vfsmount *vfsmnt = path->mnt;
-	char * end = buffer+buflen;
-	char * retval;
+	char *end = buffer + buflen;
+	char *retval;
+	struct qstr *name;
 
 	prepend(&end, &buflen, "\0", 1);
 	if (!IS_ROOT(dentry) && d_unhashed(dentry) &&
@@ -1812,8 +1814,8 @@ char *__d_path(const struct path *path, 
 		}
 		parent = dentry->d_parent;
 		prefetch(parent);
-		if ((prepend(&end, &buflen, dentry->d_name.name,
-				dentry->d_name.len) != 0) ||
+		name = &dentry->d_name;
+		if ((prepend(&end, &buflen, name->name, name->len) != 0) ||
 		    (prepend(&end, &buflen, "/", 1) != 0))
 			goto Elong;
 		retval = end;
@@ -1824,8 +1826,8 @@ char *__d_path(const struct path *path, 
 
 global_root:
 	retval += 1;	/* hit the slash */
-	if (prepend(&retval, &buflen, dentry->d_name.name,
-		    dentry->d_name.len) != 0)
+	name = &dentry->d_name;
+	if (prepend(&retval, &buflen, name->name, name->len) != 0)
 		goto Elong;
 	root->mnt = vfsmnt;
 	root->dentry = dentry;
@@ -1845,7 +1847,7 @@ Elong:
  *
  * Returns the buffer or an error code if the path was too long.
  *
- * "buflen" should be positive. Caller holds the dcache_lock.
+ * "buflen" should be positive.
  */
 char *d_path(struct path *path, char *buf, int buflen)
 {
@@ -1915,16 +1917,13 @@ char *dentry_path(struct dentry *dentry,
 	retval = end-1;
 	*retval = '/';
 
-	for (;;) {
-		struct dentry *parent;
-		if (IS_ROOT(dentry))
-			break;
+	while (!IS_ROOT(dentry)) {
+		struct dentry *parent = dentry->d_parent;
+		struct qstr *name;
 
-		parent = dentry->d_parent;
 		prefetch(parent);
-
-		if ((prepend(&end, &buflen, dentry->d_name.name,
-				dentry->d_name.len) != 0) ||
+		name = &dentry->d_name;
+		if ((prepend(&end, &buflen, name->name, name->len) != 0) ||
 		    (prepend(&end, &buflen, "/", 1) != 0))
 			goto Elong;
 
@@ -1975,7 +1974,7 @@ asmlinkage long sys_getcwd(char __user *
 	error = -ENOENT;
 	/* Has the current directory has been unlinked? */
 	spin_lock(&dcache_lock);
-	if (pwd.dentry->d_parent == pwd.dentry || !d_unhashed(pwd.dentry)) {
+	if (IS_ROOT(pwd.dentry) || !d_unhashed(pwd.dentry)) {
 		unsigned long len;
 		struct path tmp = root;
 		char * cwd;

--

Re: [patch 1/8] vfs: dcache cleanups

[Matthew Wilcox]

On Thu, May 29, 2008 at 01:32:46PM +0200, Miklos Szeredi wrote:
> d_path() cleanups and fix the following sparse warnings:
> 
> fs/dcache.c:2183:19: warning: symbol 'filp_cachep' was not declared. Should it be static?
> fs/dcache.c:115:3: warning: context imbalance in 'dentry_iput' - unexpected unlock
> fs/dcache.c:188:2: warning: context imbalance in 'dput' - different lock contexts for basic block
> fs/dcache.c:400:2: warning: context imbalance in 'prune_one_dentry' - different lock contexts for basic block
> fs/dcache.c:431:22: warning: context imbalance in 'prune_dcache' - different lock contexts for basic block
> fs/dcache.c:563:2: warning: context imbalance in 'shrink_dcache_sb' - different lock contexts for basic block
> fs/dcache.c:1385:6: warning: context imbalance in 'd_delete' - wrong count at exit
> fs/dcache.c:1636:2: warning: context imbalance in '__d_unalias' - unexpected unlock
> fs/dcache.c:1735:2: warning: context imbalance in 'd_materialise_unique' - different lock contexts for basic block
> 
> Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>

Reviewed-by: Matthew Wilcox <willy@linux.intel.com>

-- 
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."

Re: [patch 1/8] vfs: dcache cleanups

[Christoph Hellwig]

Looks good, although mixing up the sparse warning fixes and the d_path
cleanup in one patch is a little odd.

[patch 2/8] vfs: fix sys_getcwd for detached mounts

[Miklos Szeredi]

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

From: Miklos Szeredi <mszeredi@suse.cz>

Currently getcwd(2) on a detached mount will give a garbled result:

   > mkdir /mnt/foo
   > mount --bind /etc /mnt/foo
   > cd /mnt/foo/skel
   > /bin/pwd
   /mnt/foo/skel
   > umount -l /mnt/foo
   > /bin/pwd
   etcskel

After the patch it will give a much saner "/skel" result.

Thanks to John Johansen for pointing out this bug.

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
CC: John Johansen <jjohansen@suse.de>
---
 fs/dcache.c |   18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

Index: linux-2.6/fs/dcache.c
===================================================================
--- linux-2.6.orig/fs/dcache.c	2008-05-29 12:46:22.000000000 +0200
+++ linux-2.6/fs/dcache.c	2008-05-29 12:46:22.000000000 +0200
@@ -1825,10 +1825,20 @@ char *__d_path(const struct path *path, 
 	return retval;
 
 global_root:
-	retval += 1;	/* hit the slash */
-	name = &dentry->d_name;
-	if (prepend(&retval, &buflen, name->name, name->len) != 0)
-		goto Elong;
+	/*
+	 * If this is a root dentry, then overwrite the slash.  This
+	 * will also DTRT with pseudo filesystems which have root
+	 * dentries named "foo:".
+	 *
+	 * Otherwise this is the root of a detached mount, so don't do
+	 * anything.
+	 */
+	if (IS_ROOT(dentry)) {
+		retval += 1;
+		name = &dentry->d_name;
+		if (prepend(&retval, &buflen, name->name, name->len) != 0)
+			goto Elong;
+	}
 	root->mnt = vfsmnt;
 	root->dentry = dentry;
 	return retval;

--

Re: [patch 2/8] vfs: fix sys_getcwd for detached mounts

[Christoph Hellwig]

On Thu, May 29, 2008 at 01:32:47PM +0200, Miklos Szeredi wrote:
> From: Miklos Szeredi <mszeredi@suse.cz>
> 
> Currently getcwd(2) on a detached mount will give a garbled result:
> 
>    > mkdir /mnt/foo
>    > mount --bind /etc /mnt/foo
>    > cd /mnt/foo/skel
>    > /bin/pwd
>    /mnt/foo/skel
>    > umount -l /mnt/foo
>    > /bin/pwd
>    etcskel
> 
> After the patch it will give a much saner "/skel" result.
> 
> Thanks to John Johansen for pointing out this bug.

I'm not sure just cuttinig of the prefix is perfect, but the previous
behvaiour is obviously completely wrong, and the change is simple
enough.  So okay from me.

[patch 3/8] Make d_path() consistent across mount operations

[Miklos Szeredi]

[-- Attachment #1: mount-consistent-__d_path.diff --]
[-- Type: text/plain, Size: 2027 bytes --]

The path that __d_path() computes can become slightly inconsistent when it
races with mount operations: it grabs the vfsmount_lock when traversing mount
points but immediately drops it again, only to re-grab it when it reaches the
next mount point.  The result is that the filename computed is not always
consisent, and the file may never have had that name. (This is unlikely, but
still possible.)

Fix this by grabbing the vfsmount_lock for the whole duration of
__d_path().

Signed-off-by: Andreas Gruenbacher <agruen@suse.de>
Signed-off-by: John Johansen <jjohansen@suse.de>
Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---
 fs/dcache.c |   12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

Index: linux-2.6/fs/dcache.c
===================================================================
--- linux-2.6.orig/fs/dcache.c	2008-05-29 12:46:22.000000000 +0200
+++ linux-2.6/fs/dcache.c	2008-05-29 12:46:23.000000000 +0200
@@ -1784,6 +1784,7 @@ char *__d_path(const struct path *path, 
 	char *retval;
 	struct qstr *name;
 
+	spin_lock(&vfsmount_lock);
 	prepend(&end, &buflen, "\0", 1);
 	if (!IS_ROOT(dentry) && d_unhashed(dentry) &&
 		(prepend(&end, &buflen, " (deleted)", 10) != 0))
@@ -1802,14 +1803,11 @@ char *__d_path(const struct path *path, 
 			break;
 		if (dentry == vfsmnt->mnt_root || IS_ROOT(dentry)) {
 			/* Global root? */
-			spin_lock(&vfsmount_lock);
 			if (vfsmnt->mnt_parent == vfsmnt) {
-				spin_unlock(&vfsmount_lock);
 				goto global_root;
 			}
 			dentry = vfsmnt->mnt_mountpoint;
 			vfsmnt = vfsmnt->mnt_parent;
-			spin_unlock(&vfsmount_lock);
 			continue;
 		}
 		parent = dentry->d_parent;
@@ -1822,6 +1820,8 @@ char *__d_path(const struct path *path, 
 		dentry = parent;
 	}
 
+out:
+	spin_unlock(&vfsmount_lock);
 	return retval;
 
 global_root:
@@ -1841,9 +1841,11 @@ global_root:
 	}
 	root->mnt = vfsmnt;
 	root->dentry = dentry;
-	return retval;
+	goto out;
+
 Elong:
-	return ERR_PTR(-ENAMETOOLONG);
+	retval = ERR_PTR(-ENAMETOOLONG);
+	goto out;
 }
 
 /**

--

Re: [patch 3/8] Make d_path() consistent across mount operations

[Christoph Hellwig]

On Thu, May 29, 2008 at 01:32:48PM +0200, Miklos Szeredi wrote:
> The path that __d_path() computes can become slightly inconsistent when it
> races with mount operations: it grabs the vfsmount_lock when traversing mount
> points but immediately drops it again, only to re-grab it when it reaches the
> next mount point.  The result is that the filename computed is not always
> consisent, and the file may never have had that name. (This is unlikely, but
> still possible.)
> 
> Fix this by grabbing the vfsmount_lock for the whole duration of
> __d_path().

Looks good, and lock holding times shouldn't be a problem either.

Re: [patch 3/8] Make d_path() consistent across mount operations

[Miklos Szeredi]

> > The path that __d_path() computes can become slightly inconsistent when it
> > races with mount operations: it grabs the vfsmount_lock when traversing mount
> > points but immediately drops it again, only to re-grab it when it reaches the
> > next mount point.  The result is that the filename computed is not always
> > consisent, and the file may never have had that name. (This is unlikely, but
> > still possible.)
> > 
> > Fix this by grabbing the vfsmount_lock for the whole duration of
> > __d_path().
> 
> Looks good, and lock holding times shouldn't be a problem either.

Thanks for the review of this batch.

Can you please in the future either explicitly ACK or NACK?  Because I
really wouldn't want any more of this "Looks good, but bla bla bla"
and then NACKing the patch when I send it off to Andrew.

Miklos

Re: [patch 3/8] Make d_path() consistent across mount operations

[Christoph Hellwig]

On Sun, Jun 01, 2008 at 10:38:35PM +0200, Miklos Szeredi wrote:
> > > The path that __d_path() computes can become slightly inconsistent when it
> > > races with mount operations: it grabs the vfsmount_lock when traversing mount
> > > points but immediately drops it again, only to re-grab it when it reaches the
> > > next mount point.  The result is that the filename computed is not always
> > > consisent, and the file may never have had that name. (This is unlikely, but
> > > still possible.)
> > > 
> > > Fix this by grabbing the vfsmount_lock for the whole duration of
> > > __d_path().
> > 
> > Looks good, and lock holding times shouldn't be a problem either.
> 
> Thanks for the review of this batch.
> 
> Can you please in the future either explicitly ACK or NACK?  Because I
> really wouldn't want any more of this "Looks good, but bla bla bla"
> and then NACKing the patch when I send it off to Andrew.

ACK for patches 1-3.

[patch 4/8] nfsd: rename MAY_ flags

[Miklos Szeredi]

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

From: Miklos Szeredi <mszeredi@suse.cz>

Rename nfsd specific MAY_* flags to NFSD_MAY_* to make it clear, that
these are not used outside nfsd, and to avoid namespace conflicts with
the VFS.

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
CC: J. Bruce Fields <bfields@citi.umich.edu>
CC: NeilBrown <neilb@suse.de>
---
 fs/nfsd/lockd.c           |    2 -
 fs/nfsd/nfs2acl.c         |    7 ++--
 fs/nfsd/nfs3acl.c         |    5 +--
 fs/nfsd/nfs3proc.c        |    8 ++---
 fs/nfsd/nfs4proc.c        |   19 +++++++-----
 fs/nfsd/nfs4state.c       |    2 -
 fs/nfsd/nfsfh.c           |    2 -
 fs/nfsd/nfsproc.c         |    4 +-
 fs/nfsd/vfs.c             |   71 +++++++++++++++++++++++-----------------------
 include/linux/nfsd/nfsd.h |   20 ++++++------
 10 files changed, 73 insertions(+), 67 deletions(-)

Index: linux-2.6/fs/nfsd/nfs2acl.c
===================================================================
--- linux-2.6.orig/fs/nfsd/nfs2acl.c	2008-05-29 12:46:20.000000000 +0200
+++ linux-2.6/fs/nfsd/nfs2acl.c	2008-05-29 12:46:23.000000000 +0200
@@ -40,7 +40,8 @@ static __be32 nfsacld_proc_getacl(struct
 	dprintk("nfsd: GETACL(2acl)   %s\n", SVCFH_fmt(&argp->fh));
 
 	fh = fh_copy(&resp->fh, &argp->fh);
-	if ((nfserr = fh_verify(rqstp, &resp->fh, 0, MAY_NOP)))
+	nfserr = fh_verify(rqstp, &resp->fh, 0, NFSD_MAY_NOP);
+	if (nfserr)
 		RETURN_STATUS(nfserr);
 
 	if (argp->mask & ~(NFS_ACL|NFS_ACLCNT|NFS_DFACL|NFS_DFACLCNT))
@@ -107,7 +108,7 @@ static __be32 nfsacld_proc_setacl(struct
 	dprintk("nfsd: SETACL(2acl)   %s\n", SVCFH_fmt(&argp->fh));
 
 	fh = fh_copy(&resp->fh, &argp->fh);
-	nfserr = fh_verify(rqstp, &resp->fh, 0, MAY_SATTR);
+	nfserr = fh_verify(rqstp, &resp->fh, 0, NFSD_MAY_SATTR);
 
 	if (!nfserr) {
 		nfserr = nfserrno( nfsd_set_posix_acl(
@@ -134,7 +135,7 @@ static __be32 nfsacld_proc_getattr(struc
 	dprintk("nfsd: GETATTR  %s\n", SVCFH_fmt(&argp->fh));
 
 	fh_copy(&resp->fh, &argp->fh);
-	return fh_verify(rqstp, &resp->fh, 0, MAY_NOP);
+	return fh_verify(rqstp, &resp->fh, 0, NFSD_MAY_NOP);
 }
 
 /*
Index: linux-2.6/fs/nfsd/nfs3acl.c
===================================================================
--- linux-2.6.orig/fs/nfsd/nfs3acl.c	2008-05-29 12:46:20.000000000 +0200
+++ linux-2.6/fs/nfsd/nfs3acl.c	2008-05-29 12:46:23.000000000 +0200
@@ -36,7 +36,8 @@ static __be32 nfsd3_proc_getacl(struct s
 	__be32 nfserr = 0;
 
 	fh = fh_copy(&resp->fh, &argp->fh);
-	if ((nfserr = fh_verify(rqstp, &resp->fh, 0, MAY_NOP)))
+	nfserr = fh_verify(rqstp, &resp->fh, 0, NFSD_MAY_NOP);
+	if (nfserr)
 		RETURN_STATUS(nfserr);
 
 	if (argp->mask & ~(NFS_ACL|NFS_ACLCNT|NFS_DFACL|NFS_DFACLCNT))
@@ -101,7 +102,7 @@ static __be32 nfsd3_proc_setacl(struct s
 	__be32 nfserr = 0;
 
 	fh = fh_copy(&resp->fh, &argp->fh);
-	nfserr = fh_verify(rqstp, &resp->fh, 0, MAY_SATTR);
+	nfserr = fh_verify(rqstp, &resp->fh, 0, NFSD_MAY_SATTR);
 
 	if (!nfserr) {
 		nfserr = nfserrno( nfsd_set_posix_acl(
Index: linux-2.6/fs/nfsd/nfs3proc.c
===================================================================
--- linux-2.6.orig/fs/nfsd/nfs3proc.c	2008-05-29 12:46:20.000000000 +0200
+++ linux-2.6/fs/nfsd/nfs3proc.c	2008-05-29 12:46:23.000000000 +0200
@@ -63,7 +63,7 @@ nfsd3_proc_getattr(struct svc_rqst *rqst
 		SVCFH_fmt(&argp->fh));
 
 	fh_copy(&resp->fh, &argp->fh);
-	nfserr = fh_verify(rqstp, &resp->fh, 0, MAY_NOP);
+	nfserr = fh_verify(rqstp, &resp->fh, 0, NFSD_MAY_NOP);
 	if (nfserr)
 		RETURN_STATUS(nfserr);
 
@@ -242,7 +242,7 @@ nfsd3_proc_create(struct svc_rqst *rqstp
 	attr   = &argp->attrs;
 
 	/* Get the directory inode */
-	nfserr = fh_verify(rqstp, dirfhp, S_IFDIR, MAY_CREATE);
+	nfserr = fh_verify(rqstp, dirfhp, S_IFDIR, NFSD_MAY_CREATE);
 	if (nfserr)
 		RETURN_STATUS(nfserr);
 
@@ -558,7 +558,7 @@ nfsd3_proc_fsinfo(struct svc_rqst * rqst
 	resp->f_maxfilesize = ~(u32) 0;
 	resp->f_properties = NFS3_FSF_DEFAULT;
 
-	nfserr = fh_verify(rqstp, &argp->fh, 0, MAY_NOP);
+	nfserr = fh_verify(rqstp, &argp->fh, 0, NFSD_MAY_NOP);
 
 	/* Check special features of the file system. May request
 	 * different read/write sizes for file systems known to have
@@ -597,7 +597,7 @@ nfsd3_proc_pathconf(struct svc_rqst * rq
 	resp->p_case_insensitive = 0;
 	resp->p_case_preserving = 1;
 
-	nfserr = fh_verify(rqstp, &argp->fh, 0, MAY_NOP);
+	nfserr = fh_verify(rqstp, &argp->fh, 0, NFSD_MAY_NOP);
 
 	if (nfserr == 0) {
 		struct super_block *sb = argp->fh.fh_dentry->d_inode->i_sb;
Index: linux-2.6/fs/nfsd/nfs4proc.c
===================================================================
--- linux-2.6.orig/fs/nfsd/nfs4proc.c	2008-05-29 12:46:20.000000000 +0200
+++ linux-2.6/fs/nfsd/nfs4proc.c	2008-05-29 12:46:23.000000000 +0200
@@ -73,7 +73,7 @@ do_open_permission(struct svc_rqst *rqst
 	if (open->op_share_access & NFS4_SHARE_ACCESS_READ)
 		accmode |= MAY_READ;
 	if (open->op_share_access & NFS4_SHARE_ACCESS_WRITE)
-		accmode |= (MAY_WRITE | MAY_TRUNC);
+		accmode |= (MAY_WRITE | NFSD_MAY_TRUNC);
 	if (open->op_share_deny & NFS4_SHARE_DENY_WRITE)
 		accmode |= MAY_WRITE;
 
@@ -126,7 +126,8 @@ do_open_lookup(struct svc_rqst *rqstp, s
 			&resfh.fh_handle.fh_base, resfh.fh_handle.fh_size);
 
 	if (!created)
-		status = do_open_permission(rqstp, current_fh, open, MAY_NOP);
+		status = do_open_permission(rqstp, current_fh, open,
+					    NFSD_MAY_NOP);
 
 out:
 	fh_put(&resfh);
@@ -157,7 +158,8 @@ do_open_fhandle(struct svc_rqst *rqstp, 
 	open->op_truncate = (open->op_iattr.ia_valid & ATTR_SIZE) &&
 		(open->op_iattr.ia_size == 0);
 
-	status = do_open_permission(rqstp, current_fh, open, MAY_OWNER_OVERRIDE);
+	status = do_open_permission(rqstp, current_fh, open,
+				    NFSD_MAY_OWNER_OVERRIDE);
 
 	return status;
 }
@@ -186,7 +188,7 @@ nfsd4_open(struct svc_rqst *rqstp, struc
 		cstate->current_fh.fh_handle.fh_size = rp->rp_openfh_len;
 		memcpy(&cstate->current_fh.fh_handle.fh_base, rp->rp_openfh,
 				rp->rp_openfh_len);
-		status = fh_verify(rqstp, &cstate->current_fh, 0, MAY_NOP);
+		status = fh_verify(rqstp, &cstate->current_fh, 0, NFSD_MAY_NOP);
 		if (status)
 			dprintk("nfsd4_open: replay failed"
 				" restoring previous filehandle\n");
@@ -285,7 +287,7 @@ nfsd4_putfh(struct svc_rqst *rqstp, stru
 	cstate->current_fh.fh_handle.fh_size = putfh->pf_fhlen;
 	memcpy(&cstate->current_fh.fh_handle.fh_base, putfh->pf_fhval,
 	       putfh->pf_fhlen);
-	return fh_verify(rqstp, &cstate->current_fh, 0, MAY_NOP);
+	return fh_verify(rqstp, &cstate->current_fh, 0, NFSD_MAY_NOP);
 }
 
 static __be32
@@ -363,7 +365,8 @@ nfsd4_create(struct svc_rqst *rqstp, str
 
 	fh_init(&resfh, NFS4_FHSIZE);
 
-	status = fh_verify(rqstp, &cstate->current_fh, S_IFDIR, MAY_CREATE);
+	status = fh_verify(rqstp, &cstate->current_fh, S_IFDIR,
+			   NFSD_MAY_CREATE);
 	if (status == nfserr_symlink)
 		status = nfserr_notdir;
 	if (status)
@@ -445,7 +448,7 @@ nfsd4_getattr(struct svc_rqst *rqstp, st
 {
 	__be32 status;
 
-	status = fh_verify(rqstp, &cstate->current_fh, 0, MAY_NOP);
+	status = fh_verify(rqstp, &cstate->current_fh, 0, NFSD_MAY_NOP);
 	if (status)
 		return status;
 
@@ -730,7 +733,7 @@ _nfsd4_verify(struct svc_rqst *rqstp, st
 	int count;
 	__be32 status;
 
-	status = fh_verify(rqstp, &cstate->current_fh, 0, MAY_NOP);
+	status = fh_verify(rqstp, &cstate->current_fh, 0, NFSD_MAY_NOP);
 	if (status)
 		return status;
 
Index: linux-2.6/include/linux/nfsd/nfsd.h
===================================================================
--- linux-2.6.orig/include/linux/nfsd/nfsd.h	2008-05-29 12:46:20.000000000 +0200
+++ linux-2.6/include/linux/nfsd/nfsd.h	2008-05-29 12:46:23.000000000 +0200
@@ -31,17 +31,17 @@
  * Special flags for nfsd_permission. These must be different from MAY_READ,
  * MAY_WRITE, and MAY_EXEC.
  */
-#define MAY_NOP			0
-#define MAY_SATTR		8
-#define MAY_TRUNC		16
-#define MAY_LOCK		32
-#define MAY_OWNER_OVERRIDE	64
-#define	MAY_LOCAL_ACCESS	128 /* IRIX doing local access check on device special file*/
-#if (MAY_SATTR | MAY_TRUNC | MAY_LOCK | MAY_OWNER_OVERRIDE | MAY_LOCAL_ACCESS) & (MAY_READ | MAY_WRITE | MAY_EXEC)
-# error "please use a different value for MAY_SATTR or MAY_TRUNC or MAY_LOCK or MAY_LOCAL_ACCESS or MAY_OWNER_OVERRIDE."
+#define NFSD_MAY_NOP			0
+#define NFSD_MAY_SATTR		8
+#define NFSD_MAY_TRUNC		16
+#define NFSD_MAY_LOCK		32
+#define NFSD_MAY_OWNER_OVERRIDE	64
+#define	NFSD_MAY_LOCAL_ACCESS	128 /* IRIX doing local access check on device special file*/
+#if (NFSD_MAY_SATTR | NFSD_MAY_TRUNC | NFSD_MAY_LOCK | NFSD_MAY_OWNER_OVERRIDE | NFSD_MAY_LOCAL_ACCESS) & (MAY_READ | MAY_WRITE | MAY_EXEC)
+# error "please use a different value for NFSD_MAY_SATTR or NFSD_MAY_TRUNC or NFSD_MAY_LOCK or NFSD_MAY_LOCAL_ACCESS or NFSD_MAY_OWNER_OVERRIDE."
 #endif
-#define MAY_CREATE		(MAY_EXEC|MAY_WRITE)
-#define MAY_REMOVE		(MAY_EXEC|MAY_WRITE|MAY_TRUNC)
+#define NFSD_MAY_CREATE		(MAY_EXEC|MAY_WRITE)
+#define NFSD_MAY_REMOVE		(MAY_EXEC|MAY_WRITE|NFSD_MAY_TRUNC)
 
 /*
  * Callback function for readdir
Index: linux-2.6/fs/nfsd/lockd.c
===================================================================
--- linux-2.6.orig/fs/nfsd/lockd.c	2008-05-29 12:46:20.000000000 +0200
+++ linux-2.6/fs/nfsd/lockd.c	2008-05-29 12:46:23.000000000 +0200
@@ -35,7 +35,7 @@ nlm_fopen(struct svc_rqst *rqstp, struct
 	fh.fh_export = NULL;
 
 	exp_readlock();
-	nfserr = nfsd_open(rqstp, &fh, S_IFREG, MAY_LOCK, filp);
+	nfserr = nfsd_open(rqstp, &fh, S_IFREG, NFSD_MAY_LOCK, filp);
 	fh_put(&fh);
 	rqstp->rq_client = NULL;
 	exp_readunlock();
Index: linux-2.6/fs/nfsd/nfs4state.c
===================================================================
--- linux-2.6.orig/fs/nfsd/nfs4state.c	2008-05-29 12:46:20.000000000 +0200
+++ linux-2.6/fs/nfsd/nfs4state.c	2008-05-29 12:46:23.000000000 +0200
@@ -2610,7 +2610,7 @@ nfsd4_lock(struct svc_rqst *rqstp, struc
 		 return nfserr_inval;
 
 	if ((status = fh_verify(rqstp, &cstate->current_fh,
-				S_IFREG, MAY_LOCK))) {
+				S_IFREG, NFSD_MAY_LOCK))) {
 		dprintk("NFSD: nfsd4_lock: permission denied!\n");
 		return status;
 	}
Index: linux-2.6/fs/nfsd/nfsfh.c
===================================================================
--- linux-2.6.orig/fs/nfsd/nfsfh.c	2008-05-29 12:46:20.000000000 +0200
+++ linux-2.6/fs/nfsd/nfsfh.c	2008-05-29 12:46:23.000000000 +0200
@@ -280,7 +280,7 @@ fh_verify(struct svc_rqst *rqstp, struct
 	if (error)
 		goto out;
 
-	if (!(access & MAY_LOCK)) {
+	if (!(access & NFSD_MAY_LOCK)) {
 		/*
 		 * pseudoflavor restrictions are not enforced on NLM,
 		 * which clients virtually always use auth_sys for,
Index: linux-2.6/fs/nfsd/nfsproc.c
===================================================================
--- linux-2.6.orig/fs/nfsd/nfsproc.c	2008-05-29 12:46:20.000000000 +0200
+++ linux-2.6/fs/nfsd/nfsproc.c	2008-05-29 12:46:23.000000000 +0200
@@ -65,7 +65,7 @@ nfsd_proc_getattr(struct svc_rqst *rqstp
 	dprintk("nfsd: GETATTR  %s\n", SVCFH_fmt(&argp->fh));
 
 	fh_copy(&resp->fh, &argp->fh);
-	nfserr = fh_verify(rqstp, &resp->fh, 0, MAY_NOP);
+	nfserr = fh_verify(rqstp, &resp->fh, 0, NFSD_MAY_NOP);
 	return nfsd_return_attrs(nfserr, resp);
 }
 
@@ -281,7 +281,7 @@ nfsd_proc_create(struct svc_rqst *rqstp,
 					nfserr = nfsd_permission(rqstp,
 								 newfhp->fh_export,
 								 newfhp->fh_dentry,
-								 MAY_WRITE|MAY_LOCAL_ACCESS);
+								 MAY_WRITE|NFSD_MAY_LOCAL_ACCESS);
 					if (nfserr && nfserr != nfserr_rofs)
 						goto out_unlock;
 				}
Index: linux-2.6/fs/nfsd/vfs.c
===================================================================
--- linux-2.6.orig/fs/nfsd/vfs.c	2008-05-29 12:46:20.000000000 +0200
+++ linux-2.6/fs/nfsd/vfs.c	2008-05-29 12:46:23.000000000 +0200
@@ -268,14 +268,14 @@ nfsd_setattr(struct svc_rqst *rqstp, str
 {
 	struct dentry	*dentry;
 	struct inode	*inode;
-	int		accmode = MAY_SATTR;
+	int		accmode = NFSD_MAY_SATTR;
 	int		ftype = 0;
 	__be32		err;
 	int		host_err;
 	int		size_change = 0;
 
 	if (iap->ia_valid & (ATTR_ATIME | ATTR_MTIME | ATTR_SIZE))
-		accmode |= MAY_WRITE|MAY_OWNER_OVERRIDE;
+		accmode |= MAY_WRITE|NFSD_MAY_OWNER_OVERRIDE;
 	if (iap->ia_valid & ATTR_SIZE)
 		ftype = S_IFREG;
 
@@ -337,7 +337,8 @@ nfsd_setattr(struct svc_rqst *rqstp, str
 	 */
 	if (iap->ia_valid & ATTR_SIZE) {
 		if (iap->ia_size < inode->i_size) {
-			err = nfsd_permission(rqstp, fhp->fh_export, dentry, MAY_TRUNC|MAY_OWNER_OVERRIDE);
+			err = nfsd_permission(rqstp, fhp->fh_export, dentry,
+					NFSD_MAY_TRUNC|NFSD_MAY_OWNER_OVERRIDE);
 			if (err)
 				goto out;
 		}
@@ -471,7 +472,7 @@ nfsd4_set_nfs4_acl(struct svc_rqst *rqst
 	unsigned int flags = 0;
 
 	/* Get inode */
-	error = fh_verify(rqstp, fhp, 0 /* S_IFREG */, MAY_SATTR);
+	error = fh_verify(rqstp, fhp, 0 /* S_IFREG */, NFSD_MAY_SATTR);
 	if (error)
 		return error;
 
@@ -579,7 +580,7 @@ struct accessmap {
 static struct accessmap	nfs3_regaccess[] = {
     {	NFS3_ACCESS_READ,	MAY_READ			},
     {	NFS3_ACCESS_EXECUTE,	MAY_EXEC			},
-    {	NFS3_ACCESS_MODIFY,	MAY_WRITE|MAY_TRUNC		},
+    {	NFS3_ACCESS_MODIFY,	MAY_WRITE|NFSD_MAY_TRUNC	},
     {	NFS3_ACCESS_EXTEND,	MAY_WRITE			},
 
     {	0,			0				}
@@ -588,9 +589,9 @@ static struct accessmap	nfs3_regaccess[]
 static struct accessmap	nfs3_diraccess[] = {
     {	NFS3_ACCESS_READ,	MAY_READ			},
     {	NFS3_ACCESS_LOOKUP,	MAY_EXEC			},
-    {	NFS3_ACCESS_MODIFY,	MAY_EXEC|MAY_WRITE|MAY_TRUNC	},
+    {	NFS3_ACCESS_MODIFY,	MAY_EXEC|MAY_WRITE|NFSD_MAY_TRUNC},
     {	NFS3_ACCESS_EXTEND,	MAY_EXEC|MAY_WRITE		},
-    {	NFS3_ACCESS_DELETE,	MAY_REMOVE			},
+    {	NFS3_ACCESS_DELETE,	NFSD_MAY_REMOVE			},
 
     {	0,			0				}
 };
@@ -605,8 +606,8 @@ static struct accessmap	nfs3_anyaccess[]
 	 */
     {	NFS3_ACCESS_READ,	MAY_READ			},
     {	NFS3_ACCESS_EXECUTE,	MAY_EXEC			},
-    {	NFS3_ACCESS_MODIFY,	MAY_WRITE|MAY_LOCAL_ACCESS	},
-    {	NFS3_ACCESS_EXTEND,	MAY_WRITE|MAY_LOCAL_ACCESS	},
+    {	NFS3_ACCESS_MODIFY,	MAY_WRITE|NFSD_MAY_LOCAL_ACCESS	},
+    {	NFS3_ACCESS_EXTEND,	MAY_WRITE|NFSD_MAY_LOCAL_ACCESS	},
 
     {	0,			0				}
 };
@@ -620,7 +621,7 @@ nfsd_access(struct svc_rqst *rqstp, stru
 	u32			query, result = 0, sresult = 0;
 	__be32			error;
 
-	error = fh_verify(rqstp, fhp, 0, MAY_NOP);
+	error = fh_verify(rqstp, fhp, 0, NFSD_MAY_NOP);
 	if (error)
 		goto out;
 
@@ -692,7 +693,7 @@ nfsd_open(struct svc_rqst *rqstp, struct
 	 * and (hopefully) checked permission - so allow OWNER_OVERRIDE
 	 * in case a chmod has now revoked permission.
 	 */
-	err = fh_verify(rqstp, fhp, type, access | MAY_OWNER_OVERRIDE);
+	err = fh_verify(rqstp, fhp, type, access | NFSD_MAY_OWNER_OVERRIDE);
 	if (err)
 		goto out;
 
@@ -1085,7 +1086,7 @@ nfsd_read(struct svc_rqst *rqstp, struct
 
 	if (file) {
 		err = nfsd_permission(rqstp, fhp->fh_export, fhp->fh_dentry,
-				MAY_READ|MAY_OWNER_OVERRIDE);
+				MAY_READ|NFSD_MAY_OWNER_OVERRIDE);
 		if (err)
 			goto out;
 		err = nfsd_vfs_read(rqstp, fhp, file, offset, vec, vlen, count);
@@ -1114,7 +1115,7 @@ nfsd_write(struct svc_rqst *rqstp, struc
 
 	if (file) {
 		err = nfsd_permission(rqstp, fhp->fh_export, fhp->fh_dentry,
-				MAY_WRITE|MAY_OWNER_OVERRIDE);
+				MAY_WRITE|NFSD_MAY_OWNER_OVERRIDE);
 		if (err)
 			goto out;
 		err = nfsd_vfs_write(rqstp, fhp, file, offset, vec, vlen, cnt,
@@ -1214,7 +1215,7 @@ nfsd_create(struct svc_rqst *rqstp, stru
 	if (isdotent(fname, flen))
 		goto out;
 
-	err = fh_verify(rqstp, fhp, S_IFDIR, MAY_CREATE);
+	err = fh_verify(rqstp, fhp, S_IFDIR, NFSD_MAY_CREATE);
 	if (err)
 		goto out;
 
@@ -1347,7 +1348,7 @@ nfsd_create_v3(struct svc_rqst *rqstp, s
 		goto out;
 	if (!(iap->ia_valid & ATTR_MODE))
 		iap->ia_mode = 0;
-	err = fh_verify(rqstp, fhp, S_IFDIR, MAY_CREATE);
+	err = fh_verify(rqstp, fhp, S_IFDIR, NFSD_MAY_CREATE);
 	if (err)
 		goto out;
 
@@ -1485,7 +1486,7 @@ nfsd_readlink(struct svc_rqst *rqstp, st
 	__be32		err;
 	int		host_err;
 
-	err = fh_verify(rqstp, fhp, S_IFLNK, MAY_NOP);
+	err = fh_verify(rqstp, fhp, S_IFLNK, NFSD_MAY_NOP);
 	if (err)
 		goto out;
 
@@ -1540,7 +1541,7 @@ nfsd_symlink(struct svc_rqst *rqstp, str
 	if (isdotent(fname, flen))
 		goto out;
 
-	err = fh_verify(rqstp, fhp, S_IFDIR, MAY_CREATE);
+	err = fh_verify(rqstp, fhp, S_IFDIR, NFSD_MAY_CREATE);
 	if (err)
 		goto out;
 	fh_lock(fhp);
@@ -1596,10 +1597,10 @@ nfsd_link(struct svc_rqst *rqstp, struct
 	__be32		err;
 	int		host_err;
 
-	err = fh_verify(rqstp, ffhp, S_IFDIR, MAY_CREATE);
+	err = fh_verify(rqstp, ffhp, S_IFDIR, NFSD_MAY_CREATE);
 	if (err)
 		goto out;
-	err = fh_verify(rqstp, tfhp, -S_IFDIR, MAY_NOP);
+	err = fh_verify(rqstp, tfhp, -S_IFDIR, NFSD_MAY_NOP);
 	if (err)
 		goto out;
 
@@ -1660,10 +1661,10 @@ nfsd_rename(struct svc_rqst *rqstp, stru
 	__be32		err;
 	int		host_err;
 
-	err = fh_verify(rqstp, ffhp, S_IFDIR, MAY_REMOVE);
+	err = fh_verify(rqstp, ffhp, S_IFDIR, NFSD_MAY_REMOVE);
 	if (err)
 		goto out;
-	err = fh_verify(rqstp, tfhp, S_IFDIR, MAY_CREATE);
+	err = fh_verify(rqstp, tfhp, S_IFDIR, NFSD_MAY_CREATE);
 	if (err)
 		goto out;
 
@@ -1757,7 +1758,7 @@ nfsd_unlink(struct svc_rqst *rqstp, stru
 	err = nfserr_acces;
 	if (!flen || isdotent(fname, flen))
 		goto out;
-	err = fh_verify(rqstp, fhp, S_IFDIR, MAY_REMOVE);
+	err = fh_verify(rqstp, fhp, S_IFDIR, NFSD_MAY_REMOVE);
 	if (err)
 		goto out;
 
@@ -1858,7 +1859,7 @@ out:
 __be32
 nfsd_statfs(struct svc_rqst *rqstp, struct svc_fh *fhp, struct kstatfs *stat)
 {
-	__be32 err = fh_verify(rqstp, fhp, 0, MAY_NOP);
+	__be32 err = fh_verify(rqstp, fhp, 0, NFSD_MAY_NOP);
 	if (!err && vfs_statfs(fhp->fh_dentry,stat))
 		err = nfserr_io;
 	return err;
@@ -1883,7 +1884,7 @@ nfsd_permission(struct svc_rqst *rqstp, 
 		.dentry = dentry,
 	};
 
-	if (acc == MAY_NOP)
+	if (acc == NFSD_MAY_NOP)
 		return 0;
 #if 0
 	dprintk("nfsd: permission 0x%x%s%s%s%s%s%s%s mode 0%o%s%s%s\n",
@@ -1891,10 +1892,10 @@ nfsd_permission(struct svc_rqst *rqstp, 
 		(acc & MAY_READ)?	" read"  : "",
 		(acc & MAY_WRITE)?	" write" : "",
 		(acc & MAY_EXEC)?	" exec"  : "",
-		(acc & MAY_SATTR)?	" sattr" : "",
-		(acc & MAY_TRUNC)?	" trunc" : "",
-		(acc & MAY_LOCK)?	" lock"  : "",
-		(acc & MAY_OWNER_OVERRIDE)? " owneroverride" : "",
+		(acc & NFSD_MAY_SATTR)?	" sattr" : "",
+		(acc & NFSD_MAY_TRUNC)?	" trunc" : "",
+		(acc & NFSD_MAY_LOCK)?	" lock"  : "",
+		(acc & NFSD_MAY_OWNER_OVERRIDE)? " owneroverride" : "",
 		inode->i_mode,
 		IS_IMMUTABLE(inode)?	" immut" : "",
 		IS_APPEND(inode)?	" append" : "",
@@ -1907,18 +1908,18 @@ nfsd_permission(struct svc_rqst *rqstp, 
 	 * system.  But if it is IRIX doing check on write-access for a 
 	 * device special file, we ignore rofs.
 	 */
-	if (!(acc & MAY_LOCAL_ACCESS))
-		if (acc & (MAY_WRITE | MAY_SATTR | MAY_TRUNC)) {
+	if (!(acc & NFSD_MAY_LOCAL_ACCESS))
+		if (acc & (MAY_WRITE | NFSD_MAY_SATTR | NFSD_MAY_TRUNC)) {
 			if (exp_rdonly(rqstp, exp) ||
 			    __mnt_is_readonly(exp->ex_path.mnt))
 				return nfserr_rofs;
 			if (/* (acc & MAY_WRITE) && */ IS_IMMUTABLE(inode))
 				return nfserr_perm;
 		}
-	if ((acc & MAY_TRUNC) && IS_APPEND(inode))
+	if ((acc & NFSD_MAY_TRUNC) && IS_APPEND(inode))
 		return nfserr_perm;
 
-	if (acc & MAY_LOCK) {
+	if (acc & NFSD_MAY_LOCK) {
 		/* If we cannot rely on authentication in NLM requests,
 		 * just allow locks, otherwise require read permission, or
 		 * ownership
@@ -1926,7 +1927,7 @@ nfsd_permission(struct svc_rqst *rqstp, 
 		if (exp->ex_flags & NFSEXP_NOAUTHNLM)
 			return 0;
 		else
-			acc = MAY_READ | MAY_OWNER_OVERRIDE;
+			acc = MAY_READ | NFSD_MAY_OWNER_OVERRIDE;
 	}
 	/*
 	 * The file owner always gets access permission for accesses that
@@ -1942,7 +1943,7 @@ nfsd_permission(struct svc_rqst *rqstp, 
 	 * We must trust the client to do permission checking - using "ACCESS"
 	 * with NFSv3.
 	 */
-	if ((acc & MAY_OWNER_OVERRIDE) &&
+	if ((acc & NFSD_MAY_OWNER_OVERRIDE) &&
 	    inode->i_uid == current->fsuid)
 		return 0;
 
@@ -1950,7 +1951,7 @@ nfsd_permission(struct svc_rqst *rqstp, 
 
 	/* Allow read access to binaries even when mode 111 */
 	if (err == -EACCES && S_ISREG(inode->i_mode) &&
-	    acc == (MAY_READ | MAY_OWNER_OVERRIDE))
+	    acc == (MAY_READ | NFSD_MAY_OWNER_OVERRIDE))
 		err = path_permission(&path, MAY_EXEC);
 
 	return err? nfserrno(err) : 0;

--

Re: [patch 4/8] nfsd: rename MAY_ flags

[Christoph Hellwig]

On Thu, May 29, 2008 at 01:32:49PM +0200, Miklos Szeredi wrote:
> From: Miklos Szeredi <mszeredi@suse.cz>
> 
> Rename nfsd specific MAY_* flags to NFSD_MAY_* to make it clear, that
> these are not used outside nfsd, and to avoid namespace conflicts with
> the VFS.

But they _are_ in the same namespace as the VFS ones.  This needs to be
sorted out by real by understanding what's going on here and either
separating this flags out and passing them in a separate argument, or
moving them to include/linux/fs.h so it's obvious for anyone adding new
flags that they must not collide.  In that later case it might make
sense to add add a NFSD in the name, although MAY_NFSD_* makes more
sense to keep a unique prefix.

Re: [patch 4/8] nfsd: rename MAY_ flags

[Miklos Szeredi]

> > From: Miklos Szeredi <mszeredi@suse.cz>
> > 
> > Rename nfsd specific MAY_* flags to NFSD_MAY_* to make it clear, that
> > these are not used outside nfsd, and to avoid namespace conflicts with
> > the VFS.
> 
> But they _are_ in the same namespace as the VFS ones.  This needs to be
> sorted out by real by understanding what's going on here and either
> separating this flags out and passing them in a separate argument, or
> moving them to include/linux/fs.h so it's obvious for anyone adding new
> flags that they must not collide.

Neither I think.  What's going on is that nfsd has a private set of
permission flags, and a private permission checking function, which
incidentally calls the vfs' permission checking function.

Does it hurt to share the three base MAY_ flags for this purpose?  I
don't think it does, but I'm interested in the nfsd maintainers'
opinions.

Thanks,
Miklos

Re: [patch 4/8] nfsd: rename MAY_ flags

[J. Bruce Fields]

On Thu, May 29, 2008 at 04:02:39PM +0200, Miklos Szeredi wrote:
> > > From: Miklos Szeredi <mszeredi@suse.cz>
> > > 
> > > Rename nfsd specific MAY_* flags to NFSD_MAY_* to make it clear, that
> > > these are not used outside nfsd, and to avoid namespace conflicts with
> > > the VFS.
> > 
> > But they _are_ in the same namespace as the VFS ones.  This needs to be
> > sorted out by real by understanding what's going on here and either
> > separating this flags out and passing them in a separate argument, or
> > moving them to include/linux/fs.h so it's obvious for anyone adding new
> > flags that they must not collide.
> 
> Neither I think.  What's going on is that nfsd has a private set of
> permission flags, and a private permission checking function, which
> incidentally calls the vfs' permission checking function.
> 
> Does it hurt to share the three base MAY_ flags for this purpose?  I
> don't think it does, but I'm interested in the nfsd maintainers'
> opinions.

This isn't something I've ever had a reason to care about.  What are you
trying to fix exactly?

--b.

Re: [patch 4/8] nfsd: rename MAY_ flags

[Christoph Hellwig]

On Thu, May 29, 2008 at 04:40:02PM -0400, J. Bruce Fields wrote:
> > don't think it does, but I'm interested in the nfsd maintainers'
> > opinions.
> 
> This isn't something I've ever had a reason to care about.  What are you
> trying to fix exactly?

The NFS MAY_ flags operate in the same name and number space and we'd
easily get collisions when someone adds new MAY_ flags which miklos
as well as at least two other independent efforts want to do.  To sort
this out we'd either defined the nfsd MAY_ flags in fs.h to make it
obvious we should not double-allocates bits or names, or use a different
name and number space for the nfsd flags.  The first would be rather
trivial but also ugly, the seconds sound much better but is a little
more effort.  Just defined NFSD_MAY_ and use it everywhere and do a
little translation inside nfsd_permission before passing it on to
permission().


> 
> --b.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
---end quoted text---

Re: [patch 4/8] nfsd: rename MAY_ flags

[Miklos Szeredi]

> > > don't think it does, but I'm interested in the nfsd maintainers'
> > > opinions.
> > 
> > This isn't something I've ever had a reason to care about.  What are you
> > trying to fix exactly?
> 
> The NFS MAY_ flags operate in the same name and number space and we'd
> easily get collisions when someone adds new MAY_ flags which miklos
> as well as at least two other independent efforts want to do.  To sort
> this out we'd either defined the nfsd MAY_ flags in fs.h to make it
> obvious we should not double-allocates bits or names, or use a different
> name and number space for the nfsd flags.  The first would be rather
> trivial but also ugly, the seconds sound much better but is a little
> more effort.  Just defined NFSD_MAY_ and use it everywhere and do a
> little translation inside nfsd_permission before passing it on to
> permission().

Yeah, I wouldn't mind that.  Although I'd still define NFSD_MAY_EXEC,
NFSD_MAY_READ and NFSD_MAY_WRITE to be exactly the same as MAY_EXEC,
etc..., and have the translation actually just mask off the rest of
the bits (as it does currently).

These three constants are very much ingrained in the UNIX tradition,
and it's no accident that the MAY_ flags correspond exactly to the
i_mode bits, which is something often exploited by permission checking
code.

Miklos

Re: [patch 4/8] nfsd: rename MAY_ flags

[Miklos Szeredi]

> These three constants are very much ingrained in the UNIX tradition,
> and it's no accident that the MAY_ flags correspond exactly to the
> i_mode bits, which is something often exploited by permission checking
> code.

Funny example of naming confusion:

> asmlinkage long sys_faccessat(int dfd, const char __user *filename, int mode)
> {
> 	...
> 	if (mode & ~S_IRWXO)	/* where's F_OK, X_OK, W_OK, R_OK? */
> 		return -EINVAL;

The MAY_* namespace is never mentioned, even though that's the one
most relevant here :)

Miklos

Re: [patch 4/8] nfsd: rename MAY_ flags

[J. Bruce Fields]

On Fri, May 30, 2008 at 11:07:53AM +0200, Miklos Szeredi wrote:
> > > > don't think it does, but I'm interested in the nfsd maintainers'
> > > > opinions.
> > > 
> > > This isn't something I've ever had a reason to care about.  What are you
> > > trying to fix exactly?
> > 
> > The NFS MAY_ flags operate in the same name and number space and we'd
> > easily get collisions when someone adds new MAY_ flags which miklos
> > as well as at least two other independent efforts want to do.  To sort
> > this out we'd either defined the nfsd MAY_ flags in fs.h to make it
> > obvious we should not double-allocates bits or names, or use a different
> > name and number space for the nfsd flags.  The first would be rather
> > trivial but also ugly, the seconds sound much better but is a little
> > more effort.  Just defined NFSD_MAY_ and use it everywhere and do a
> > little translation inside nfsd_permission before passing it on to
> > permission().
> 
> Yeah, I wouldn't mind that.  Although I'd still define NFSD_MAY_EXEC,
> NFSD_MAY_READ and NFSD_MAY_WRITE to be exactly the same as MAY_EXEC,
> etc..., and have the translation actually just mask off the rest of
> the bits (as it does currently).

OK by me.

--b.

[patch 5/8] vfs: annotate permission operations

[Miklos Szeredi]

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

From: Miklos Szeredi <mszeredi@suse.cz>

Add more PERM_OP_ flags to permission() calls.  This allows
filesystems like NFS and security modules like AppArmor to make
precise decisions about whent permissions need to be checked, and when
they will be checked later together with the actual operation.

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---
 fs/namei.c         |   16 ++++++++--------
 fs/nfsd/nfsfh.c    |    2 +-
 fs/open.c          |    6 +++---
 fs/utimes.c        |    2 +-
 fs/xattr.c         |    2 +-
 include/linux/fs.h |   19 +++++++++++++++++++
 6 files changed, 33 insertions(+), 14 deletions(-)

Index: linux-2.6/fs/namei.c
===================================================================
--- linux-2.6.orig/fs/namei.c	2008-05-29 12:46:20.000000000 +0200
+++ linux-2.6/fs/namei.c	2008-05-29 12:46:24.000000000 +0200
@@ -492,7 +492,7 @@ static int exec_permission_lite(struct i
 
 	return -EACCES;
 ok:
-	return security_inode_permission(inode, MAY_EXEC);
+	return security_inode_permission(inode, MAY_LOOKUP);
 }
 
 /*
@@ -899,7 +899,7 @@ static int __link_path_walk(const char *
 		nd->flags |= LOOKUP_CONTINUE;
 		err = exec_permission_lite(inode);
 		if (err == -EAGAIN)
-			err = path_permission(&nd->path, MAY_EXEC);
+			err = path_permission(&nd->path, MAY_LOOKUP);
  		if (err)
 			break;
 
@@ -1174,7 +1174,7 @@ static int do_path_lookup(int dfd, const
 		if (!S_ISDIR(dentry->d_inode->i_mode))
 			goto fput_fail;
 
-		retval = file_permission(file, MAY_EXEC);
+		retval = file_permission(file, MAY_LOOKUP);
 		if (retval)
 			goto fput_fail;
 
@@ -1347,7 +1347,7 @@ static struct dentry *lookup_hash(struct
 {
 	int err;
 
-	err = path_permission(&nd->path, MAY_EXEC);
+	err = path_permission(&nd->path, MAY_LOOKUP);
 	if (err)
 		return ERR_PTR(err);
 	return __lookup_hash(&nd->last, nd->path.dentry, nd);
@@ -1395,7 +1395,7 @@ struct dentry *lookup_one_len(const char
 	if (err)
 		return ERR_PTR(err);
 
-	err = dentry_permission(base, MAY_EXEC);
+	err = dentry_permission(base, MAY_LOOKUP);
 	if (err)
 		return ERR_PTR(err);
 	return __lookup_hash(&this, base, NULL);
@@ -1487,7 +1487,7 @@ static int may_delete(struct dentry *dir
 	BUG_ON(victim->d_parent->d_inode != dir);
 	audit_inode_child(victim->d_name.name, victim, dir);
 
-	error = dentry_permission(dir_dentry, MAY_WRITE | MAY_EXEC);
+	error = dentry_permission(dir_dentry, MAY_DELETE);
 	if (error)
 		return error;
 	if (IS_APPEND(dir))
@@ -1523,7 +1523,7 @@ static inline int may_create(struct dent
 		return -EEXIST;
 	if (IS_DEADDIR(dir_dentry->d_inode))
 		return -ENOENT;
-	return dentry_permission(dir_dentry, MAY_WRITE | MAY_EXEC);
+	return dentry_permission(dir_dentry, MAY_CREATE);
 }
 
 /* 
@@ -2687,7 +2687,7 @@ static int vfs_rename_dir(struct inode *
 	 * we'll need to flip '..'.
 	 */
 	if (new_dir != old_dir) {
-		error = dentry_permission(old_dentry, MAY_WRITE);
+		error = dentry_permission(old_dentry, MAY_MOVE_DIR);
 		if (error)
 			return error;
 	}
Index: linux-2.6/fs/nfsd/nfsfh.c
===================================================================
--- linux-2.6.orig/fs/nfsd/nfsfh.c	2008-05-29 12:46:23.000000000 +0200
+++ linux-2.6/fs/nfsd/nfsfh.c	2008-05-29 12:46:24.000000000 +0200
@@ -52,7 +52,7 @@ static int nfsd_acceptable(void *expv, s
 		int err;
 
 		parent.dentry = dget_parent(tdentry);
-		err = path_permission(&parent, MAY_EXEC);
+		err = path_permission(&parent, MAY_LOOKUP);
 		if (err < 0) {
 			dput(parent.dentry);
 			break;
Index: linux-2.6/fs/open.c
===================================================================
--- linux-2.6.orig/fs/open.c	2008-05-29 12:46:20.000000000 +0200
+++ linux-2.6/fs/open.c	2008-05-29 12:46:24.000000000 +0200
@@ -513,7 +513,7 @@ asmlinkage long sys_chdir(const char __u
 	if (error)
 		goto out;
 
-	error = path_permission(&nd.path, MAY_EXEC | PERM_OP_CHDIR);
+	error = path_permission(&nd.path, MAY_CHDIR);
 	if (error)
 		goto dput_and_out;
 
@@ -542,7 +542,7 @@ asmlinkage long sys_fchdir(unsigned int 
 	if (!S_ISDIR(inode->i_mode))
 		goto out_putf;
 
-	error = file_permission(file, MAY_EXEC | PERM_OP_CHDIR);
+	error = file_permission(file, MAY_CHDIR);
 	if (!error)
 		set_fs_pwd(current->fs, &file->f_path);
 out_putf:
@@ -560,7 +560,7 @@ asmlinkage long sys_chroot(const char __
 	if (error)
 		goto out;
 
-	error = path_permission(&nd.path, MAY_EXEC);
+	error = path_permission(&nd.path, MAY_CHROOT);
 	if (error)
 		goto dput_and_out;
 
Index: linux-2.6/fs/utimes.c
===================================================================
--- linux-2.6.orig/fs/utimes.c	2008-05-29 12:46:20.000000000 +0200
+++ linux-2.6/fs/utimes.c	2008-05-29 12:46:24.000000000 +0200
@@ -141,7 +141,7 @@ static int do_utimes_name(int dfd, char 
 			goto out_path_put;
 
 		if (!is_owner_or_cap(inode)) {
-			error = path_permission(&nd.path, MAY_WRITE);
+			error = path_permission(&nd.path, MAY_UTIMES);
 			if (error)
 				goto out_path_put;
 		}
Index: linux-2.6/fs/xattr.c
===================================================================
--- linux-2.6.orig/fs/xattr.c	2008-05-29 12:46:20.000000000 +0200
+++ linux-2.6/fs/xattr.c	2008-05-29 12:46:24.000000000 +0200
@@ -65,7 +65,7 @@ xattr_permission(struct path *path, cons
 			return -EPERM;
 	}
 
-	return path_permission(path, mask);
+	return path_permission(path, mask | PERM_OP_XATTR);
 }
 
 static int
Index: linux-2.6/include/linux/fs.h
===================================================================
--- linux-2.6.orig/include/linux/fs.h	2008-05-29 12:46:20.000000000 +0200
+++ linux-2.6/include/linux/fs.h	2008-05-29 12:46:24.000000000 +0200
@@ -70,6 +70,25 @@ extern int dir_notify_enable;
 #define PERM_OP_OPEN	(0x1 << 28)
 #define PERM_OP_ACCESS	(0x2 << 28)
 #define PERM_OP_CHDIR	(0x3 << 28)
+#define PERM_OP_CHROOT	(0x4 << 28)
+#define PERM_OP_LOOKUP	(0x5 << 28)
+#define PERM_OP_CREATE	(0x6 << 28)
+#define PERM_OP_DELETE	(0x7 << 28)
+#define PERM_OP_MOVE_DIR	(0x8 << 28)
+#define PERM_OP_UTIMES	(0x9 << 28)
+#define PERM_OP_XATTR	(0xa << 28)
+
+/*
+ * Combined MAY_ flags
+ */
+#define MAY_CHDIR	(PERM_OP_CHDIR | MAY_EXEC)
+#define MAY_CHROOT	(PERM_OP_CHROOT | MAY_EXEC)
+#define MAY_LOOKUP	(PERM_OP_LOOKUP | MAY_EXEC)
+#define MAY_CREATE	(PERM_OP_CREATE | MAY_EXEC | MAY_WRITE)
+#define MAY_DELETE	(PERM_OP_DELETE | MAY_EXEC | MAY_WRITE)
+#define MAY_MOVE_DIR	(PERM_OP_MOVE_DIR | MAY_WRITE)
+#define MAY_UTIMES	(PERM_OP_UTIMES | MAY_WRITE)
+
 
 #define FMODE_READ 1
 #define FMODE_WRITE 2

--

Re: [patch 5/8] vfs: annotate permission operations

[Christoph Hellwig]

On Thu, May 29, 2008 at 01:32:50PM +0200, Miklos Szeredi wrote:
> From: Miklos Szeredi <mszeredi@suse.cz>
> 
> Add more PERM_OP_ flags to permission() calls.  This allows
> filesystems like NFS and security modules like AppArmor to make
> precise decisions about whent permissions need to be checked, and when
> they will be checked later together with the actual operation.

Where the heck does this PERM_OP crap come from?  More MAY_ flags make
sense in some cases, but we should introduce them on a per-need basis
(e.g. to replace lookup intents in many cases), and without a new
PERM_OP_ namespace.

[patch 6/8] Factor out sysctl pathname code

[Miklos Szeredi]

[-- Attachment #1: sysctl-pathname.diff --]
[-- Type: text/plain, Size: 3505 bytes --]

Convert the selinux sysctl pathname computation code into a standalone
function.

Signed-off-by: Andreas Gruenbacher <agruen@suse.de>
Signed-off-by: John Johansen <jjohansen@suse.de>
Reviewed-by: James Morris <jmorris@namei.org>
Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---
 include/linux/sysctl.h   |    2 ++
 kernel/sysctl.c          |   27 +++++++++++++++++++++++++++
 security/selinux/hooks.c |   35 +++++------------------------------
 3 files changed, 34 insertions(+), 30 deletions(-)

Index: linux-2.6/include/linux/sysctl.h
===================================================================
--- linux-2.6.orig/include/linux/sysctl.h	2008-05-29 12:46:20.000000000 +0200
+++ linux-2.6/include/linux/sysctl.h	2008-05-29 12:46:25.000000000 +0200
@@ -980,6 +980,8 @@ extern int proc_doulongvec_minmax(struct
 extern int proc_doulongvec_ms_jiffies_minmax(struct ctl_table *table, int,
 				      struct file *, void __user *, size_t *, loff_t *);
 
+extern char *sysctl_pathname(ctl_table *, char *, int);
+
 extern int do_sysctl (int __user *name, int nlen,
 		      void __user *oldval, size_t __user *oldlenp,
 		      void __user *newval, size_t newlen);
Index: linux-2.6/kernel/sysctl.c
===================================================================
--- linux-2.6.orig/kernel/sysctl.c	2008-05-29 12:46:20.000000000 +0200
+++ linux-2.6/kernel/sysctl.c	2008-05-29 12:46:25.000000000 +0200
@@ -1432,6 +1432,33 @@ void register_sysctl_root(struct ctl_tab
 	spin_unlock(&sysctl_lock);
 }
 
+char *sysctl_pathname(ctl_table *table, char *buffer, int buflen)
+{
+	if (buflen < 1)
+		return NULL;
+	buffer += --buflen;
+	*buffer = '\0';
+
+	while (table) {
+		int namelen = strlen(table->procname);
+
+		if (buflen < namelen + 1)
+			return NULL;
+		buflen -= namelen + 1;
+		buffer -= namelen;
+		memcpy(buffer, table->procname, namelen);
+		*--buffer = '/';
+		table = table->parent;
+	}
+	if (buflen < 4)
+		return NULL;
+	buffer -= 4;
+	memcpy(buffer, "/sys", 4);
+
+	return buffer;
+}
+EXPORT_SYMBOL(sysctl_pathname);
+
 #ifdef CONFIG_SYSCTL_SYSCALL
 /* Perform the actual read/write of a sysctl table entry. */
 static int do_sysctl_strategy(struct ctl_table_root *root,
Index: linux-2.6/security/selinux/hooks.c
===================================================================
--- linux-2.6.orig/security/selinux/hooks.c	2008-05-29 12:46:20.000000000 +0200
+++ linux-2.6/security/selinux/hooks.c	2008-05-29 12:46:25.000000000 +0200
@@ -1736,40 +1736,15 @@ static int selinux_capable(struct task_s
 
 static int selinux_sysctl_get_sid(ctl_table *table, u16 tclass, u32 *sid)
 {
-	int buflen, rc;
-	char *buffer, *path, *end;
+	char *buffer, *path;
+	int rc = -ENOMEM;
 
-	rc = -ENOMEM;
 	buffer = (char *)__get_free_page(GFP_KERNEL);
 	if (!buffer)
 		goto out;
-
-	buflen = PAGE_SIZE;
-	end = buffer+buflen;
-	*--end = '\0';
-	buflen--;
-	path = end-1;
-	*path = '/';
-	while (table) {
-		const char *name = table->procname;
-		size_t namelen = strlen(name);
-		buflen -= namelen + 1;
-		if (buflen < 0)
-			goto out_free;
-		end -= namelen;
-		memcpy(end, name, namelen);
-		*--end = '/';
-		path = end;
-		table = table->parent;
-	}
-	buflen -= 4;
-	if (buflen < 0)
-		goto out_free;
-	end -= 4;
-	memcpy(end, "/sys", 4);
-	path = end;
-	rc = security_genfs_sid("proc", path, tclass, sid);
-out_free:
+	path = sysctl_pathname(table, buffer, PAGE_SIZE);
+	if (path)
+		rc = security_genfs_sid("proc", path, tclass, sid);
 	free_page((unsigned long)buffer);
 out:
 	return rc;

--

Re: [patch 6/8] Factor out sysctl pathname code

[Christoph Hellwig]

On Thu, May 29, 2008 at 01:32:51PM +0200, Miklos Szeredi wrote:
> Convert the selinux sysctl pathname computation code into a standalone
> function.

No point bloating core kernel for selinux mess.  And this whole routine
should rather go away rather than moving it to core code.  While doing
pathname based lookup for the label might work for the limited case
of sysctl where there are no symlinks but is a rather dumb idea in
general.  And reconstructing this path from the sysctl tables is twice
as dumb.

Re: [patch 6/8] Factor out sysctl pathname code

[Stephen Smalley]

On Sat, 2008-05-31 at 04:27 -0400, Christoph Hellwig wrote:
> On Thu, May 29, 2008 at 01:32:51PM +0200, Miklos Szeredi wrote:
> > Convert the selinux sysctl pathname computation code into a standalone
> > function.
> 
> No point bloating core kernel for selinux mess.  And this whole routine
> should rather go away rather than moving it to core code.  While doing
> pathname based lookup for the label might work for the limited case
> of sysctl where there are no symlinks but is a rather dumb idea in
> general.  And reconstructing this path from the sysctl tables is twice
> as dumb.

I didn't see an alternative for fine-grained labeling of sysctl - the
pathname was the only stable key I could use as an index into policy;
xattrs or the like didn't make sense there.  And generating the pathname
from the sysctl tables ensured that we obtained a stable result that
wasn't mutable by userspace.  Do you have an alternative suggestion?

-- 
Stephen Smalley
National Security Agency

[patch 7/8] vfs: clean up getattr API

[Miklos Szeredi]

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

From: Miklos Szeredi <mszeredi@suse.cz>

Instead of a separate vfsmount and dentry arguments, pass a 'struct
path' to getattr related function.  Don't touch i_op->getattr()
though, because the vfsmount should be removed altogether.

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---
 drivers/block/loop.c       |    2 +-
 fs/gfs2/ops_fstype.c       |    2 +-
 fs/nfsd/nfs3proc.c         |    5 +++--
 fs/nfsd/nfs3xdr.c          |    9 ++++++---
 fs/nfsd/nfs4xdr.c          |   10 +++++++---
 fs/nfsd/nfsproc.c          |   32 ++++++++++++++++++++------------
 fs/nfsd/nfsxdr.c           |    5 ++++-
 fs/nfsd/vfs.c              |    2 +-
 fs/stat.c                  |   16 ++++++++--------
 include/linux/fs.h         |    2 +-
 include/linux/nfsd/nfsd.h  |    1 +
 include/linux/security.h   |   10 ++++------
 security/dummy.c           |    2 +-
 security/security.c        |    6 +++---
 security/selinux/hooks.c   |    4 ++--
 security/smack/smack_lsm.c |    7 +++----
 16 files changed, 66 insertions(+), 49 deletions(-)

Index: linux-2.6/drivers/block/loop.c
===================================================================
--- linux-2.6.orig/drivers/block/loop.c	2008-05-29 12:46:20.000000000 +0200
+++ linux-2.6/drivers/block/loop.c	2008-05-29 12:46:25.000000000 +0200
@@ -1007,7 +1007,7 @@ loop_get_status(struct loop_device *lo, 
 
 	if (lo->lo_state != Lo_bound)
 		return -ENXIO;
-	error = vfs_getattr(file->f_path.mnt, file->f_path.dentry, &stat);
+	error = path_getattr(&file->f_path, &stat);
 	if (error)
 		return error;
 	memset(info, 0, sizeof(*info));
Index: linux-2.6/fs/gfs2/ops_fstype.c
===================================================================
--- linux-2.6.orig/fs/gfs2/ops_fstype.c	2008-05-29 12:46:20.000000000 +0200
+++ linux-2.6/fs/gfs2/ops_fstype.c	2008-05-29 12:46:25.000000000 +0200
@@ -952,7 +952,7 @@ static struct super_block* get_gfs2_sb(c
 		       dev_name);
 		goto out;
 	}
-	error = vfs_getattr(nd.path.mnt, nd.path.dentry, &stat);
+	error = path_getattr(&nd.path, &stat);
 
 	list_for_each_entry(s, &gfs2_fs_type.fs_supers, s_instances) {
 		if ((S_ISBLK(stat.mode) && s->s_dev == stat.rdev) ||
Index: linux-2.6/fs/nfsd/nfs3proc.c
===================================================================
--- linux-2.6.orig/fs/nfsd/nfs3proc.c	2008-05-29 12:46:23.000000000 +0200
+++ linux-2.6/fs/nfsd/nfs3proc.c	2008-05-29 12:46:25.000000000 +0200
@@ -58,6 +58,7 @@ nfsd3_proc_getattr(struct svc_rqst *rqst
 {
 	int	err;
 	__be32	nfserr;
+	struct path path;
 
 	dprintk("nfsd: GETATTR(3)  %s\n",
 		SVCFH_fmt(&argp->fh));
@@ -67,8 +68,8 @@ nfsd3_proc_getattr(struct svc_rqst *rqst
 	if (nfserr)
 		RETURN_STATUS(nfserr);
 
-	err = vfs_getattr(resp->fh.fh_export->ex_path.mnt,
-			  resp->fh.fh_dentry, &resp->stat);
+	fh_to_path(&resp->fh, &path);
+	err = path_getattr(&path, &resp->stat);
 	nfserr = nfserrno(err);
 
 	RETURN_STATUS(nfserr);
Index: linux-2.6/fs/nfsd/nfs3xdr.c
===================================================================
--- linux-2.6.orig/fs/nfsd/nfs3xdr.c	2008-05-29 12:46:20.000000000 +0200
+++ linux-2.6/fs/nfsd/nfs3xdr.c	2008-05-29 12:46:25.000000000 +0200
@@ -217,8 +217,10 @@ encode_post_op_attr(struct svc_rqst *rqs
 	if (dentry && dentry->d_inode) {
 	        int err;
 		struct kstat stat;
+		struct path path;
 
-		err = vfs_getattr(fhp->fh_export->ex_path.mnt, dentry, &stat);
+		fh_to_path(fhp, &path);
+		err = path_getattr(&path, &stat);
 		if (!err) {
 			*p++ = xdr_one;		/* attributes follow */
 			lease_get_mtime(dentry->d_inode, &stat.mtime);
@@ -265,13 +267,14 @@ encode_wcc_data(struct svc_rqst *rqstp, 
  */
 void fill_post_wcc(struct svc_fh *fhp)
 {
+	struct path path;
 	int err;
 
 	if (fhp->fh_post_saved)
 		printk("nfsd: inode locked twice during operation.\n");
 
-	err = vfs_getattr(fhp->fh_export->ex_path.mnt, fhp->fh_dentry,
-			&fhp->fh_post_attr);
+	fh_to_path(fhp, &path);
+	err = path_getattr(&path, &fhp->fh_post_attr);
 	if (err)
 		fhp->fh_post_saved = 0;
 	else
Index: linux-2.6/fs/nfsd/nfs4xdr.c
===================================================================
--- linux-2.6.orig/fs/nfsd/nfs4xdr.c	2008-05-29 12:46:20.000000000 +0200
+++ linux-2.6/fs/nfsd/nfs4xdr.c	2008-05-29 12:46:25.000000000 +0200
@@ -1456,6 +1456,7 @@ nfsd4_encode_fattr(struct svc_fh *fhp, s
 	int err;
 	int aclsupport = 0;
 	struct nfs4_acl *acl = NULL;
+	struct path path;
 
 	BUG_ON(bmval1 & NFSD_WRITEONLY_ATTRS_WORD1);
 	BUG_ON(bmval0 & ~NFSD_SUPPORTED_ATTRS_WORD0);
@@ -1467,7 +1468,9 @@ nfsd4_encode_fattr(struct svc_fh *fhp, s
 			goto out;
 	}
 
-	err = vfs_getattr(exp->ex_path.mnt, dentry, &stat);
+	path.mnt = exp->ex_path.mnt;
+	path.dentry = dentry;
+	err = path_getattr(&path, &stat);
 	if (err)
 		goto out_nfserr;
 	if ((bmval0 & (FATTR4_WORD0_FILES_FREE | FATTR4_WORD0_FILES_TOTAL |
@@ -1825,8 +1828,9 @@ out_acl:
 		 */
 		if (ignore_crossmnt == 0 &&
 		    exp->ex_path.mnt->mnt_root->d_inode == dentry->d_inode) {
-			err = vfs_getattr(exp->ex_path.mnt->mnt_parent,
-				exp->ex_path.mnt->mnt_mountpoint, &stat);
+			path.mnt = exp->ex_path.mnt->mnt_parent;
+			path.dentry = exp->ex_path.mnt->mnt_mountpoint;
+			err = path_getattr(&path, &stat);
 			if (err)
 				goto out_nfserr;
 		}
Index: linux-2.6/fs/nfsd/nfsproc.c
===================================================================
--- linux-2.6.orig/fs/nfsd/nfsproc.c	2008-05-29 12:46:23.000000000 +0200
+++ linux-2.6/fs/nfsd/nfsproc.c	2008-05-29 12:46:25.000000000 +0200
@@ -40,18 +40,24 @@ nfsd_proc_null(struct svc_rqst *rqstp, v
 static __be32
 nfsd_return_attrs(__be32 err, struct nfsd_attrstat *resp)
 {
-	if (err) return err;
-	return nfserrno(vfs_getattr(resp->fh.fh_export->ex_path.mnt,
-				    resp->fh.fh_dentry,
-				    &resp->stat));
+	struct path path;
+
+	if (err)
+		return err;
+
+	fh_to_path(&resp->fh, &path);
+	return nfserrno(path_getattr(&path, &resp->stat));
 }
 static __be32
 nfsd_return_dirop(__be32 err, struct nfsd_diropres *resp)
 {
-	if (err) return err;
-	return nfserrno(vfs_getattr(resp->fh.fh_export->ex_path.mnt,
-				    resp->fh.fh_dentry,
-				    &resp->stat));
+	struct path path;
+
+	if (err)
+		return err;
+
+	fh_to_path(&resp->fh, &path);
+	return nfserrno(path_getattr(&path, &resp->stat));
 }
 /*
  * Get a file's attributes
@@ -137,6 +143,7 @@ static __be32
 nfsd_proc_read(struct svc_rqst *rqstp, struct nfsd_readargs *argp,
 				       struct nfsd_readres  *resp)
 {
+	struct path path;
 	__be32	nfserr;
 
 	dprintk("nfsd: READ    %s %d bytes at %d\n",
@@ -163,10 +170,11 @@ nfsd_proc_read(struct svc_rqst *rqstp, s
 			   	  rqstp->rq_vec, argp->vlen,
 				  &resp->count);
 
-	if (nfserr) return nfserr;
-	return nfserrno(vfs_getattr(resp->fh.fh_export->ex_path.mnt,
-				    resp->fh.fh_dentry,
-				    &resp->stat));
+	if (nfserr)
+		return nfserr;
+
+	fh_to_path(&resp->fh, &path);
+	return nfserrno(path_getattr(&path, &resp->stat));
 }
 
 /*
Index: linux-2.6/fs/nfsd/nfsxdr.c
===================================================================
--- linux-2.6.orig/fs/nfsd/nfsxdr.c	2008-05-29 12:46:20.000000000 +0200
+++ linux-2.6/fs/nfsd/nfsxdr.c	2008-05-29 12:46:25.000000000 +0200
@@ -207,7 +207,10 @@ encode_fattr(struct svc_rqst *rqstp, __b
 __be32 *nfs2svc_encode_fattr(struct svc_rqst *rqstp, __be32 *p, struct svc_fh *fhp)
 {
 	struct kstat stat;
-	vfs_getattr(fhp->fh_export->ex_path.mnt, fhp->fh_dentry, &stat);
+	struct path path;
+
+	fh_to_path(fhp, &path);
+	path_getattr(&path, &stat);
 	return encode_fattr(rqstp, p, fhp, &stat);
 }
 
Index: linux-2.6/fs/nfsd/vfs.c
===================================================================
--- linux-2.6.orig/fs/nfsd/vfs.c	2008-05-29 12:46:23.000000000 +0200
+++ linux-2.6/fs/nfsd/vfs.c	2008-05-29 12:46:25.000000000 +0200
@@ -130,7 +130,7 @@ out:
 	return err;
 }
 
-static void fh_to_path(struct svc_fh *fhp, struct path *path)
+void fh_to_path(struct svc_fh *fhp, struct path *path)
 {
 	path->dentry = fhp->fh_dentry;
 	path->mnt = fhp->fh_export->ex_path.mnt;
Index: linux-2.6/fs/stat.c
===================================================================
--- linux-2.6.orig/fs/stat.c	2008-05-29 12:46:20.000000000 +0200
+++ linux-2.6/fs/stat.c	2008-05-29 12:46:25.000000000 +0200
@@ -37,23 +37,23 @@ void generic_fillattr(struct inode *inod
 
 EXPORT_SYMBOL(generic_fillattr);
 
-int vfs_getattr(struct vfsmount *mnt, struct dentry *dentry, struct kstat *stat)
+int path_getattr(struct path *path, struct kstat *stat)
 {
-	struct inode *inode = dentry->d_inode;
+	struct inode *inode = path->dentry->d_inode;
 	int retval;
 
-	retval = security_inode_getattr(mnt, dentry);
+	retval = security_inode_getattr(path);
 	if (retval)
 		return retval;
 
 	if (inode->i_op->getattr)
-		return inode->i_op->getattr(mnt, dentry, stat);
+		return inode->i_op->getattr(path->mnt, path->dentry, stat);
 
 	generic_fillattr(inode, stat);
 	return 0;
 }
 
-EXPORT_SYMBOL(vfs_getattr);
+EXPORT_SYMBOL(path_getattr);
 
 int vfs_stat_fd(int dfd, char __user *name, struct kstat *stat)
 {
@@ -62,7 +62,7 @@ int vfs_stat_fd(int dfd, char __user *na
 
 	error = __user_walk_fd(dfd, name, LOOKUP_FOLLOW, &nd);
 	if (!error) {
-		error = vfs_getattr(nd.path.mnt, nd.path.dentry, stat);
+		error = path_getattr(&nd.path, stat);
 		path_put(&nd.path);
 	}
 	return error;
@@ -82,7 +82,7 @@ int vfs_lstat_fd(int dfd, char __user *n
 
 	error = __user_walk_fd(dfd, name, 0, &nd);
 	if (!error) {
-		error = vfs_getattr(nd.path.mnt, nd.path.dentry, stat);
+		error = path_getattr(&nd.path, stat);
 		path_put(&nd.path);
 	}
 	return error;
@@ -101,7 +101,7 @@ int vfs_fstat(unsigned int fd, struct ks
 	int error = -EBADF;
 
 	if (f) {
-		error = vfs_getattr(f->f_path.mnt, f->f_path.dentry, stat);
+		error = path_getattr(&f->f_path, stat);
 		fput(f);
 	}
 	return error;
Index: linux-2.6/include/linux/fs.h
===================================================================
--- linux-2.6.orig/include/linux/fs.h	2008-05-29 12:46:24.000000000 +0200
+++ linux-2.6/include/linux/fs.h	2008-05-29 12:46:25.000000000 +0200
@@ -1977,7 +1977,7 @@ extern int page_symlink(struct inode *in
 extern const struct inode_operations page_symlink_inode_operations;
 extern int generic_readlink(struct dentry *, char __user *, int);
 extern void generic_fillattr(struct inode *, struct kstat *);
-extern int vfs_getattr(struct vfsmount *, struct dentry *, struct kstat *);
+extern int path_getattr(struct path *, struct kstat *);
 void inode_add_bytes(struct inode *inode, loff_t bytes);
 void inode_sub_bytes(struct inode *inode, loff_t bytes);
 loff_t inode_get_bytes(struct inode *inode);
Index: linux-2.6/include/linux/nfsd/nfsd.h
===================================================================
--- linux-2.6.orig/include/linux/nfsd/nfsd.h	2008-05-29 12:46:23.000000000 +0200
+++ linux-2.6/include/linux/nfsd/nfsd.h	2008-05-29 12:46:25.000000000 +0200
@@ -71,6 +71,7 @@ int		nfsd_set_nrthreads(int n, int *);
 
 /* nfsd/vfs.c */
 int		fh_lock_parent(struct svc_fh *, struct dentry *);
+void		fh_to_path(struct svc_fh *, struct path *);
 int		nfsd_racache_init(int);
 void		nfsd_racache_shutdown(void);
 int		nfsd_cross_mnt(struct svc_rqst *rqstp, struct dentry **dpp,
Index: linux-2.6/include/linux/security.h
===================================================================
--- linux-2.6.orig/include/linux/security.h	2008-05-29 12:46:20.000000000 +0200
+++ linux-2.6/include/linux/security.h	2008-05-29 12:46:25.000000000 +0200
@@ -418,8 +418,7 @@ static inline void security_free_mnt_opt
  *	Return 0 if permission is granted.
  * @inode_getattr:
  *	Check permission before obtaining file attributes.
- *	@mnt is the vfsmount where the dentry was looked up
- *	@dentry contains the dentry structure for the file.
+ *	@path the path to check
  *	Return 0 if permission is granted.
  * @inode_delete:
  *	@inode contains the inode structure for deleted inode.
@@ -1371,7 +1370,7 @@ struct security_operations {
 	int (*inode_follow_link) (struct dentry *dentry, struct nameidata *nd);
 	int (*inode_permission) (struct inode *inode, int mask);
 	int (*inode_setattr)	(struct dentry *dentry, struct iattr *attr);
-	int (*inode_getattr) (struct vfsmount *mnt, struct dentry *dentry);
+	int (*inode_getattr) (struct path *path);
 	void (*inode_delete) (struct inode *inode);
 	int (*inode_setxattr) (struct dentry *dentry, const char *name,
 			       const void *value, size_t size, int flags);
@@ -1642,7 +1641,7 @@ int security_inode_readlink(struct dentr
 int security_inode_follow_link(struct dentry *dentry, struct nameidata *nd);
 int security_inode_permission(struct inode *inode, int mask);
 int security_inode_setattr(struct dentry *dentry, struct iattr *attr);
-int security_inode_getattr(struct vfsmount *mnt, struct dentry *dentry);
+int security_inode_getattr(struct path *path);
 void security_inode_delete(struct inode *inode);
 int security_inode_setxattr(struct dentry *dentry, const char *name,
 			    const void *value, size_t size, int flags);
@@ -2042,8 +2041,7 @@ static inline int security_inode_setattr
 	return 0;
 }
 
-static inline int security_inode_getattr(struct vfsmount *mnt,
-					  struct dentry *dentry)
+static inline int security_inode_getattr(struct path *path)
 {
 	return 0;
 }
Index: linux-2.6/security/dummy.c
===================================================================
--- linux-2.6.orig/security/dummy.c	2008-05-29 12:46:20.000000000 +0200
+++ linux-2.6/security/dummy.c	2008-05-29 12:46:25.000000000 +0200
@@ -355,7 +355,7 @@ static int dummy_inode_setattr (struct d
 	return 0;
 }
 
-static int dummy_inode_getattr (struct vfsmount *mnt, struct dentry *dentry)
+static int dummy_inode_getattr(struct path *path)
 {
 	return 0;
 }
Index: linux-2.6/security/security.c
===================================================================
--- linux-2.6.orig/security/security.c	2008-05-29 12:46:20.000000000 +0200
+++ linux-2.6/security/security.c	2008-05-29 12:46:25.000000000 +0200
@@ -478,11 +478,11 @@ int security_inode_setattr(struct dentry
 }
 EXPORT_SYMBOL_GPL(security_inode_setattr);
 
-int security_inode_getattr(struct vfsmount *mnt, struct dentry *dentry)
+int security_inode_getattr(struct path *path)
 {
-	if (unlikely(IS_PRIVATE(dentry->d_inode)))
+	if (unlikely(IS_PRIVATE(path->dentry->d_inode)))
 		return 0;
-	return security_ops->inode_getattr(mnt, dentry);
+	return security_ops->inode_getattr(path);
 }
 
 void security_inode_delete(struct inode *inode)
Index: linux-2.6/security/selinux/hooks.c
===================================================================
--- linux-2.6.orig/security/selinux/hooks.c	2008-05-29 12:46:25.000000000 +0200
+++ linux-2.6/security/selinux/hooks.c	2008-05-29 12:46:25.000000000 +0200
@@ -2590,9 +2590,9 @@ static int selinux_inode_setattr(struct 
 	return dentry_has_perm(current, NULL, dentry, FILE__WRITE);
 }
 
-static int selinux_inode_getattr(struct vfsmount *mnt, struct dentry *dentry)
+static int selinux_inode_getattr(struct path *path)
 {
-	return dentry_has_perm(current, mnt, dentry, FILE__GETATTR);
+	return dentry_has_perm(current, path->mnt, path->dentry, FILE__GETATTR);
 }
 
 static int selinux_inode_setotherxattr(struct dentry *dentry, const char *name)
Index: linux-2.6/security/smack/smack_lsm.c
===================================================================
--- linux-2.6.orig/security/smack/smack_lsm.c	2008-05-29 12:46:20.000000000 +0200
+++ linux-2.6/security/smack/smack_lsm.c	2008-05-29 12:46:25.000000000 +0200
@@ -552,14 +552,13 @@ static int smack_inode_setattr(struct de
 
 /**
  * smack_inode_getattr - Smack check for getting attributes
- * @mnt: unused
- * @dentry: the object
+ * @path: the object
  *
  * Returns 0 if access is permitted, an error code otherwise
  */
-static int smack_inode_getattr(struct vfsmount *mnt, struct dentry *dentry)
+static int smack_inode_getattr(struct path *path)
 {
-	return smk_curacc(smk_of_inode(dentry->d_inode), MAY_READ);
+	return smk_curacc(smk_of_inode(path->dentry->d_inode), MAY_READ);
 }
 
 /**

--

[patch 8/8] vfs: create file_remove_suid() helper

[Miklos Szeredi]

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

From: Miklos Szeredi <mszeredi@suse.cz>

All calls to remove_suid() are made with a file pointer.  This is not
accidental: similarly to file_update_time(), this function is called
when the file is written.

So simplify callers by passing the file pointer instead of the dentry.

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---
 fs/fuse/file.c             |    2 +-
 fs/ntfs/file.c             |    2 +-
 fs/splice.c                |    4 ++--
 fs/xfs/linux-2.6/xfs_lrw.c |    2 +-
 include/linux/fs.h         |    2 +-
 mm/filemap.c               |    7 ++++---
 mm/filemap_xip.c           |    2 +-
 7 files changed, 11 insertions(+), 10 deletions(-)

Index: linux-2.6/fs/ntfs/file.c
===================================================================
--- linux-2.6.orig/fs/ntfs/file.c	2008-05-29 12:46:19.000000000 +0200
+++ linux-2.6/fs/ntfs/file.c	2008-05-29 12:46:26.000000000 +0200
@@ -2118,7 +2118,7 @@ static ssize_t ntfs_file_aio_write_noloc
 		goto out;
 	if (!count)
 		goto out;
-	err = remove_suid(file->f_path.dentry);
+	err = file_remove_suid(file);
 	if (err)
 		goto out;
 	file_update_time(file);
Index: linux-2.6/fs/splice.c
===================================================================
--- linux-2.6.orig/fs/splice.c	2008-05-29 12:46:19.000000000 +0200
+++ linux-2.6/fs/splice.c	2008-05-29 12:46:26.000000000 +0200
@@ -762,7 +762,7 @@ generic_file_splice_write_nolock(struct 
 	ssize_t ret;
 	int err;
 
-	err = remove_suid(out->f_path.dentry);
+	err = file_remove_suid(out);
 	if (unlikely(err))
 		return err;
 
@@ -820,7 +820,7 @@ generic_file_splice_write(struct pipe_in
 	ssize_t ret;
 
 	inode_double_lock(inode, pipe->inode);
-	ret = remove_suid(out->f_path.dentry);
+	ret = file_remove_suid(out);
 	if (likely(!ret))
 		ret = __splice_from_pipe(pipe, &sd, pipe_to_file);
 	inode_double_unlock(inode, pipe->inode);
Index: linux-2.6/fs/xfs/linux-2.6/xfs_lrw.c
===================================================================
--- linux-2.6.orig/fs/xfs/linux-2.6/xfs_lrw.c	2008-05-29 12:46:19.000000000 +0200
+++ linux-2.6/fs/xfs/linux-2.6/xfs_lrw.c	2008-05-29 12:46:26.000000000 +0200
@@ -711,7 +711,7 @@ start:
 	     !capable(CAP_FSETID)) {
 		error = xfs_write_clear_setuid(xip);
 		if (likely(!error))
-			error = -remove_suid(file->f_path.dentry);
+			error = -file_remove_suid(file);
 		if (unlikely(error)) {
 			goto out_unlock_internal;
 		}
Index: linux-2.6/include/linux/fs.h
===================================================================
--- linux-2.6.orig/include/linux/fs.h	2008-05-29 12:46:25.000000000 +0200
+++ linux-2.6/include/linux/fs.h	2008-05-29 12:46:26.000000000 +0200
@@ -1850,7 +1850,7 @@ extern void clear_inode(struct inode *);
 extern void destroy_inode(struct inode *);
 extern struct inode *new_inode(struct super_block *);
 extern int should_remove_suid(struct dentry *);
-extern int remove_suid(struct dentry *);
+extern int file_remove_suid(struct file *);
 
 extern void __insert_inode_hash(struct inode *, unsigned long hashval);
 extern void remove_inode_hash(struct inode *);
Index: linux-2.6/mm/filemap.c
===================================================================
--- linux-2.6.orig/mm/filemap.c	2008-05-29 12:46:19.000000000 +0200
+++ linux-2.6/mm/filemap.c	2008-05-29 12:46:26.000000000 +0200
@@ -1668,8 +1668,9 @@ static int __remove_suid(struct dentry *
 	return notify_change(dentry, &newattrs);
 }
 
-int remove_suid(struct dentry *dentry)
+int file_remove_suid(struct file *file)
 {
+	struct dentry *dentry = file->f_path.dentry;
 	int killsuid = should_remove_suid(dentry);
 	int killpriv = security_inode_need_killpriv(dentry);
 	int error = 0;
@@ -1683,7 +1684,7 @@ int remove_suid(struct dentry *dentry)
 
 	return error;
 }
-EXPORT_SYMBOL(remove_suid);
+EXPORT_SYMBOL(file_remove_suid);
 
 static size_t __iovec_copy_from_user_inatomic(char *vaddr,
 			const struct iovec *iov, size_t base, size_t bytes)
@@ -2394,7 +2395,7 @@ __generic_file_aio_write_nolock(struct k
 	if (count == 0)
 		goto out;
 
-	err = remove_suid(file->f_path.dentry);
+	err = file_remove_suid(file);
 	if (err)
 		goto out;
 
Index: linux-2.6/mm/filemap_xip.c
===================================================================
--- linux-2.6.orig/mm/filemap_xip.c	2008-05-29 12:46:19.000000000 +0200
+++ linux-2.6/mm/filemap_xip.c	2008-05-29 12:46:26.000000000 +0200
@@ -380,7 +380,7 @@ xip_file_write(struct file *filp, const 
 	if (count == 0)
 		goto out_backing;
 
-	ret = remove_suid(filp->f_path.dentry);
+	ret = file_remove_suid(filp);
 	if (ret)
 		goto out_backing;
 
Index: linux-2.6/fs/fuse/file.c
===================================================================
--- linux-2.6.orig/fs/fuse/file.c	2008-05-29 12:46:19.000000000 +0200
+++ linux-2.6/fs/fuse/file.c	2008-05-29 12:46:26.000000000 +0200
@@ -893,7 +893,7 @@ static ssize_t fuse_file_aio_write(struc
 	if (count == 0)
 		goto out;
 
-	err = remove_suid(file->f_path.dentry);
+	err = file_remove_suid(file);
 	if (err)
 		goto out;
 

--

Re: [patch 8/8] vfs: create file_remove_suid() helper

[Christoph Hellwig]

On Thu, May 29, 2008 at 01:32:53PM +0200, Miklos Szeredi wrote:
> From: Miklos Szeredi <mszeredi@suse.cz>
> 
> All calls to remove_suid() are made with a file pointer.  This is not
> accidental: similarly to file_update_time(), this function is called
> when the file is written.
> 
> So simplify callers by passing the file pointer instead of the dentry.

It's not simplifying anything at all.  You just move the pointer
derefence down one step the callchain without any need.

Re: [patch 8/8] vfs: create file_remove_suid() helper

[Miklos Szeredi]

> > From: Miklos Szeredi <mszeredi@suse.cz>
> > 
> > All calls to remove_suid() are made with a file pointer.  This is not
> > accidental: similarly to file_update_time(), this function is called
> > when the file is written.
> > 
> > So simplify callers by passing the file pointer instead of the dentry.
> 
> It's not simplifying anything at all.

Yes it is, read the sentence carefully.

>  You just move the pointer
> derefence down one step the callchain without any need.

Which means, that that pointer dereference will be done once instead
of in each caller, no?  It's a gain (albeit a small one) is in code
cleanliness, and possibly in generated code size.

Miklos

Re: [patch 8/8] vfs: create file_remove_suid() helper

[Christoph Hellwig]

On Sun, Jun 01, 2008 at 10:45:30PM +0200, Miklos Szeredi wrote:
> Which means, that that pointer dereference will be done once instead
> of in each caller, no?  It's a gain (albeit a small one) is in code
> cleanliness, and possibly in generated code size.

Again, it doesn't make anyhing simpler.  It might be a cleanup at best.
I don't really care too much about this one because we could easily
change it back in case a caller appear that just has the inode, but the
tone you set here defintively doesn't help getting these trivial patches
without real gain in :)

Re: [patch 8/8] vfs: create file_remove_suid() helper

[Miklos Szeredi]

> > Which means, that that pointer dereference will be done once instead
> > of in each caller, no?  It's a gain (albeit a small one) is in code
> > cleanliness, and possibly in generated code size.
> 
> Again, it doesn't make anyhing simpler.  It might be a cleanup at best.

OK, that depends what you call simpler :)

Yes it is a cleanup.

Miklos