Hi,
I've recently encountered a problem which could be a bug in the nfs implementation.

It could be illustrated with the following small program,
#include <fcntl.h>

#include <unistd.h>
main()

{

        int fd;
        fd = open("abc", O_WRONLY | O_CREAT, 0644);

        if (fd < 0) {

                perror("open");

                exit(-1);

        }
        write(fd, "test\n", 5);
        setuid(65534);
        if (ftruncate(fd, 3) < 0)

                perror("ftruncate");
        close(fd);

}
Compile and run it as root on an NFS mount without root squash, ftruncate() would

return an EACCESS error. On a local disk, it would complete successfully, leaving

behind a file "abc" with the string "tes". It would also be successful on NFS

if you change the mode from 0644 to 0666 (make sure to set your umask to 0).
I'm not familiar with linux nfs code, but it seems to me that the nfs code does

an additional access mode check in ftruncate/setattr, which is not done on a local

fs. I've checked on freebsd, the program works fine on both local and nfs.
Could someone more familiar with the nfs code take a look? I'm running 2.6.9-42.Elsmp

64-bit, nfsv3 mount. For nfs server, I've tried linux/freebsd and a commercial one

with a proprietary OS.
Thanks

-luoqi
PS: I'm not a subscriber of the linux kernel mailing list, I'd appreciate if any

response could be send to me directly.

--

Known problem. I suspect that the following untested patch (against

2.6.26-rc5) should fix it.
Cheers

  Trond

-------------------------------------------------------------------------------

From: Trond Myklebust <Trond.Myklebust@netapp.com>

Date: Tue, 10 Jun 2008 19:39:41 -0400

NFS: Fix the ftruncate() credential problem
ftruncate() access checking is supposed to be performed at open() time,

just like reads and writes.
Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>

---
 fs/nfs/inode.c          |   10 +++++++---

 fs/nfs/nfs3proc.c       |    7 ++++---

 fs/nfs/nfs4proc.c       |   47 +++++++++++++++++++++++++++--------------------

 fs/nfs/proc.c           |    5 +++--

 include/linux/nfs_xdr.h |    4 ++--

 5 files changed, 43 insertions(+), 30 deletions(-)
diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c

index 596c5d8..072ee6c 100644

--- a/fs/nfs/inode.c

+++ b/fs/nfs/inode.c

@@ -347,13 +347,14 @@ out_no_inode:

 	goto out;

 }
-#define NFS_VALID_ATTRS (ATTR_MODE|ATTR_UID|ATTR_GID|ATTR_SIZE|ATTR_ATIME|ATTR_ATIME_SET|ATTR_MTIME|ATTR_MTIME_SET)

+#define NFS_VALID_ATTRS (ATTR_MODE|ATTR_UID|ATTR_GID|ATTR_SIZE|ATTR_ATIME|ATTR_ATIME_SET|ATTR_MTIME|ATTR_MTIME_SET|ATTR_FILE)
 int

 nfs_setattr(struct dentry *dentry, struct iattr *attr)

 {

 	struct inode *inode = dentry->d_inode;

 	struct nfs_fattr fattr;

+	struct rpc_cred *cred = NULL;

 	int error;
 	nfs_inc_stats(inode, NFSIOS_VFSSETATTR);

@@ -369,9 +370,12 @@ nfs_setattr(struct dentry *dentry, struct iattr *attr)
 	/* Optimization: if the end result is no change, don't RPC */

 	attr->ia_valid &= NFS_VALID_ATTRS;

-	if (attr->ia_valid == 0)

+	if ((attr->ia_valid & ~ATTR_FILE) == 0)

 		return 0;
+	if (attr->ia_valid & ATTR_FILE)

+		cred = nfs_file_cred(attr->ia_file);

+

 	lock_kernel();

 	/* Write all dirty data */

 	if (S_ISREG(inode->i_mode)) {

@@ -383,7 +387,7 @@ nfs_setattr(struct dentry *dentry, struct iattr *attr)

 	 */

 	if ((attr->ia_valid ...
[ message continues ]

Thanks, Trond. Is there any chance this patch could be

included in the linux kernel in the near future? For now,

I guess I'll workaround this problem by moving the ftruncate()

to before setuid().
-luoqi
PS: I haven't tried the patch, just browsing through, and I noticed

a typo, pointing out here to save some trouble for anyone who

wants to give it a try (I guess gcc would issue a warning too),
        /* Search for an existing open(O_WRITE) file */

-       ctx = nfs_find_open_context(inode, cred, FMODE_WRITE);

-       if (ctx != NULL)

-               state = ctx->state;

+       if (sattr->ia_valid && ATTR_FILE) {  <=== && should be &

+               ctx = nfs_file_open_context(sattr->ia_file);

+               if (ctx != NULL)

+                       state = ctx->state;

+       }

--

Thanks for testing! I've refined the patch a bit so that it is less

intrusive w.r.t. common NFS code changes, and fixed a couple more bugs.

As long as the stability proves to be good, I see no reason why we can't

merge this once the 2.6.27 merge window opens.
Cheers

  Trond

-----------------------------------------------------------------------

From: Trond Myklebust <Trond.Myklebust@netapp.com>

Date: Tue, 10 Jun 2008 19:39:41 -0400

NFS: Fix the ftruncate() credential problem
ftruncate() access checking is supposed to be performed at open() time,

just like reads and writes.
Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>

---
 fs/nfs/inode.c    |    4 ++--

 fs/nfs/nfs3proc.c |    2 ++

 fs/nfs/nfs4proc.c |   47 +++++++++++++++++++++++------------------------

 fs/nfs/proc.c     |    2 ++

 4 files changed, 29 insertions(+), 26 deletions(-)
diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c

index 596c5d8..2e4ab4a 100644

--- a/fs/nfs/inode.c

+++ b/fs/nfs/inode.c

@@ -347,7 +347,7 @@ out_no_inode:

 	goto out;

 }
-#define NFS_VALID_ATTRS (ATTR_MODE|ATTR_UID|ATTR_GID|ATTR_SIZE|ATTR_ATIME|ATTR_ATIME_SET|ATTR_MTIME|ATTR_MTIME_SET)

+#define NFS_VALID_ATTRS (ATTR_MODE|ATTR_UID|ATTR_GID|ATTR_SIZE|ATTR_ATIME|ATTR_ATIME_SET|ATTR_MTIME|ATTR_MTIME_SET|ATTR_FILE)
 int

 nfs_setattr(struct dentry *dentry, struct iattr *attr)

@@ -369,7 +369,7 @@ nfs_setattr(struct dentry *dentry, struct iattr *attr)
 	/* Optimization: if the end result is no change, don't RPC */

 	attr->ia_valid &= NFS_VALID_ATTRS;

-	if (attr->ia_valid == 0)

+	if ((attr->ia_valid & ~ATTR_FILE) == 0)

 		return 0;
 	lock_kernel();

diff --git a/fs/nfs/nfs3proc.c b/fs/nfs/nfs3proc.c

index c3523ad..b14b378 100644

--- a/fs/nfs/nfs3proc.c

+++ b/fs/nfs/nfs3proc.c

@@ -129,6 +129,8 @@ nfs3_proc_setattr(struct dentry *dentry, struct nfs_fattr *fattr,

 	int	status;
 	dprintk("NFS call  setattr\n");

+	if (sattr->ia_valid & ATTR_FILE)

+		msg.rpc_cred = nfs_file_cred(sattr...
[ message continues ]