[PATCH] NFS: Ensure we always update inode->i_mode when doing O_EXCL creates

When the client performs an exclusive create and opens the file for writing,
a Netapp filer will first create the file using the mode 01777. It does this
since an NFSv3/v4 exclusive create cannot immediately set the mode bits.
The 01777 mode then gets put into the inode->i_mode. After the file creation
is successful, we then do a setattr to change the mode to the correct value
(as per the NFS spec).

The problem is that nfs_refresh_inode() no longer updates inode->i_mode, so
the latter retains the 01777 mode. A bit later, the VFS notices this, and calls
remove_suid(). This of course now resets the file mode to inode->i_mode & 0777.
Hey presto, the file mode on the server is now magically changed to 0777. Duh...

Fixes http://bugzilla.linux-nfs.org/show_bug.cgi?id=32

Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
Signed-off-by: Linus Torvalds <torvalds@osdl.org>

diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
index 4845911f1c63f3e3436e5591be1b8c08b006a181..bb7ca022bcb2b62df3a19fa44a167ff411899a0b 100644 (file)
--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -814,28 +814,39 @@ nfs_setattr(struct dentry *dentry, struct iattr *attr)
                nfs_wb_all(inode);
        }
        error = NFS_PROTO(inode)->setattr(dentry, &fattr, attr);
-       if (error == 0) {
+       if (error == 0)
                nfs_refresh_inode(inode, &fattr);
+       nfs_end_data_update(inode);
+       unlock_kernel();
+       return error;
+}
+
+/**
+ * nfs_setattr_update_inode - Update inode metadata after a setattr call.
+ * @inode: pointer to struct inode
+ * @attr: pointer to struct iattr
+ *
+ * Note: we do this in the *proc.c in order to ensure that
+ *       it works for things like exclusive creates too.
+ */
+void nfs_setattr_update_inode(struct inode *inode, struct iattr *attr)
+{
+       if ((attr->ia_valid & (ATTR_MODE|ATTR_UID|ATTR_GID)) != 0) {
                if ((attr->ia_valid & ATTR_MODE) != 0) {
-                       int mode;
-                       mode = inode->i_mode & ~S_IALLUGO;
-                       mode |= attr->ia_mode & S_IALLUGO;
+                       int mode = attr->ia_mode & S_IALLUGO;
+                       mode |= inode->i_mode & ~S_IALLUGO;
                        inode->i_mode = mode;
                }
                if ((attr->ia_valid & ATTR_UID) != 0)
                        inode->i_uid = attr->ia_uid;
                if ((attr->ia_valid & ATTR_GID) != 0)
                        inode->i_gid = attr->ia_gid;
-               if ((attr->ia_valid & ATTR_SIZE) != 0) {
-                       inode->i_size = attr->ia_size;
-                       vmtruncate(inode, attr->ia_size);
-               }
-       }
-       if ((attr->ia_valid & (ATTR_MODE|ATTR_UID|ATTR_GID)) != 0)
                NFS_FLAGS(inode) |= NFS_INO_INVALID_ACCESS|NFS_INO_INVALID_ACL;
-       nfs_end_data_update(inode);
-       unlock_kernel();
-       return error;
+       }
+       if ((attr->ia_valid & ATTR_SIZE) != 0) {
+               inode->i_size = attr->ia_size;
+               vmtruncate(inode, attr->ia_size);
+       }
 }
 
 /*

diff --git a/fs/nfs/nfs3proc.c b/fs/nfs/nfs3proc.c
index 7851569b31c63f03a5095f503d0b1b18183273d4..2681485cf2d00f9f9fdd647a8bf1cd9861299104 100644 (file)
--- a/fs/nfs/nfs3proc.c
+++ b/fs/nfs/nfs3proc.c
@@ -120,6 +120,8 @@ nfs3_proc_setattr(struct dentry *dentry, struct nfs_fattr *fattr,
        dprintk("NFS call  setattr\n");
        fattr->valid = 0;
        status = rpc_call(NFS_CLIENT(inode), NFS3PROC_SETATTR, &arg, fattr, 0);
+       if (status == 0)
+               nfs_setattr_update_inode(inode, sattr);
        dprintk("NFS reply setattr: %d\n", status);
        return status;
 }
@@ -370,6 +372,8 @@ again:
                 * not sure this buys us anything (and I'd have
                 * to revamp the NFSv3 XDR code) */
                status = nfs3_proc_setattr(dentry, &fattr, sattr);
+               if (status == 0)
+                       nfs_setattr_update_inode(dentry->d_inode, sattr);
                nfs_refresh_inode(dentry->d_inode, &fattr);
                dprintk("NFS reply setattr (post-create): %d\n", status);
        }

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 1b76f80aedb921c737586fbca6ceeac3f0f522f2..0c5a308e49638171291d22aa3630fc431ac74528 100644 (file)
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -753,6 +753,7 @@ static int _nfs4_do_setattr(struct nfs_server *server, struct nfs_fattr *fattr,
                 .rpc_argp       = &arg,
                 .rpc_resp       = &res,
         };
+       int status;
 
         fattr->valid = 0;
 
@@ -762,7 +763,8 @@ static int _nfs4_do_setattr(struct nfs_server *server, struct nfs_fattr *fattr,
        } else
                memcpy(&arg.stateid, &zero_stateid, sizeof(arg.stateid));
 
-       return rpc_call_sync(server->client, &msg, 0);
+       status = rpc_call_sync(server->client, &msg, 0);
+       return status;
 }
 
 static int nfs4_do_setattr(struct nfs_server *server, struct nfs_fattr *fattr,
@@ -1145,6 +1147,8 @@ nfs4_proc_setattr(struct dentry *dentry, struct nfs_fattr *fattr,
 
        status = nfs4_do_setattr(NFS_SERVER(inode), fattr,
                        NFS_FH(inode), sattr, state);
+       if (status == 0)
+               nfs_setattr_update_inode(inode, sattr);
        if (state != NULL)
                nfs4_close_state(state, FMODE_WRITE);
        put_rpccred(cred);
@@ -1449,8 +1453,10 @@ nfs4_proc_create(struct inode *dir, struct dentry *dentry, struct iattr *sattr,
                struct nfs_fattr fattr;
                status = nfs4_do_setattr(NFS_SERVER(dir), &fattr,
                                     NFS_FH(state->inode), sattr, state);
-               if (status == 0)
+               if (status == 0) {
+                       nfs_setattr_update_inode(state->inode, sattr);
                        goto out;
+               }
        } else if (flags != 0)
                goto out;
        nfs4_close_state(state, flags);

diff --git a/fs/nfs/proc.c b/fs/nfs/proc.c
index cedf636bcf3c9f68b53b6c979d2116f5f4a4511d..be23c3fb9260051b8069ad4afbf2257bed1ca502 100644 (file)
--- a/fs/nfs/proc.c
+++ b/fs/nfs/proc.c
@@ -114,6 +114,8 @@ nfs_proc_setattr(struct dentry *dentry, struct nfs_fattr *fattr,
        dprintk("NFS call  setattr\n");
        fattr->valid = 0;
        status = rpc_call(NFS_CLIENT(inode), NFSPROC_SETATTR, &arg, fattr, 0);
+       if (status == 0)
+               nfs_setattr_update_inode(inode, sattr);
        dprintk("NFS reply setattr: %d\n", status);
        return status;
 }

diff --git a/include/linux/nfs_fs.h b/include/linux/nfs_fs.h
index 8ea249110fb0b5029c88b35717f2ce3e5387f507..7d78a783c64a1bfbfb6aa60281ee651d2be53462 100644 (file)
--- a/include/linux/nfs_fs.h
+++ b/include/linux/nfs_fs.h
@@ -292,6 +292,7 @@ extern int nfs_revalidate_inode(struct nfs_server *server, struct inode *inode);
 extern int __nfs_revalidate_inode(struct nfs_server *, struct inode *);
 extern void nfs_revalidate_mapping(struct inode *inode, struct address_space *mapping);
 extern int nfs_setattr(struct dentry *, struct iattr *);
+extern void nfs_setattr_update_inode(struct inode *inode, struct iattr *attr);
 extern void nfs_begin_attr_update(struct inode *);
 extern void nfs_end_attr_update(struct inode *);
 extern void nfs_begin_data_update(struct inode *);