[PATCH 0/2] NFSv4.2 umask support

[PATCH 0/2] NFSv4.2 umask support

[J. Bruce Fields]

From: "J. Bruce Fields" <bfields@redhat.com>

The following patches allow the umask to be ignored in the presence of
inheritable NFSv4 ACLs.  Otherwise inheritable ACLs can be rendered
mostly useless whenever the umask masks out group bits.

This solves a problem we've seen complaints about for some time, both
upstream and from RHEL users.

The new protocol has been discussed in the IETF working group and is
documented at:

	https://tools.ietf.org/html/draft-ietf-nfsv4-umask-02

It's unlikely that we'll discover problems requiring an incompatible
change, so I think we should consider this for 4.10.

--b.

Andreas Gruenbacher (2):
  nfs: add support for the umask attribute
  nfsd: add support for the umask attribute

 fs/nfs/dir.c              |  7 ++++++-
 fs/nfs/nfs4proc.c         | 21 ++++++++++++++++-----
 fs/nfs/nfs4xdr.c          | 36 ++++++++++++++++++++++++------------
 fs/nfsd/nfs4xdr.c         | 26 +++++++++++++++++++++-----
 fs/nfsd/nfsd.h            |  9 +++++++--
 fs/nfsd/nfssvc.c          |  4 ++--
 include/linux/nfs4.h      |  1 +
 include/linux/nfs_fs_sb.h |  1 +
 include/linux/nfs_xdr.h   |  2 ++
 9 files changed, 80 insertions(+), 27 deletions(-)

-- 
2.7.4

[PATCH 1/2] nfs: add support for the umask attribute

[J. Bruce Fields]

From: Andreas Gruenbacher <agruenba@redhat.com>

Clients can set the umask attribute when creating files to cause the
server to apply it always except when inheriting permissions from the
parent directory.  That way, the new files will end up with the same
permissions as files created locally.

See https://tools.ietf.org/html/draft-ietf-nfsv4-umask-02 for more details.

Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/nfs/dir.c              |  7 ++++++-
 fs/nfs/nfs4proc.c         | 21 ++++++++++++++++-----
 fs/nfs/nfs4xdr.c          | 36 ++++++++++++++++++++++++------------
 include/linux/nfs4.h      |  1 +
 include/linux/nfs_fs_sb.h |  1 +
 include/linux/nfs_xdr.h   |  2 ++
 6 files changed, 50 insertions(+), 18 deletions(-)

diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index 5f1af4cd1a33..eeb111e11972 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -1535,8 +1535,13 @@ int nfs_atomic_open(struct inode *dir, struct dentry *dentry,
 		return -ENAMETOOLONG;
 
 	if (open_flags & O_CREAT) {
+		struct nfs_server *server = NFS_SERVER(dir);
+
+		if (!(server->caps & NFS_CAP_UMASK))
+			mode &= ~current_umask();
+
 		attr.ia_valid |= ATTR_MODE;
-		attr.ia_mode = mode & ~current_umask();
+		attr.ia_mode = mode;
 	}
 	if (open_flags & O_TRUNC) {
 		attr.ia_valid |= ATTR_SIZE;
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 7897826d7c51..da08768e3cb3 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -1239,6 +1239,7 @@ static struct nfs4_opendata *nfs4_opendata_alloc(struct dentry *dentry,
 	p->o_arg.bitmask = nfs4_bitmask(server, label);
 	p->o_arg.open_bitmap = &nfs4_fattr_bitmap[0];
 	p->o_arg.label = nfs4_label_copy(p->a_label, label);
+	p->o_arg.umask = current_umask();
 	p->o_arg.claim = nfs4_map_atomic_open_claim(server, claim);
 	switch (p->o_arg.claim) {
 	case NFS4_OPEN_CLAIM_NULL:
@@ -3277,7 +3278,7 @@ static void nfs4_close_context(struct nfs_open_context *ctx, int is_sync)
 
 #define FATTR4_WORD1_NFS40_MASK (2*FATTR4_WORD1_MOUNTED_ON_FILEID - 1UL)
 #define FATTR4_WORD2_NFS41_MASK (2*FATTR4_WORD2_SUPPATTR_EXCLCREAT - 1UL)
-#define FATTR4_WORD2_NFS42_MASK (2*FATTR4_WORD2_SECURITY_LABEL - 1UL)
+#define FATTR4_WORD2_NFS42_MASK (2*FATTR4_WORD2_UMASK - 1UL)
 
 static int _nfs4_server_capabilities(struct nfs_server *server, struct nfs_fh *fhandle)
 {
@@ -3322,7 +3323,8 @@ static int _nfs4_server_capabilities(struct nfs_server *server, struct nfs_fh *f
 				NFS_CAP_MODE|NFS_CAP_NLINK|NFS_CAP_OWNER|
 				NFS_CAP_OWNER_GROUP|NFS_CAP_ATIME|
 				NFS_CAP_CTIME|NFS_CAP_MTIME|
-				NFS_CAP_SECURITY_LABEL);
+				NFS_CAP_SECURITY_LABEL|
+				NFS_CAP_UMASK);
 		if (res.attr_bitmask[0] & FATTR4_WORD0_ACL &&
 				res.acl_bitmask & ACL4_SUPPORT_ALLOW_ACL)
 			server->caps |= NFS_CAP_ACLS;
@@ -3350,6 +3352,8 @@ static int _nfs4_server_capabilities(struct nfs_server *server, struct nfs_fh *f
 		if (res.attr_bitmask[2] & FATTR4_WORD2_SECURITY_LABEL)
 			server->caps |= NFS_CAP_SECURITY_LABEL;
 #endif
+		if (res.attr_bitmask[2] & FATTR4_WORD2_UMASK)
+			server->caps |= NFS_CAP_UMASK;
 		memcpy(server->attr_bitmask_nl, res.attr_bitmask,
 				sizeof(server->attr_bitmask));
 		server->attr_bitmask_nl[2] &= ~FATTR4_WORD2_SECURITY_LABEL;
@@ -3953,6 +3957,7 @@ static int
 nfs4_proc_create(struct inode *dir, struct dentry *dentry, struct iattr *sattr,
 		 int flags)
 {
+	struct nfs_server *server = NFS_SERVER(dir);
 	struct nfs4_label l, *ilabel = NULL;
 	struct nfs_open_context *ctx;
 	struct nfs4_state *state;
@@ -3964,7 +3969,8 @@ nfs4_proc_create(struct inode *dir, struct dentry *dentry, struct iattr *sattr,
 
 	ilabel = nfs4_label_init_security(dir, dentry, sattr, &l);
 
-	sattr->ia_mode &= ~current_umask();
+	if (!(server->caps & NFS_CAP_UMASK))
+		sattr->ia_mode &= ~current_umask();
 	state = nfs4_do_open(dir, ctx, flags, sattr, ilabel, NULL);
 	if (IS_ERR(state)) {
 		status = PTR_ERR(state);
@@ -4172,6 +4178,7 @@ static struct nfs4_createdata *nfs4_alloc_createdata(struct inode *dir,
 		data->arg.attrs = sattr;
 		data->arg.ftype = ftype;
 		data->arg.bitmask = nfs4_bitmask(server, data->label);
+		data->arg.umask = current_umask();
 		data->res.server = server;
 		data->res.fh = &data->fh;
 		data->res.fattr = &data->fattr;
@@ -4269,13 +4276,15 @@ static int _nfs4_proc_mkdir(struct inode *dir, struct dentry *dentry,
 static int nfs4_proc_mkdir(struct inode *dir, struct dentry *dentry,
 		struct iattr *sattr)
 {
+	struct nfs_server *server = NFS_SERVER(dir);
 	struct nfs4_exception exception = { };
 	struct nfs4_label l, *label = NULL;
 	int err;
 
 	label = nfs4_label_init_security(dir, dentry, sattr, &l);
 
-	sattr->ia_mode &= ~current_umask();
+	if (!(server->caps & NFS_CAP_UMASK))
+		sattr->ia_mode &= ~current_umask();
 	do {
 		err = _nfs4_proc_mkdir(dir, dentry, sattr, label);
 		trace_nfs4_mkdir(dir, &dentry->d_name, err);
@@ -4378,13 +4387,15 @@ static int _nfs4_proc_mknod(struct inode *dir, struct dentry *dentry,
 static int nfs4_proc_mknod(struct inode *dir, struct dentry *dentry,
 		struct iattr *sattr, dev_t rdev)
 {
+	struct nfs_server *server = NFS_SERVER(dir);
 	struct nfs4_exception exception = { };
 	struct nfs4_label l, *label = NULL;
 	int err;
 
 	label = nfs4_label_init_security(dir, dentry, sattr, &l);
 
-	sattr->ia_mode &= ~current_umask();
+	if (!(server->caps & NFS_CAP_UMASK))
+		sattr->ia_mode &= ~current_umask();
 	do {
 		err = _nfs4_proc_mknod(dir, dentry, sattr, label, rdev);
 		trace_nfs4_mknod(dir, &dentry->d_name, err);
diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
index fc89e5ed07ee..09c1b63c73e8 100644
--- a/fs/nfs/nfs4xdr.c
+++ b/fs/nfs/nfs4xdr.c
@@ -52,6 +52,7 @@
 #include <linux/nfs.h>
 #include <linux/nfs4.h>
 #include <linux/nfs_fs.h>
+#include <linux/fs_struct.h>
 
 #include "nfs4_fs.h"
 #include "internal.h"
@@ -1003,7 +1004,7 @@ static void encode_nfs4_verifier(struct xdr_stream *xdr, const nfs4_verifier *ve
 static void encode_attrs(struct xdr_stream *xdr, const struct iattr *iap,
 				const struct nfs4_label *label,
 				const struct nfs_server *server,
-				bool excl_check)
+				bool excl_check, const umode_t *umask)
 {
 	char owner_name[IDMAP_NAMESZ];
 	char owner_group[IDMAP_NAMESZ];
@@ -1017,18 +1018,21 @@ static void encode_attrs(struct xdr_stream *xdr, const struct iattr *iap,
 
 	/*
 	 * We reserve enough space to write the entire attribute buffer at once.
-	 * In the worst-case, this would be
-	 * 16(bitmap) + 4(attrlen) + 8(size) + 4(mode) + 4(atime) + 4(mtime)
-	 * = 40 bytes, plus any contribution from variable-length fields
-	 *            such as owner/group.
 	 */
 	if (iap->ia_valid & ATTR_SIZE) {
 		bmval[0] |= FATTR4_WORD0_SIZE;
 		len += 8;
 	}
+	if (!(server->caps & NFS_CAP_UMASK))
+		umask = NULL;
 	if (iap->ia_valid & ATTR_MODE) {
-		bmval[1] |= FATTR4_WORD1_MODE;
-		len += 4;
+		if (umask) {
+			bmval[2] |= FATTR4_WORD2_UMASK;
+			len += 8;
+		} else {
+			bmval[1] |= FATTR4_WORD1_MODE;
+			len += 4;
+		}
 	}
 	if (iap->ia_valid & ATTR_UID) {
 		owner_namelen = nfs_map_uid_to_name(server, iap->ia_uid, owner_name, IDMAP_NAMESZ);
@@ -1129,6 +1133,10 @@ static void encode_attrs(struct xdr_stream *xdr, const struct iattr *iap,
 		*p++ = cpu_to_be32(label->len);
 		p = xdr_encode_opaque_fixed(p, label->label, label->len);
 	}
+	if (bmval[2] & FATTR4_WORD2_UMASK) {
+		*p++ = cpu_to_be32(iap->ia_mode & S_IALLUGO);
+		*p++ = cpu_to_be32(*umask);
+	}
 
 /* out: */
 }
@@ -1183,7 +1191,8 @@ static void encode_create(struct xdr_stream *xdr, const struct nfs4_create_arg *
 	}
 
 	encode_string(xdr, create->name->len, create->name->name);
-	encode_attrs(xdr, create->attrs, create->label, create->server, false);
+	encode_attrs(xdr, create->attrs, create->label, create->server, false,
+		     &create->umask);
 }
 
 static void encode_getattr_one(struct xdr_stream *xdr, uint32_t bitmap, struct compound_hdr *hdr)
@@ -1403,11 +1412,13 @@ static inline void encode_createmode(struct xdr_stream *xdr, const struct nfs_op
 	switch(arg->createmode) {
 	case NFS4_CREATE_UNCHECKED:
 		*p = cpu_to_be32(NFS4_CREATE_UNCHECKED);
-		encode_attrs(xdr, arg->u.attrs, arg->label, arg->server, false);
+		encode_attrs(xdr, arg->u.attrs, arg->label, arg->server, false,
+			     &arg->umask);
 		break;
 	case NFS4_CREATE_GUARDED:
 		*p = cpu_to_be32(NFS4_CREATE_GUARDED);
-		encode_attrs(xdr, arg->u.attrs, arg->label, arg->server, false);
+		encode_attrs(xdr, arg->u.attrs, arg->label, arg->server, false,
+			     &arg->umask);
 		break;
 	case NFS4_CREATE_EXCLUSIVE:
 		*p = cpu_to_be32(NFS4_CREATE_EXCLUSIVE);
@@ -1416,7 +1427,8 @@ static inline void encode_createmode(struct xdr_stream *xdr, const struct nfs_op
 	case NFS4_CREATE_EXCLUSIVE4_1:
 		*p = cpu_to_be32(NFS4_CREATE_EXCLUSIVE4_1);
 		encode_nfs4_verifier(xdr, &arg->u.verifier);
-		encode_attrs(xdr, arg->u.attrs, arg->label, arg->server, true);
+		encode_attrs(xdr, arg->u.attrs, arg->label, arg->server, true,
+			     &arg->umask);
 	}
 }
 
@@ -1672,7 +1684,7 @@ static void encode_setattr(struct xdr_stream *xdr, const struct nfs_setattrargs
 {
 	encode_op_hdr(xdr, OP_SETATTR, decode_setattr_maxsz, hdr);
 	encode_nfs4_stateid(xdr, &arg->stateid);
-	encode_attrs(xdr, arg->iap, arg->label, server, false);
+	encode_attrs(xdr, arg->iap, arg->label, server, false, NULL);
 }
 
 static void encode_setclientid(struct xdr_stream *xdr, const struct nfs4_setclientid *setclientid, struct compound_hdr *hdr)
diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h
index 9094faf0699d..2f0d37937afc 100644
--- a/include/linux/nfs4.h
+++ b/include/linux/nfs4.h
@@ -440,6 +440,7 @@ enum lock_type4 {
 #define FATTR4_WORD2_MDSTHRESHOLD       (1UL << 4)
 #define FATTR4_WORD2_CLONE_BLKSIZE	(1UL << 13)
 #define FATTR4_WORD2_SECURITY_LABEL     (1UL << 16)
+#define FATTR4_WORD2_UMASK		(1UL << 17)
 
 /* MDS threshold bitmap bits */
 #define THRESHOLD_RD                    (1UL << 0)
diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
index b34097c67848..001625fae763 100644
--- a/include/linux/nfs_fs_sb.h
+++ b/include/linux/nfs_fs_sb.h
@@ -250,5 +250,6 @@ struct nfs_server {
 #define NFS_CAP_LAYOUTSTATS	(1U << 22)
 #define NFS_CAP_CLONE		(1U << 23)
 #define NFS_CAP_COPY		(1U << 24)
+#define NFS_CAP_UMASK		(1U << 25)
 
 #endif
diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
index beb1e10f446e..ff82f42da656 100644
--- a/include/linux/nfs_xdr.h
+++ b/include/linux/nfs_xdr.h
@@ -418,6 +418,7 @@ struct nfs_openargs {
 	enum open_claim_type4	claim;
 	enum createmode4	createmode;
 	const struct nfs4_label *label;
+	umode_t			umask;
 };
 
 struct nfs_openres {
@@ -937,6 +938,7 @@ struct nfs4_create_arg {
 	const struct nfs_fh *		dir_fh;
 	const u32 *			bitmask;
 	const struct nfs4_label		*label;
+	umode_t				umask;
 };
 
 struct nfs4_create_res {
-- 
2.7.4

[PATCH 2/2] nfsd: add support for the umask attribute

[J. Bruce Fields]

From: Andreas Gruenbacher <agruenba@redhat.com>

Clients can set the umask attribute when creating files to cause the
server to apply it always except when inheriting permissions from the
parent directory.  That way, the new files will end up with the same
permissions as files created locally.

See https://tools.ietf.org/html/draft-ietf-nfsv4-umask-02 for more
details.

Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/nfsd/nfs4xdr.c | 26 +++++++++++++++++++++-----
 fs/nfsd/nfsd.h    |  9 +++++++--
 fs/nfsd/nfssvc.c  |  4 ++--
 3 files changed, 30 insertions(+), 9 deletions(-)

diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 580bff4920d6..da808254161b 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -33,6 +33,7 @@
  *  SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
+#include <linux/fs_struct.h>
 #include <linux/file.h>
 #include <linux/slab.h>
 #include <linux/namei.h>
@@ -299,7 +300,7 @@ nfsd4_decode_bitmap(struct nfsd4_compoundargs *argp, u32 *bmval)
 static __be32
 nfsd4_decode_fattr(struct nfsd4_compoundargs *argp, u32 *bmval,
 		   struct iattr *iattr, struct nfs4_acl **acl,
-		   struct xdr_netobj *label)
+		   struct xdr_netobj *label, int *umask)
 {
 	int expected_len, len = 0;
 	u32 dummy32;
@@ -457,6 +458,17 @@ nfsd4_decode_fattr(struct nfsd4_compoundargs *argp, u32 *bmval,
 			return nfserr_jukebox;
 	}
 #endif
+	if (bmval[2] & FATTR4_WORD2_UMASK) {
+		if (!umask)
+			goto xdr_error;
+		READ_BUF(8);
+		len += 8;
+		dummy32 = be32_to_cpup(p++);
+		iattr->ia_mode = dummy32 & (S_IFMT | S_IALLUGO);
+		dummy32 = be32_to_cpup(p++);
+		*umask = dummy32 & S_IRWXUGO;
+		iattr->ia_valid |= ATTR_MODE;
+	}
 	if (len != expected_len)
 		goto xdr_error;
 
@@ -651,7 +663,8 @@ nfsd4_decode_create(struct nfsd4_compoundargs *argp, struct nfsd4_create *create
 		return status;
 
 	status = nfsd4_decode_fattr(argp, create->cr_bmval, &create->cr_iattr,
-				    &create->cr_acl, &create->cr_label);
+				    &create->cr_acl, &create->cr_label,
+				    &current->fs->umask);
 	if (status)
 		goto out;
 
@@ -896,13 +909,15 @@ nfsd4_decode_open(struct nfsd4_compoundargs *argp, struct nfsd4_open *open)
 	case NFS4_OPEN_NOCREATE:
 		break;
 	case NFS4_OPEN_CREATE:
+		current->fs->umask = 0;
 		READ_BUF(4);
 		open->op_createmode = be32_to_cpup(p++);
 		switch (open->op_createmode) {
 		case NFS4_CREATE_UNCHECKED:
 		case NFS4_CREATE_GUARDED:
 			status = nfsd4_decode_fattr(argp, open->op_bmval,
-				&open->op_iattr, &open->op_acl, &open->op_label);
+				&open->op_iattr, &open->op_acl, &open->op_label,
+				&current->fs->umask);
 			if (status)
 				goto out;
 			break;
@@ -916,7 +931,8 @@ nfsd4_decode_open(struct nfsd4_compoundargs *argp, struct nfsd4_open *open)
 			READ_BUF(NFS4_VERIFIER_SIZE);
 			COPYMEM(open->op_verf.data, NFS4_VERIFIER_SIZE);
 			status = nfsd4_decode_fattr(argp, open->op_bmval,
-				&open->op_iattr, &open->op_acl, &open->op_label);
+				&open->op_iattr, &open->op_acl, &open->op_label,
+				&current->fs->umask);
 			if (status)
 				goto out;
 			break;
@@ -1153,7 +1169,7 @@ nfsd4_decode_setattr(struct nfsd4_compoundargs *argp, struct nfsd4_setattr *seta
 	if (status)
 		return status;
 	return nfsd4_decode_fattr(argp, setattr->sa_bmval, &setattr->sa_iattr,
-				  &setattr->sa_acl, &setattr->sa_label);
+				  &setattr->sa_acl, &setattr->sa_label, NULL);
 }
 
 static __be32
diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
index 7155239b2908..0d5bb0a198c4 100644
--- a/fs/nfsd/nfsd.h
+++ b/fs/nfsd/nfsd.h
@@ -359,6 +359,7 @@ void		nfsd_lockd_shutdown(void);
 
 #define NFSD4_2_SUPPORTED_ATTRS_WORD2 \
 	(NFSD4_1_SUPPORTED_ATTRS_WORD2 | \
+	FATTR4_WORD2_UMASK | \
 	NFSD4_2_SECURITY_ATTRS)
 
 extern u32 nfsd_suppattrs[3][3];
@@ -390,10 +391,14 @@ static inline bool nfsd_attrs_supported(u32 minorversion, u32 *bmval)
 	(FATTR4_WORD1_MODE | FATTR4_WORD1_OWNER | FATTR4_WORD1_OWNER_GROUP \
 	| FATTR4_WORD1_TIME_ACCESS_SET | FATTR4_WORD1_TIME_MODIFY_SET)
 #ifdef CONFIG_NFSD_V4_SECURITY_LABEL
-#define NFSD_WRITEABLE_ATTRS_WORD2 FATTR4_WORD2_SECURITY_LABEL
+#define MAYBE_FATTR4_WORD2_SECURITY_LABEL \
+	FATTR4_WORD2_SECURITY_LABEL
 #else
-#define NFSD_WRITEABLE_ATTRS_WORD2 0
+#define MAYBE_FATTR4_WORD2_SECURITY_LABEL 0
 #endif
+#define NFSD_WRITEABLE_ATTRS_WORD2 \
+	(FATTR4_WORD2_UMASK \
+	| MAYBE_FATTR4_WORD2_SECURITY_LABEL)
 
 #define NFSD_SUPPATTR_EXCLCREAT_WORD0 \
 	NFSD_WRITEABLE_ATTRS_WORD0
diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
index a2b65fc56dd6..e6bfd96734c0 100644
--- a/fs/nfsd/nfssvc.c
+++ b/fs/nfsd/nfssvc.c
@@ -661,8 +661,8 @@ nfsd(void *vrqstp)
 	mutex_lock(&nfsd_mutex);
 
 	/* At this point, the thread shares current->fs
-	 * with the init process. We need to create files with a
-	 * umask of 0 instead of init's umask. */
+	 * with the init process. We need to create files with the
+	 * umask as defined by the client instead of init's umask. */
 	if (unshare_fs_struct() < 0) {
 		printk("Unable to start nfsd thread: out of memory\n");
 		goto out;
-- 
2.7.4

Re: [PATCH 0/2] NFSv4.2 umask support

[J. Bruce Fields]

By the way, the code is as noted Andreas's work--I just did a minor
update to reflect a protocol change suggested by Trond and already
incorporated in the ietf drafts.

We also have a wireshark patch, below.

--b.

diff --git a/epan/dissectors/packet-nfs.c b/epan/dissectors/packet-nfs.c
index 010db6e96761..bf458d4bccf4 100644
--- a/epan/dissectors/packet-nfs.c
+++ b/epan/dissectors/packet-nfs.c
@@ -420,6 +420,7 @@ static int hf_nfs4_fattr_layout_blksize = -1;
 static int hf_nfs4_fattr_security_label_lfs = -1;
 static int hf_nfs4_fattr_security_label_pi = -1;
 static int hf_nfs4_fattr_security_label_context = -1;
+static int hf_nfs4_fattr_umask_mask = -1;
 static int hf_nfs4_who = -1;
 static int hf_nfs4_server = -1;
 static int hf_nfs4_fslocation = -1;
@@ -6134,6 +6135,8 @@ static const value_string fattr4_names[] = {
 	{	FATTR4_CHANGE_ATTR_TYPE,   "Change_Attr_Type"		},
 #define FATTR4_SECURITY_LABEL      80
 	{	FATTR4_SECURITY_LABEL,     "Security_Label"		},
+#define FATTR4_UMASK               81
+	{	FATTR4_UMASK,              "Umask"			},
 	{	0,	NULL	}
 };
 static value_string_ext fattr4_names_ext = VALUE_STRING_EXT_INIT(fattr4_names);
@@ -6718,6 +6721,14 @@ dissect_nfs4_security_label(tvbuff_t *tvb, proto_tree *tree, int offset)
 	return offset;
 }
 
+static int
+dissect_nfs4_umask(tvbuff_t *tvb, proto_tree *tree, int offset)
+{
+	offset = dissect_nfs4_mode(tvb, offset, tree);
+	offset = dissect_rpc_uint32(tvb, tree, hf_nfs4_fattr_umask_mask, offset);
+	return offset;
+}
+
 #define FATTR4_BITMAP_ONLY 0
 #define FATTR4_DISSECT_VALUES 1
 
@@ -7120,6 +7131,10 @@ dissect_nfs4_fattrs(tvbuff_t *tvb, int offset, packet_info *pinfo, proto_tree *t
 						offset = dissect_nfs4_security_label(tvb, attr_tree, offset);
 						break;
 
+					case FATTR4_UMASK:
+						offset = dissect_nfs4_umask(tvb, attr_tree, offset);
+						break;
+
 					default:
 						break;
 					}
@@ -12510,6 +12525,10 @@ proto_register_nfs(void)
 			"label_format", "nfs.fattr4.security_label.lfs", FT_UINT32, BASE_DEC,
 			NULL, 0, NULL, HFILL }},
 
+		{ &hf_nfs4_fattr_umask, {
+			"umask", "nfs.fattr4.umask", FT_UINT32, BASE_OCT,
+			NULL, 0, NULL, HFILL }},
+
 		{ &hf_nfs4_fattr_security_label_pi, {
 			"policy_id", "nfs.fattr4.security_label.pi", FT_UINT32, BASE_DEC,
 			NULL, 0, NULL, HFILL }},

Re: [PATCH 0/2] NFSv4.2 umask support

[Andreas Gruenbacher]

On Fri, Oct 28, 2016 at 10:23 PM, J. Bruce Fields <bfields@redhat.com> wrote:
> From: "J. Bruce Fields" <bfields@redhat.com>
>
> The following patches allow the umask to be ignored in the presence of
> inheritable NFSv4 ACLs.  Otherwise inheritable ACLs can be rendered
> mostly useless whenever the umask masks out group bits.
>
> This solves a problem we've seen complaints about for some time, both
> upstream and from RHEL users.
>
> The new protocol has been discussed in the IETF working group and is
> documented at:
>
>         https://tools.ietf.org/html/draft-ietf-nfsv4-umask-02
>
> It's unlikely that we'll discover problems requiring an incompatible
> change, so I think we should consider this for 4.10.

Nope, these patches don't implement draft-ietf-nfsv4-umask-02 yet, and
need updating first.

Thanks,
Andreas

Re: [PATCH 0/2] NFSv4.2 umask support

[J. Bruce Fields]

On Sat, Oct 29, 2016 at 11:38:43AM +0200, Andreas Gruenbacher wrote:
> On Fri, Oct 28, 2016 at 10:23 PM, J. Bruce Fields <bfields@redhat.com> wrote:
> > From: "J. Bruce Fields" <bfields@redhat.com>
> >
> > The following patches allow the umask to be ignored in the presence of
> > inheritable NFSv4 ACLs.  Otherwise inheritable ACLs can be rendered
> > mostly useless whenever the umask masks out group bits.
> >
> > This solves a problem we've seen complaints about for some time, both
> > upstream and from RHEL users.
> >
> > The new protocol has been discussed in the IETF working group and is
> > documented at:
> >
> >         https://tools.ietf.org/html/draft-ietf-nfsv4-umask-02
> >
> > It's unlikely that we'll discover problems requiring an incompatible
> > change, so I think we should consider this for 4.10.
> 
> Nope, these patches don't implement draft-ietf-nfsv4-umask-02 yet,

They don't?  What did I miss?

--b.

Re: [PATCH 0/2] NFSv4.2 umask support

[Andreas Gruenbacher]

On Sat, Oct 29, 2016 at 11:38 AM, Andreas Gruenbacher
<agruenba@redhat.com> wrote:
> On Fri, Oct 28, 2016 at 10:23 PM, J. Bruce Fields <bfields@redhat.com> wrote:
>> From: "J. Bruce Fields" <bfields@redhat.com>
>>
>> The following patches allow the umask to be ignored in the presence of
>> inheritable NFSv4 ACLs.  Otherwise inheritable ACLs can be rendered
>> mostly useless whenever the umask masks out group bits.
>>
>> This solves a problem we've seen complaints about for some time, both
>> upstream and from RHEL users.
>>
>> The new protocol has been discussed in the IETF working group and is
>> documented at:
>>
>>         https://tools.ietf.org/html/draft-ietf-nfsv4-umask-02
>>
>> It's unlikely that we'll discover problems requiring an incompatible
>> change, so I think we should consider this for 4.10.
>
> Nope, these patches don't implement draft-ietf-nfsv4-umask-02 yet, and
> need updating first.

Oops, turns out I was confused. The patches do implement the proposed
new mode_umask attribute right but call it FATTR4_WORD2_UMASK in the
code, which doesn't seem right. Let me follow up with a version that
renames the attribute to FATTR4_WORD2_MODE_UMASK. The rest looks fine.

Thanks,
Andreas

[PATCH 0/2] NFSv4.2 mode_umask support

[Andreas Gruenbacher]

Bruce and all,

On Fri, Oct 28, 2016 at 10:23 PM, J. Bruce Fields <bfields@redhat.com> wrote:
> From: "J. Bruce Fields" <bfields@redhat.com>
>
> The following patches allow the umask to be ignored in the presence of
> inheritable NFSv4 ACLs.  Otherwise inheritable ACLs can be rendered
> mostly useless whenever the umask masks out group bits.
>
> This solves a problem we've seen complaints about for some time, both
> upstream and from RHEL users.
>
> The new protocol has been discussed in the IETF working group and is
> documented at:
>
>         https://tools.ietf.org/html/draft-ietf-nfsv4-umask-02
>
> It's unlikely that we'll discover problems requiring an incompatible
> change, so I think we should consider this for 4.10.

the patches still refer to the new attribute as FATTR4_WORD2_UMASK which is
confusing.  Can we please call it FATTR4_WORD2_MODE_UMASK as in the patches in
this series to better match what the attribute is called in
draft-ietf-nfsv4-umask-02.

Other than refreshing the patches and renaming FATTR4_WORD2_UMASK to
FATTR4_WORD2_MODE_UMASK and NFS_CAP_UMASK to NFS_CAP_MODE_UMASK, the patches
here are the same.

Thanks,
Andreas

Andreas Gruenbacher (2):
  nfs: add support for the umask attribute
  nfsd: add support for the umask attribute

 fs/nfs/dir.c              |  7 ++++++-
 fs/nfs/nfs4proc.c         | 21 ++++++++++++++++-----
 fs/nfs/nfs4xdr.c          | 36 ++++++++++++++++++++++++------------
 fs/nfsd/nfs4xdr.c         | 26 +++++++++++++++++++++-----
 fs/nfsd/nfsd.h            |  9 +++++++--
 fs/nfsd/nfssvc.c          |  4 ++--
 include/linux/nfs4.h      |  1 +
 include/linux/nfs_fs_sb.h |  1 +
 include/linux/nfs_xdr.h   |  2 ++
 9 files changed, 80 insertions(+), 27 deletions(-)

-- 
2.7.4

[PATCH 1/2] nfs: add support for the umask attribute

[Andreas Gruenbacher]

Clients can set the umask attribute when creating files to cause the
server to apply it always except when inheriting permissions from the
parent directory.  That way, the new files will end up with the same
permissions as files created locally.

See https://tools.ietf.org/html/draft-ietf-nfsv4-umask-02 for more details.

Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/nfs/dir.c              |  7 ++++++-
 fs/nfs/nfs4proc.c         | 21 ++++++++++++++++-----
 fs/nfs/nfs4xdr.c          | 36 ++++++++++++++++++++++++------------
 include/linux/nfs4.h      |  1 +
 include/linux/nfs_fs_sb.h |  1 +
 include/linux/nfs_xdr.h   |  2 ++
 6 files changed, 50 insertions(+), 18 deletions(-)

diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index 5f1af4c..0a8326b 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -1535,8 +1535,13 @@ int nfs_atomic_open(struct inode *dir, struct dentry *dentry,
 		return -ENAMETOOLONG;
 
 	if (open_flags & O_CREAT) {
+		struct nfs_server *server = NFS_SERVER(dir);
+
+		if (!(server->caps & NFS_CAP_MODE_UMASK))
+			mode &= ~current_umask();
+
 		attr.ia_valid |= ATTR_MODE;
-		attr.ia_mode = mode & ~current_umask();
+		attr.ia_mode = mode;
 	}
 	if (open_flags & O_TRUNC) {
 		attr.ia_valid |= ATTR_SIZE;
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 7897826..1fa15fb 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -1239,6 +1239,7 @@ static struct nfs4_opendata *nfs4_opendata_alloc(struct dentry *dentry,
 	p->o_arg.bitmask = nfs4_bitmask(server, label);
 	p->o_arg.open_bitmap = &nfs4_fattr_bitmap[0];
 	p->o_arg.label = nfs4_label_copy(p->a_label, label);
+	p->o_arg.umask = current_umask();
 	p->o_arg.claim = nfs4_map_atomic_open_claim(server, claim);
 	switch (p->o_arg.claim) {
 	case NFS4_OPEN_CLAIM_NULL:
@@ -3277,7 +3278,7 @@ static void nfs4_close_context(struct nfs_open_context *ctx, int is_sync)
 
 #define FATTR4_WORD1_NFS40_MASK (2*FATTR4_WORD1_MOUNTED_ON_FILEID - 1UL)
 #define FATTR4_WORD2_NFS41_MASK (2*FATTR4_WORD2_SUPPATTR_EXCLCREAT - 1UL)
-#define FATTR4_WORD2_NFS42_MASK (2*FATTR4_WORD2_SECURITY_LABEL - 1UL)
+#define FATTR4_WORD2_NFS42_MASK (2*FATTR4_WORD2_MODE_UMASK - 1UL)
 
 static int _nfs4_server_capabilities(struct nfs_server *server, struct nfs_fh *fhandle)
 {
@@ -3322,7 +3323,8 @@ static int _nfs4_server_capabilities(struct nfs_server *server, struct nfs_fh *f
 				NFS_CAP_MODE|NFS_CAP_NLINK|NFS_CAP_OWNER|
 				NFS_CAP_OWNER_GROUP|NFS_CAP_ATIME|
 				NFS_CAP_CTIME|NFS_CAP_MTIME|
-				NFS_CAP_SECURITY_LABEL);
+				NFS_CAP_SECURITY_LABEL|
+				NFS_CAP_MODE_UMASK);
 		if (res.attr_bitmask[0] & FATTR4_WORD0_ACL &&
 				res.acl_bitmask & ACL4_SUPPORT_ALLOW_ACL)
 			server->caps |= NFS_CAP_ACLS;
@@ -3350,6 +3352,8 @@ static int _nfs4_server_capabilities(struct nfs_server *server, struct nfs_fh *f
 		if (res.attr_bitmask[2] & FATTR4_WORD2_SECURITY_LABEL)
 			server->caps |= NFS_CAP_SECURITY_LABEL;
 #endif
+		if (res.attr_bitmask[2] & FATTR4_WORD2_MODE_UMASK)
+			server->caps |= NFS_CAP_MODE_UMASK;
 		memcpy(server->attr_bitmask_nl, res.attr_bitmask,
 				sizeof(server->attr_bitmask));
 		server->attr_bitmask_nl[2] &= ~FATTR4_WORD2_SECURITY_LABEL;
@@ -3953,6 +3957,7 @@ static int
 nfs4_proc_create(struct inode *dir, struct dentry *dentry, struct iattr *sattr,
 		 int flags)
 {
+	struct nfs_server *server = NFS_SERVER(dir);
 	struct nfs4_label l, *ilabel = NULL;
 	struct nfs_open_context *ctx;
 	struct nfs4_state *state;
@@ -3964,7 +3969,8 @@ nfs4_proc_create(struct inode *dir, struct dentry *dentry, struct iattr *sattr,
 
 	ilabel = nfs4_label_init_security(dir, dentry, sattr, &l);
 
-	sattr->ia_mode &= ~current_umask();
+	if (!(server->caps & NFS_CAP_MODE_UMASK))
+		sattr->ia_mode &= ~current_umask();
 	state = nfs4_do_open(dir, ctx, flags, sattr, ilabel, NULL);
 	if (IS_ERR(state)) {
 		status = PTR_ERR(state);
@@ -4172,6 +4178,7 @@ static struct nfs4_createdata *nfs4_alloc_createdata(struct inode *dir,
 		data->arg.attrs = sattr;
 		data->arg.ftype = ftype;
 		data->arg.bitmask = nfs4_bitmask(server, data->label);
+		data->arg.umask = current_umask();
 		data->res.server = server;
 		data->res.fh = &data->fh;
 		data->res.fattr = &data->fattr;
@@ -4269,13 +4276,15 @@ static int _nfs4_proc_mkdir(struct inode *dir, struct dentry *dentry,
 static int nfs4_proc_mkdir(struct inode *dir, struct dentry *dentry,
 		struct iattr *sattr)
 {
+	struct nfs_server *server = NFS_SERVER(dir);
 	struct nfs4_exception exception = { };
 	struct nfs4_label l, *label = NULL;
 	int err;
 
 	label = nfs4_label_init_security(dir, dentry, sattr, &l);
 
-	sattr->ia_mode &= ~current_umask();
+	if (!(server->caps & NFS_CAP_MODE_UMASK))
+		sattr->ia_mode &= ~current_umask();
 	do {
 		err = _nfs4_proc_mkdir(dir, dentry, sattr, label);
 		trace_nfs4_mkdir(dir, &dentry->d_name, err);
@@ -4378,13 +4387,15 @@ static int _nfs4_proc_mknod(struct inode *dir, struct dentry *dentry,
 static int nfs4_proc_mknod(struct inode *dir, struct dentry *dentry,
 		struct iattr *sattr, dev_t rdev)
 {
+	struct nfs_server *server = NFS_SERVER(dir);
 	struct nfs4_exception exception = { };
 	struct nfs4_label l, *label = NULL;
 	int err;
 
 	label = nfs4_label_init_security(dir, dentry, sattr, &l);
 
-	sattr->ia_mode &= ~current_umask();
+	if (!(server->caps & NFS_CAP_MODE_UMASK))
+		sattr->ia_mode &= ~current_umask();
 	do {
 		err = _nfs4_proc_mknod(dir, dentry, sattr, label, rdev);
 		trace_nfs4_mknod(dir, &dentry->d_name, err);
diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
index fc89e5e..420d278 100644
--- a/fs/nfs/nfs4xdr.c
+++ b/fs/nfs/nfs4xdr.c
@@ -52,6 +52,7 @@
 #include <linux/nfs.h>
 #include <linux/nfs4.h>
 #include <linux/nfs_fs.h>
+#include <linux/fs_struct.h>
 
 #include "nfs4_fs.h"
 #include "internal.h"
@@ -1003,7 +1004,7 @@ static void encode_nfs4_verifier(struct xdr_stream *xdr, const nfs4_verifier *ve
 static void encode_attrs(struct xdr_stream *xdr, const struct iattr *iap,
 				const struct nfs4_label *label,
 				const struct nfs_server *server,
-				bool excl_check)
+				bool excl_check, const umode_t *umask)
 {
 	char owner_name[IDMAP_NAMESZ];
 	char owner_group[IDMAP_NAMESZ];
@@ -1017,18 +1018,21 @@ static void encode_attrs(struct xdr_stream *xdr, const struct iattr *iap,
 
 	/*
 	 * We reserve enough space to write the entire attribute buffer at once.
-	 * In the worst-case, this would be
-	 * 16(bitmap) + 4(attrlen) + 8(size) + 4(mode) + 4(atime) + 4(mtime)
-	 * = 40 bytes, plus any contribution from variable-length fields
-	 *            such as owner/group.
 	 */
 	if (iap->ia_valid & ATTR_SIZE) {
 		bmval[0] |= FATTR4_WORD0_SIZE;
 		len += 8;
 	}
+	if (!(server->caps & NFS_CAP_MODE_UMASK))
+		umask = NULL;
 	if (iap->ia_valid & ATTR_MODE) {
-		bmval[1] |= FATTR4_WORD1_MODE;
-		len += 4;
+		if (umask) {
+			bmval[2] |= FATTR4_WORD2_MODE_UMASK;
+			len += 8;
+		} else {
+			bmval[1] |= FATTR4_WORD1_MODE;
+			len += 4;
+		}
 	}
 	if (iap->ia_valid & ATTR_UID) {
 		owner_namelen = nfs_map_uid_to_name(server, iap->ia_uid, owner_name, IDMAP_NAMESZ);
@@ -1129,6 +1133,10 @@ static void encode_attrs(struct xdr_stream *xdr, const struct iattr *iap,
 		*p++ = cpu_to_be32(label->len);
 		p = xdr_encode_opaque_fixed(p, label->label, label->len);
 	}
+	if (bmval[2] & FATTR4_WORD2_MODE_UMASK) {
+		*p++ = cpu_to_be32(iap->ia_mode & S_IALLUGO);
+		*p++ = cpu_to_be32(*umask);
+	}
 
 /* out: */
 }
@@ -1183,7 +1191,8 @@ static void encode_create(struct xdr_stream *xdr, const struct nfs4_create_arg *
 	}
 
 	encode_string(xdr, create->name->len, create->name->name);
-	encode_attrs(xdr, create->attrs, create->label, create->server, false);
+	encode_attrs(xdr, create->attrs, create->label, create->server, false,
+		     &create->umask);
 }
 
 static void encode_getattr_one(struct xdr_stream *xdr, uint32_t bitmap, struct compound_hdr *hdr)
@@ -1403,11 +1412,13 @@ static inline void encode_createmode(struct xdr_stream *xdr, const struct nfs_op
 	switch(arg->createmode) {
 	case NFS4_CREATE_UNCHECKED:
 		*p = cpu_to_be32(NFS4_CREATE_UNCHECKED);
-		encode_attrs(xdr, arg->u.attrs, arg->label, arg->server, false);
+		encode_attrs(xdr, arg->u.attrs, arg->label, arg->server, false,
+			     &arg->umask);
 		break;
 	case NFS4_CREATE_GUARDED:
 		*p = cpu_to_be32(NFS4_CREATE_GUARDED);
-		encode_attrs(xdr, arg->u.attrs, arg->label, arg->server, false);
+		encode_attrs(xdr, arg->u.attrs, arg->label, arg->server, false,
+			     &arg->umask);
 		break;
 	case NFS4_CREATE_EXCLUSIVE:
 		*p = cpu_to_be32(NFS4_CREATE_EXCLUSIVE);
@@ -1416,7 +1427,8 @@ static inline void encode_createmode(struct xdr_stream *xdr, const struct nfs_op
 	case NFS4_CREATE_EXCLUSIVE4_1:
 		*p = cpu_to_be32(NFS4_CREATE_EXCLUSIVE4_1);
 		encode_nfs4_verifier(xdr, &arg->u.verifier);
-		encode_attrs(xdr, arg->u.attrs, arg->label, arg->server, true);
+		encode_attrs(xdr, arg->u.attrs, arg->label, arg->server, true,
+			     &arg->umask);
 	}
 }
 
@@ -1672,7 +1684,7 @@ static void encode_setattr(struct xdr_stream *xdr, const struct nfs_setattrargs
 {
 	encode_op_hdr(xdr, OP_SETATTR, decode_setattr_maxsz, hdr);
 	encode_nfs4_stateid(xdr, &arg->stateid);
-	encode_attrs(xdr, arg->iap, arg->label, server, false);
+	encode_attrs(xdr, arg->iap, arg->label, server, false, NULL);
 }
 
 static void encode_setclientid(struct xdr_stream *xdr, const struct nfs4_setclientid *setclientid, struct compound_hdr *hdr)
diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h
index 9094faf..bca5363 100644
--- a/include/linux/nfs4.h
+++ b/include/linux/nfs4.h
@@ -440,6 +440,7 @@ enum lock_type4 {
 #define FATTR4_WORD2_MDSTHRESHOLD       (1UL << 4)
 #define FATTR4_WORD2_CLONE_BLKSIZE	(1UL << 13)
 #define FATTR4_WORD2_SECURITY_LABEL     (1UL << 16)
+#define FATTR4_WORD2_MODE_UMASK		(1UL << 17)
 
 /* MDS threshold bitmap bits */
 #define THRESHOLD_RD                    (1UL << 0)
diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
index b34097c..87ab710 100644
--- a/include/linux/nfs_fs_sb.h
+++ b/include/linux/nfs_fs_sb.h
@@ -250,5 +250,6 @@ struct nfs_server {
 #define NFS_CAP_LAYOUTSTATS	(1U << 22)
 #define NFS_CAP_CLONE		(1U << 23)
 #define NFS_CAP_COPY		(1U << 24)
+#define NFS_CAP_MODE_UMASK	(1U << 25)
 
 #endif
diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
index beb1e10..ff82f42 100644
--- a/include/linux/nfs_xdr.h
+++ b/include/linux/nfs_xdr.h
@@ -418,6 +418,7 @@ struct nfs_openargs {
 	enum open_claim_type4	claim;
 	enum createmode4	createmode;
 	const struct nfs4_label *label;
+	umode_t			umask;
 };
 
 struct nfs_openres {
@@ -937,6 +938,7 @@ struct nfs4_create_arg {
 	const struct nfs_fh *		dir_fh;
 	const u32 *			bitmask;
 	const struct nfs4_label		*label;
+	umode_t				umask;
 };
 
 struct nfs4_create_res {
-- 
2.7.4

[PATCH 2/2] nfsd: add support for the umask attribute

[Andreas Gruenbacher]

Clients can set the umask attribute when creating files to cause the
server to apply it always except when inheriting permissions from the
parent directory.  That way, the new files will end up with the same
permissions as files created locally.

See https://tools.ietf.org/html/draft-ietf-nfsv4-umask-02 for more
details.

Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/nfsd/nfs4xdr.c | 26 +++++++++++++++++++++-----
 fs/nfsd/nfsd.h    |  9 +++++++--
 fs/nfsd/nfssvc.c  |  4 ++--
 3 files changed, 30 insertions(+), 9 deletions(-)

diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index c2d2895..26779cf 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -33,6 +33,7 @@
  *  SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
+#include <linux/fs_struct.h>
 #include <linux/file.h>
 #include <linux/slab.h>
 #include <linux/namei.h>
@@ -285,7 +286,7 @@ nfsd4_decode_bitmap(struct nfsd4_compoundargs *argp, u32 *bmval)
 static __be32
 nfsd4_decode_fattr(struct nfsd4_compoundargs *argp, u32 *bmval,
 		   struct iattr *iattr, struct nfs4_acl **acl,
-		   struct xdr_netobj *label)
+		   struct xdr_netobj *label, int *umask)
 {
 	int expected_len, len = 0;
 	u32 dummy32;
@@ -435,6 +436,17 @@ nfsd4_decode_fattr(struct nfsd4_compoundargs *argp, u32 *bmval,
 			return nfserr_jukebox;
 	}
 #endif
+	if (bmval[2] & FATTR4_WORD2_MODE_UMASK) {
+		if (!umask)
+			goto xdr_error;
+		READ_BUF(8);
+		len += 8;
+		dummy32 = be32_to_cpup(p++);
+		iattr->ia_mode = dummy32 & (S_IFMT | S_IALLUGO);
+		dummy32 = be32_to_cpup(p++);
+		*umask = dummy32 & S_IRWXUGO;
+		iattr->ia_valid |= ATTR_MODE;
+	}
 
 	if (bmval[0] & ~NFSD_WRITEABLE_ATTRS_WORD0
 	    || bmval[1] & ~NFSD_WRITEABLE_ATTRS_WORD1
@@ -634,7 +646,8 @@ nfsd4_decode_create(struct nfsd4_compoundargs *argp, struct nfsd4_create *create
 		return status;
 
 	status = nfsd4_decode_fattr(argp, create->cr_bmval, &create->cr_iattr,
-				    &create->cr_acl, &create->cr_label);
+				    &create->cr_acl, &create->cr_label,
+				    &current->fs->umask);
 	if (status)
 		goto out;
 
@@ -879,13 +892,15 @@ nfsd4_decode_open(struct nfsd4_compoundargs *argp, struct nfsd4_open *open)
 	case NFS4_OPEN_NOCREATE:
 		break;
 	case NFS4_OPEN_CREATE:
+		current->fs->umask = 0;
 		READ_BUF(4);
 		open->op_createmode = be32_to_cpup(p++);
 		switch (open->op_createmode) {
 		case NFS4_CREATE_UNCHECKED:
 		case NFS4_CREATE_GUARDED:
 			status = nfsd4_decode_fattr(argp, open->op_bmval,
-				&open->op_iattr, &open->op_acl, &open->op_label);
+				&open->op_iattr, &open->op_acl, &open->op_label,
+				&current->fs->umask);
 			if (status)
 				goto out;
 			break;
@@ -899,7 +914,8 @@ nfsd4_decode_open(struct nfsd4_compoundargs *argp, struct nfsd4_open *open)
 			READ_BUF(NFS4_VERIFIER_SIZE);
 			COPYMEM(open->op_verf.data, NFS4_VERIFIER_SIZE);
 			status = nfsd4_decode_fattr(argp, open->op_bmval,
-				&open->op_iattr, &open->op_acl, &open->op_label);
+				&open->op_iattr, &open->op_acl, &open->op_label,
+				&current->fs->umask);
 			if (status)
 				goto out;
 			break;
@@ -1136,7 +1152,7 @@ nfsd4_decode_setattr(struct nfsd4_compoundargs *argp, struct nfsd4_setattr *seta
 	if (status)
 		return status;
 	return nfsd4_decode_fattr(argp, setattr->sa_bmval, &setattr->sa_iattr,
-				  &setattr->sa_acl, &setattr->sa_label);
+				  &setattr->sa_acl, &setattr->sa_label, NULL);
 }
 
 static __be32
diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h
index 9446849..f29b8d4 100644
--- a/fs/nfsd/nfsd.h
+++ b/fs/nfsd/nfsd.h
@@ -359,6 +359,7 @@ void		nfsd_lockd_shutdown(void);
 
 #define NFSD4_2_SUPPORTED_ATTRS_WORD2 \
 	(NFSD4_1_SUPPORTED_ATTRS_WORD2 | \
+	FATTR4_WORD2_MODE_UMASK | \
 	NFSD4_2_SECURITY_ATTRS)
 
 static inline u32 nfsd_suppattrs0(u32 minorversion)
@@ -393,10 +394,14 @@ static inline u32 nfsd_suppattrs2(u32 minorversion)
 	(FATTR4_WORD1_MODE | FATTR4_WORD1_OWNER | FATTR4_WORD1_OWNER_GROUP \
 	| FATTR4_WORD1_TIME_ACCESS_SET | FATTR4_WORD1_TIME_MODIFY_SET)
 #ifdef CONFIG_NFSD_V4_SECURITY_LABEL
-#define NFSD_WRITEABLE_ATTRS_WORD2 FATTR4_WORD2_SECURITY_LABEL
+#define MAYBE_FATTR4_WORD2_SECURITY_LABEL \
+	FATTR4_WORD2_SECURITY_LABEL
 #else
-#define NFSD_WRITEABLE_ATTRS_WORD2 0
+#define MAYBE_FATTR4_WORD2_SECURITY_LABEL 0
 #endif
+#define NFSD_WRITEABLE_ATTRS_WORD2 \
+	(FATTR4_WORD2_MODE_UMASK \
+	| MAYBE_FATTR4_WORD2_SECURITY_LABEL)
 
 #define NFSD_SUPPATTR_EXCLCREAT_WORD0 \
 	NFSD_WRITEABLE_ATTRS_WORD0
diff --git a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c
index a2b65fc..e6bfd96 100644
--- a/fs/nfsd/nfssvc.c
+++ b/fs/nfsd/nfssvc.c
@@ -661,8 +661,8 @@ nfsd(void *vrqstp)
 	mutex_lock(&nfsd_mutex);
 
 	/* At this point, the thread shares current->fs
-	 * with the init process. We need to create files with a
-	 * umask of 0 instead of init's umask. */
+	 * with the init process. We need to create files with the
+	 * umask as defined by the client instead of init's umask. */
 	if (unshare_fs_struct() < 0) {
 		printk("Unable to start nfsd thread: out of memory\n");
 		goto out;
-- 
2.7.4

Re: [PATCH 0/2] NFSv4.2 mode_umask support

[J. Bruce Fields]

On Sun, Oct 30, 2016 at 12:20:50AM +0200, Andreas Gruenbacher wrote:
> Bruce and all,
> 
> On Fri, Oct 28, 2016 at 10:23 PM, J. Bruce Fields <bfields@redhat.com> wrote:
> > From: "J. Bruce Fields" <bfields@redhat.com>
> >
> > The following patches allow the umask to be ignored in the presence of
> > inheritable NFSv4 ACLs.  Otherwise inheritable ACLs can be rendered
> > mostly useless whenever the umask masks out group bits.
> >
> > This solves a problem we've seen complaints about for some time, both
> > upstream and from RHEL users.
> >
> > The new protocol has been discussed in the IETF working group and is
> > documented at:
> >
> >         https://tools.ietf.org/html/draft-ietf-nfsv4-umask-02
> >
> > It's unlikely that we'll discover problems requiring an incompatible
> > change, so I think we should consider this for 4.10.
> 
> the patches still refer to the new attribute as FATTR4_WORD2_UMASK which is
> confusing.  Can we please call it FATTR4_WORD2_MODE_UMASK as in the patches in
> this series to better match what the attribute is called in
> draft-ietf-nfsv4-umask-02.

Thanks!--b.

> 
> Other than refreshing the patches and renaming FATTR4_WORD2_UMASK to
> FATTR4_WORD2_MODE_UMASK and NFS_CAP_UMASK to NFS_CAP_MODE_UMASK, the patches
> here are the same.
> 
> Thanks,
> Andreas
> 
> Andreas Gruenbacher (2):
>   nfs: add support for the umask attribute
>   nfsd: add support for the umask attribute
> 
>  fs/nfs/dir.c              |  7 ++++++-
>  fs/nfs/nfs4proc.c         | 21 ++++++++++++++++-----
>  fs/nfs/nfs4xdr.c          | 36 ++++++++++++++++++++++++------------
>  fs/nfsd/nfs4xdr.c         | 26 +++++++++++++++++++++-----
>  fs/nfsd/nfsd.h            |  9 +++++++--
>  fs/nfsd/nfssvc.c          |  4 ++--
>  include/linux/nfs4.h      |  1 +
>  include/linux/nfs_fs_sb.h |  1 +
>  include/linux/nfs_xdr.h   |  2 ++
>  9 files changed, 80 insertions(+), 27 deletions(-)
> 
> -- 
> 2.7.4
> 

[PATCH 1/2] nfs: add support for the umask attribute

[J. Bruce Fields]

From: Andreas Gruenbacher <agruenba@redhat.com>

Clients can set the umask attribute when creating files to cause the
server to apply it always except when inheriting permissions from the
parent directory.  That way, the new files will end up with the same
permissions as files created locally.

See https://tools.ietf.org/html/draft-ietf-nfsv4-umask-02 for more details.

Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/nfs/dir.c              |  7 ++++++-
 fs/nfs/nfs4proc.c         | 21 ++++++++++++++++-----
 fs/nfs/nfs4xdr.c          | 36 ++++++++++++++++++++++++------------
 include/linux/nfs4.h      |  1 +
 include/linux/nfs_fs_sb.h |  1 +
 include/linux/nfs_xdr.h   |  2 ++
 6 files changed, 50 insertions(+), 18 deletions(-)

diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index 5f1af4cd1a33..0a8326bcb481 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -1535,8 +1535,13 @@ int nfs_atomic_open(struct inode *dir, struct dentry *dentry,
 		return -ENAMETOOLONG;
 
 	if (open_flags & O_CREAT) {
+		struct nfs_server *server = NFS_SERVER(dir);
+
+		if (!(server->caps & NFS_CAP_MODE_UMASK))
+			mode &= ~current_umask();
+
 		attr.ia_valid |= ATTR_MODE;
-		attr.ia_mode = mode & ~current_umask();
+		attr.ia_mode = mode;
 	}
 	if (open_flags & O_TRUNC) {
 		attr.ia_valid |= ATTR_SIZE;
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 7897826d7c51..1fa15fbf7b48 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -1239,6 +1239,7 @@ static struct nfs4_opendata *nfs4_opendata_alloc(struct dentry *dentry,
 	p->o_arg.bitmask = nfs4_bitmask(server, label);
 	p->o_arg.open_bitmap = &nfs4_fattr_bitmap[0];
 	p->o_arg.label = nfs4_label_copy(p->a_label, label);
+	p->o_arg.umask = current_umask();
 	p->o_arg.claim = nfs4_map_atomic_open_claim(server, claim);
 	switch (p->o_arg.claim) {
 	case NFS4_OPEN_CLAIM_NULL:
@@ -3277,7 +3278,7 @@ static void nfs4_close_context(struct nfs_open_context *ctx, int is_sync)
 
 #define FATTR4_WORD1_NFS40_MASK (2*FATTR4_WORD1_MOUNTED_ON_FILEID - 1UL)
 #define FATTR4_WORD2_NFS41_MASK (2*FATTR4_WORD2_SUPPATTR_EXCLCREAT - 1UL)
-#define FATTR4_WORD2_NFS42_MASK (2*FATTR4_WORD2_SECURITY_LABEL - 1UL)
+#define FATTR4_WORD2_NFS42_MASK (2*FATTR4_WORD2_MODE_UMASK - 1UL)
 
 static int _nfs4_server_capabilities(struct nfs_server *server, struct nfs_fh *fhandle)
 {
@@ -3322,7 +3323,8 @@ static int _nfs4_server_capabilities(struct nfs_server *server, struct nfs_fh *f
 				NFS_CAP_MODE|NFS_CAP_NLINK|NFS_CAP_OWNER|
 				NFS_CAP_OWNER_GROUP|NFS_CAP_ATIME|
 				NFS_CAP_CTIME|NFS_CAP_MTIME|
-				NFS_CAP_SECURITY_LABEL);
+				NFS_CAP_SECURITY_LABEL|
+				NFS_CAP_MODE_UMASK);
 		if (res.attr_bitmask[0] & FATTR4_WORD0_ACL &&
 				res.acl_bitmask & ACL4_SUPPORT_ALLOW_ACL)
 			server->caps |= NFS_CAP_ACLS;
@@ -3350,6 +3352,8 @@ static int _nfs4_server_capabilities(struct nfs_server *server, struct nfs_fh *f
 		if (res.attr_bitmask[2] & FATTR4_WORD2_SECURITY_LABEL)
 			server->caps |= NFS_CAP_SECURITY_LABEL;
 #endif
+		if (res.attr_bitmask[2] & FATTR4_WORD2_MODE_UMASK)
+			server->caps |= NFS_CAP_MODE_UMASK;
 		memcpy(server->attr_bitmask_nl, res.attr_bitmask,
 				sizeof(server->attr_bitmask));
 		server->attr_bitmask_nl[2] &= ~FATTR4_WORD2_SECURITY_LABEL;
@@ -3953,6 +3957,7 @@ static int
 nfs4_proc_create(struct inode *dir, struct dentry *dentry, struct iattr *sattr,
 		 int flags)
 {
+	struct nfs_server *server = NFS_SERVER(dir);
 	struct nfs4_label l, *ilabel = NULL;
 	struct nfs_open_context *ctx;
 	struct nfs4_state *state;
@@ -3964,7 +3969,8 @@ nfs4_proc_create(struct inode *dir, struct dentry *dentry, struct iattr *sattr,
 
 	ilabel = nfs4_label_init_security(dir, dentry, sattr, &l);
 
-	sattr->ia_mode &= ~current_umask();
+	if (!(server->caps & NFS_CAP_MODE_UMASK))
+		sattr->ia_mode &= ~current_umask();
 	state = nfs4_do_open(dir, ctx, flags, sattr, ilabel, NULL);
 	if (IS_ERR(state)) {
 		status = PTR_ERR(state);
@@ -4172,6 +4178,7 @@ static struct nfs4_createdata *nfs4_alloc_createdata(struct inode *dir,
 		data->arg.attrs = sattr;
 		data->arg.ftype = ftype;
 		data->arg.bitmask = nfs4_bitmask(server, data->label);
+		data->arg.umask = current_umask();
 		data->res.server = server;
 		data->res.fh = &data->fh;
 		data->res.fattr = &data->fattr;
@@ -4269,13 +4276,15 @@ static int _nfs4_proc_mkdir(struct inode *dir, struct dentry *dentry,
 static int nfs4_proc_mkdir(struct inode *dir, struct dentry *dentry,
 		struct iattr *sattr)
 {
+	struct nfs_server *server = NFS_SERVER(dir);
 	struct nfs4_exception exception = { };
 	struct nfs4_label l, *label = NULL;
 	int err;
 
 	label = nfs4_label_init_security(dir, dentry, sattr, &l);
 
-	sattr->ia_mode &= ~current_umask();
+	if (!(server->caps & NFS_CAP_MODE_UMASK))
+		sattr->ia_mode &= ~current_umask();
 	do {
 		err = _nfs4_proc_mkdir(dir, dentry, sattr, label);
 		trace_nfs4_mkdir(dir, &dentry->d_name, err);
@@ -4378,13 +4387,15 @@ static int _nfs4_proc_mknod(struct inode *dir, struct dentry *dentry,
 static int nfs4_proc_mknod(struct inode *dir, struct dentry *dentry,
 		struct iattr *sattr, dev_t rdev)
 {
+	struct nfs_server *server = NFS_SERVER(dir);
 	struct nfs4_exception exception = { };
 	struct nfs4_label l, *label = NULL;
 	int err;
 
 	label = nfs4_label_init_security(dir, dentry, sattr, &l);
 
-	sattr->ia_mode &= ~current_umask();
+	if (!(server->caps & NFS_CAP_MODE_UMASK))
+		sattr->ia_mode &= ~current_umask();
 	do {
 		err = _nfs4_proc_mknod(dir, dentry, sattr, label, rdev);
 		trace_nfs4_mknod(dir, &dentry->d_name, err);
diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
index fc89e5ed07ee..420d27865504 100644
--- a/fs/nfs/nfs4xdr.c
+++ b/fs/nfs/nfs4xdr.c
@@ -52,6 +52,7 @@
 #include <linux/nfs.h>
 #include <linux/nfs4.h>
 #include <linux/nfs_fs.h>
+#include <linux/fs_struct.h>
 
 #include "nfs4_fs.h"
 #include "internal.h"
@@ -1003,7 +1004,7 @@ static void encode_nfs4_verifier(struct xdr_stream *xdr, const nfs4_verifier *ve
 static void encode_attrs(struct xdr_stream *xdr, const struct iattr *iap,
 				const struct nfs4_label *label,
 				const struct nfs_server *server,
-				bool excl_check)
+				bool excl_check, const umode_t *umask)
 {
 	char owner_name[IDMAP_NAMESZ];
 	char owner_group[IDMAP_NAMESZ];
@@ -1017,18 +1018,21 @@ static void encode_attrs(struct xdr_stream *xdr, const struct iattr *iap,
 
 	/*
 	 * We reserve enough space to write the entire attribute buffer at once.
-	 * In the worst-case, this would be
-	 * 16(bitmap) + 4(attrlen) + 8(size) + 4(mode) + 4(atime) + 4(mtime)
-	 * = 40 bytes, plus any contribution from variable-length fields
-	 *            such as owner/group.
 	 */
 	if (iap->ia_valid & ATTR_SIZE) {
 		bmval[0] |= FATTR4_WORD0_SIZE;
 		len += 8;
 	}
+	if (!(server->caps & NFS_CAP_MODE_UMASK))
+		umask = NULL;
 	if (iap->ia_valid & ATTR_MODE) {
-		bmval[1] |= FATTR4_WORD1_MODE;
-		len += 4;
+		if (umask) {
+			bmval[2] |= FATTR4_WORD2_MODE_UMASK;
+			len += 8;
+		} else {
+			bmval[1] |= FATTR4_WORD1_MODE;
+			len += 4;
+		}
 	}
 	if (iap->ia_valid & ATTR_UID) {
 		owner_namelen = nfs_map_uid_to_name(server, iap->ia_uid, owner_name, IDMAP_NAMESZ);
@@ -1129,6 +1133,10 @@ static void encode_attrs(struct xdr_stream *xdr, const struct iattr *iap,
 		*p++ = cpu_to_be32(label->len);
 		p = xdr_encode_opaque_fixed(p, label->label, label->len);
 	}
+	if (bmval[2] & FATTR4_WORD2_MODE_UMASK) {
+		*p++ = cpu_to_be32(iap->ia_mode & S_IALLUGO);
+		*p++ = cpu_to_be32(*umask);
+	}
 
 /* out: */
 }
@@ -1183,7 +1191,8 @@ static void encode_create(struct xdr_stream *xdr, const struct nfs4_create_arg *
 	}
 
 	encode_string(xdr, create->name->len, create->name->name);
-	encode_attrs(xdr, create->attrs, create->label, create->server, false);
+	encode_attrs(xdr, create->attrs, create->label, create->server, false,
+		     &create->umask);
 }
 
 static void encode_getattr_one(struct xdr_stream *xdr, uint32_t bitmap, struct compound_hdr *hdr)
@@ -1403,11 +1412,13 @@ static inline void encode_createmode(struct xdr_stream *xdr, const struct nfs_op
 	switch(arg->createmode) {
 	case NFS4_CREATE_UNCHECKED:
 		*p = cpu_to_be32(NFS4_CREATE_UNCHECKED);
-		encode_attrs(xdr, arg->u.attrs, arg->label, arg->server, false);
+		encode_attrs(xdr, arg->u.attrs, arg->label, arg->server, false,
+			     &arg->umask);
 		break;
 	case NFS4_CREATE_GUARDED:
 		*p = cpu_to_be32(NFS4_CREATE_GUARDED);
-		encode_attrs(xdr, arg->u.attrs, arg->label, arg->server, false);
+		encode_attrs(xdr, arg->u.attrs, arg->label, arg->server, false,
+			     &arg->umask);
 		break;
 	case NFS4_CREATE_EXCLUSIVE:
 		*p = cpu_to_be32(NFS4_CREATE_EXCLUSIVE);
@@ -1416,7 +1427,8 @@ static inline void encode_createmode(struct xdr_stream *xdr, const struct nfs_op
 	case NFS4_CREATE_EXCLUSIVE4_1:
 		*p = cpu_to_be32(NFS4_CREATE_EXCLUSIVE4_1);
 		encode_nfs4_verifier(xdr, &arg->u.verifier);
-		encode_attrs(xdr, arg->u.attrs, arg->label, arg->server, true);
+		encode_attrs(xdr, arg->u.attrs, arg->label, arg->server, true,
+			     &arg->umask);
 	}
 }
 
@@ -1672,7 +1684,7 @@ static void encode_setattr(struct xdr_stream *xdr, const struct nfs_setattrargs
 {
 	encode_op_hdr(xdr, OP_SETATTR, decode_setattr_maxsz, hdr);
 	encode_nfs4_stateid(xdr, &arg->stateid);
-	encode_attrs(xdr, arg->iap, arg->label, server, false);
+	encode_attrs(xdr, arg->iap, arg->label, server, false, NULL);
 }
 
 static void encode_setclientid(struct xdr_stream *xdr, const struct nfs4_setclientid *setclientid, struct compound_hdr *hdr)
diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h
index 9094faf0699d..bca536341d1a 100644
--- a/include/linux/nfs4.h
+++ b/include/linux/nfs4.h
@@ -440,6 +440,7 @@ enum lock_type4 {
 #define FATTR4_WORD2_MDSTHRESHOLD       (1UL << 4)
 #define FATTR4_WORD2_CLONE_BLKSIZE	(1UL << 13)
 #define FATTR4_WORD2_SECURITY_LABEL     (1UL << 16)
+#define FATTR4_WORD2_MODE_UMASK		(1UL << 17)
 
 /* MDS threshold bitmap bits */
 #define THRESHOLD_RD                    (1UL << 0)
diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
index b34097c67848..87ab710772b3 100644
--- a/include/linux/nfs_fs_sb.h
+++ b/include/linux/nfs_fs_sb.h
@@ -250,5 +250,6 @@ struct nfs_server {
 #define NFS_CAP_LAYOUTSTATS	(1U << 22)
 #define NFS_CAP_CLONE		(1U << 23)
 #define NFS_CAP_COPY		(1U << 24)
+#define NFS_CAP_MODE_UMASK	(1U << 25)
 
 #endif
diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
index beb1e10f446e..ff82f42da656 100644
--- a/include/linux/nfs_xdr.h
+++ b/include/linux/nfs_xdr.h
@@ -418,6 +418,7 @@ struct nfs_openargs {
 	enum open_claim_type4	claim;
 	enum createmode4	createmode;
 	const struct nfs4_label *label;
+	umode_t			umask;
 };
 
 struct nfs_openres {
@@ -937,6 +938,7 @@ struct nfs4_create_arg {
 	const struct nfs_fh *		dir_fh;
 	const u32 *			bitmask;
 	const struct nfs4_label		*label;
+	umode_t				umask;
 };
 
 struct nfs4_create_res {
-- 
2.9.3

Re: [PATCH 1/2] nfs: add support for the umask attribute

[J. Bruce Fields]

On Wed, Nov 23, 2016 at 03:41:39PM -0500, J. Bruce Fields wrote:
> From: Andreas Gruenbacher <agruenba@redhat.com>
> 
> Clients can set the umask attribute when creating files to cause the
> server to apply it always except when inheriting permissions from the
> parent directory.  That way, the new files will end up with the same
> permissions as files created locally.

Trond asked out of band whether we could do away with the new
server->caps bit and instead directly use the supported attribute
bitmask (which is now also stored with the server).

I haven't given this a real test yet, but it looks fine to me.

If nobody sees an objection then I'll fold this into the client patch
and resend.

--b.

diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index 0a8326bcb481..32969907e218 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -1537,7 +1537,7 @@ int nfs_atomic_open(struct inode *dir, struct dentry *dentry,
 	if (open_flags & O_CREAT) {
 		struct nfs_server *server = NFS_SERVER(dir);
 
-		if (!(server->caps & NFS_CAP_MODE_UMASK))
+		if (!(server->attr_bitmask_nl[2] & FATTR4_WORD2_MODE_UMASK))
 			mode &= ~current_umask();
 
 		attr.ia_valid |= ATTR_MODE;
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 1fa15fbf7b48..960ba55ddde7 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -3323,8 +3323,7 @@ static int _nfs4_server_capabilities(struct nfs_server *server, struct nfs_fh *f
 				NFS_CAP_MODE|NFS_CAP_NLINK|NFS_CAP_OWNER|
 				NFS_CAP_OWNER_GROUP|NFS_CAP_ATIME|
 				NFS_CAP_CTIME|NFS_CAP_MTIME|
-				NFS_CAP_SECURITY_LABEL|
-				NFS_CAP_MODE_UMASK);
+				NFS_CAP_SECURITY_LABEL);
 		if (res.attr_bitmask[0] & FATTR4_WORD0_ACL &&
 				res.acl_bitmask & ACL4_SUPPORT_ALLOW_ACL)
 			server->caps |= NFS_CAP_ACLS;
@@ -3352,8 +3351,6 @@ static int _nfs4_server_capabilities(struct nfs_server *server, struct nfs_fh *f
 		if (res.attr_bitmask[2] & FATTR4_WORD2_SECURITY_LABEL)
 			server->caps |= NFS_CAP_SECURITY_LABEL;
 #endif
-		if (res.attr_bitmask[2] & FATTR4_WORD2_MODE_UMASK)
-			server->caps |= NFS_CAP_MODE_UMASK;
 		memcpy(server->attr_bitmask_nl, res.attr_bitmask,
 				sizeof(server->attr_bitmask));
 		server->attr_bitmask_nl[2] &= ~FATTR4_WORD2_SECURITY_LABEL;
@@ -3969,7 +3966,7 @@ nfs4_proc_create(struct inode *dir, struct dentry *dentry, struct iattr *sattr,
 
 	ilabel = nfs4_label_init_security(dir, dentry, sattr, &l);
 
-	if (!(server->caps & NFS_CAP_MODE_UMASK))
+	if (!(server->attr_bitmask_nl[2] & FATTR4_WORD2_MODE_UMASK))
 		sattr->ia_mode &= ~current_umask();
 	state = nfs4_do_open(dir, ctx, flags, sattr, ilabel, NULL);
 	if (IS_ERR(state)) {
@@ -4283,7 +4280,7 @@ static int nfs4_proc_mkdir(struct inode *dir, struct dentry *dentry,
 
 	label = nfs4_label_init_security(dir, dentry, sattr, &l);
 
-	if (!(server->caps & NFS_CAP_MODE_UMASK))
+	if (!(server->attr_bitmask_nl[2] & FATTR4_WORD2_MODE_UMASK))
 		sattr->ia_mode &= ~current_umask();
 	do {
 		err = _nfs4_proc_mkdir(dir, dentry, sattr, label);
@@ -4394,7 +4391,7 @@ static int nfs4_proc_mknod(struct inode *dir, struct dentry *dentry,
 
 	label = nfs4_label_init_security(dir, dentry, sattr, &l);
 
-	if (!(server->caps & NFS_CAP_MODE_UMASK))
+	if (!(server->attr_bitmask_nl[2] & FATTR4_WORD2_MODE_UMASK))
 		sattr->ia_mode &= ~current_umask();
 	do {
 		err = _nfs4_proc_mknod(dir, dentry, sattr, label, rdev);
diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
index 420d27865504..54714ce5dbd1 100644
--- a/fs/nfs/nfs4xdr.c
+++ b/fs/nfs/nfs4xdr.c
@@ -1023,7 +1023,7 @@ static void encode_attrs(struct xdr_stream *xdr, const struct iattr *iap,
 		bmval[0] |= FATTR4_WORD0_SIZE;
 		len += 8;
 	}
-	if (!(server->caps & NFS_CAP_MODE_UMASK))
+	if (!(server->attr_bitmask_nl[2] & FATTR4_WORD2_MODE_UMASK))
 		umask = NULL;
 	if (iap->ia_valid & ATTR_MODE) {
 		if (umask) {
diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
index 87ab710772b3..b34097c67848 100644
--- a/include/linux/nfs_fs_sb.h
+++ b/include/linux/nfs_fs_sb.h
@@ -250,6 +250,5 @@ struct nfs_server {
 #define NFS_CAP_LAYOUTSTATS	(1U << 22)
 #define NFS_CAP_CLONE		(1U << 23)
 #define NFS_CAP_COPY		(1U << 24)
-#define NFS_CAP_MODE_UMASK	(1U << 25)
 
 #endif

Re: [PATCH 1/2] nfs: add support for the umask attribute

[Andreas Gruenbacher]

On Thu, Dec 1, 2016 at 11:07 PM, J. Bruce Fields <bfields@fieldses.org> wrote:
> On Wed, Nov 23, 2016 at 03:41:39PM -0500, J. Bruce Fields wrote:
>> From: Andreas Gruenbacher <agruenba@redhat.com>
>>
>> Clients can set the umask attribute when creating files to cause the
>> server to apply it always except when inheriting permissions from the
>> parent directory.  That way, the new files will end up with the same
>> permissions as files created locally.
>
> Trond asked out of band whether we could do away with the new
> server->caps bit and instead directly use the supported attribute
> bitmask (which is now also stored with the server).
>
> I haven't given this a real test yet, but it looks fine to me.
>
> If nobody sees an objection then I'll fold this into the client patch
> and resend.

Sure. I'm wondering why the code isn't checking for any other
attributes like that.

> --b.
>
> diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
> index 0a8326bcb481..32969907e218 100644
> --- a/fs/nfs/dir.c
> +++ b/fs/nfs/dir.c
> @@ -1537,7 +1537,7 @@ int nfs_atomic_open(struct inode *dir, struct dentry *dentry,
>         if (open_flags & O_CREAT) {
>                 struct nfs_server *server = NFS_SERVER(dir);
>
> -               if (!(server->caps & NFS_CAP_MODE_UMASK))
> +               if (!(server->attr_bitmask_nl[2] & FATTR4_WORD2_MODE_UMASK))

It seems that this patch should be checking in server->attr_bitmask,
not server->attr_bitmask_nl.

>                         mode &= ~current_umask();
>
>                 attr.ia_valid |= ATTR_MODE;
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 1fa15fbf7b48..960ba55ddde7 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -3323,8 +3323,7 @@ static int _nfs4_server_capabilities(struct nfs_server *server, struct nfs_fh *f
>                                 NFS_CAP_MODE|NFS_CAP_NLINK|NFS_CAP_OWNER|
>                                 NFS_CAP_OWNER_GROUP|NFS_CAP_ATIME|
>                                 NFS_CAP_CTIME|NFS_CAP_MTIME|
> -                               NFS_CAP_SECURITY_LABEL|
> -                               NFS_CAP_MODE_UMASK);
> +                               NFS_CAP_SECURITY_LABEL);
>                 if (res.attr_bitmask[0] & FATTR4_WORD0_ACL &&
>                                 res.acl_bitmask & ACL4_SUPPORT_ALLOW_ACL)
>                         server->caps |= NFS_CAP_ACLS;
> @@ -3352,8 +3351,6 @@ static int _nfs4_server_capabilities(struct nfs_server *server, struct nfs_fh *f
>                 if (res.attr_bitmask[2] & FATTR4_WORD2_SECURITY_LABEL)
>                         server->caps |= NFS_CAP_SECURITY_LABEL;
>  #endif
> -               if (res.attr_bitmask[2] & FATTR4_WORD2_MODE_UMASK)
> -                       server->caps |= NFS_CAP_MODE_UMASK;
>                 memcpy(server->attr_bitmask_nl, res.attr_bitmask,
>                                 sizeof(server->attr_bitmask));
>                 server->attr_bitmask_nl[2] &= ~FATTR4_WORD2_SECURITY_LABEL;
> @@ -3969,7 +3966,7 @@ nfs4_proc_create(struct inode *dir, struct dentry *dentry, struct iattr *sattr,
>
>         ilabel = nfs4_label_init_security(dir, dentry, sattr, &l);
>
> -       if (!(server->caps & NFS_CAP_MODE_UMASK))
> +       if (!(server->attr_bitmask_nl[2] & FATTR4_WORD2_MODE_UMASK))
>                 sattr->ia_mode &= ~current_umask();
>         state = nfs4_do_open(dir, ctx, flags, sattr, ilabel, NULL);
>         if (IS_ERR(state)) {
> @@ -4283,7 +4280,7 @@ static int nfs4_proc_mkdir(struct inode *dir, struct dentry *dentry,
>
>         label = nfs4_label_init_security(dir, dentry, sattr, &l);
>
> -       if (!(server->caps & NFS_CAP_MODE_UMASK))
> +       if (!(server->attr_bitmask_nl[2] & FATTR4_WORD2_MODE_UMASK))
>                 sattr->ia_mode &= ~current_umask();
>         do {
>                 err = _nfs4_proc_mkdir(dir, dentry, sattr, label);
> @@ -4394,7 +4391,7 @@ static int nfs4_proc_mknod(struct inode *dir, struct dentry *dentry,
>
>         label = nfs4_label_init_security(dir, dentry, sattr, &l);
>
> -       if (!(server->caps & NFS_CAP_MODE_UMASK))
> +       if (!(server->attr_bitmask_nl[2] & FATTR4_WORD2_MODE_UMASK))
>                 sattr->ia_mode &= ~current_umask();
>         do {
>                 err = _nfs4_proc_mknod(dir, dentry, sattr, label, rdev);
> diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
> index 420d27865504..54714ce5dbd1 100644
> --- a/fs/nfs/nfs4xdr.c
> +++ b/fs/nfs/nfs4xdr.c
> @@ -1023,7 +1023,7 @@ static void encode_attrs(struct xdr_stream *xdr, const struct iattr *iap,
>                 bmval[0] |= FATTR4_WORD0_SIZE;
>                 len += 8;
>         }
> -       if (!(server->caps & NFS_CAP_MODE_UMASK))
> +       if (!(server->attr_bitmask_nl[2] & FATTR4_WORD2_MODE_UMASK))
>                 umask = NULL;
>         if (iap->ia_valid & ATTR_MODE) {
>                 if (umask) {
> diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
> index 87ab710772b3..b34097c67848 100644
> --- a/include/linux/nfs_fs_sb.h
> +++ b/include/linux/nfs_fs_sb.h
> @@ -250,6 +250,5 @@ struct nfs_server {
>  #define NFS_CAP_LAYOUTSTATS    (1U << 22)
>  #define NFS_CAP_CLONE          (1U << 23)
>  #define NFS_CAP_COPY           (1U << 24)
> -#define NFS_CAP_MODE_UMASK     (1U << 25)
>
>  #endif

Thanks,
Andreas

Re: [PATCH 1/2] nfs: add support for the umask attribute

[J. Bruce Fields]

On Fri, Dec 02, 2016 at 02:12:26PM +0100, Andreas Gruenbacher wrote:
> On Thu, Dec 1, 2016 at 11:07 PM, J. Bruce Fields <bfields@fieldses.org> wrote:
> > On Wed, Nov 23, 2016 at 03:41:39PM -0500, J. Bruce Fields wrote:
> >> From: Andreas Gruenbacher <agruenba@redhat.com>
> >>
> >> Clients can set the umask attribute when creating files to cause the
> >> server to apply it always except when inheriting permissions from the
> >> parent directory.  That way, the new files will end up with the same
> >> permissions as files created locally.
> >
> > Trond asked out of band whether we could do away with the new
> > server->caps bit and instead directly use the supported attribute
> > bitmask (which is now also stored with the server).
> >
> > I haven't given this a real test yet, but it looks fine to me.
> >
> > If nobody sees an objection then I'll fold this into the client patch
> > and resend.
> 
> Sure. I'm wondering why the code isn't checking for any other
> attributes like that.

No idea.

> > diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
> > index 0a8326bcb481..32969907e218 100644
> > --- a/fs/nfs/dir.c
> > +++ b/fs/nfs/dir.c
> > @@ -1537,7 +1537,7 @@ int nfs_atomic_open(struct inode *dir, struct dentry *dentry,
> >         if (open_flags & O_CREAT) {
> >                 struct nfs_server *server = NFS_SERVER(dir);
> >
> > -               if (!(server->caps & NFS_CAP_MODE_UMASK))
> > +               if (!(server->attr_bitmask_nl[2] & FATTR4_WORD2_MODE_UMASK))
> 
> It seems that this patch should be checking in server->attr_bitmask,
> not server->attr_bitmask_nl.

Huh, I missed that distinction.

Looks like the results are the same, but, sure, attr_bitmask is probably
better; fixed.  Thanks for taking a look.

--b.

> 
> >                         mode &= ~current_umask();
> >
> >                 attr.ia_valid |= ATTR_MODE;
> > diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> > index 1fa15fbf7b48..960ba55ddde7 100644
> > --- a/fs/nfs/nfs4proc.c
> > +++ b/fs/nfs/nfs4proc.c
> > @@ -3323,8 +3323,7 @@ static int _nfs4_server_capabilities(struct nfs_server *server, struct nfs_fh *f
> >                                 NFS_CAP_MODE|NFS_CAP_NLINK|NFS_CAP_OWNER|
> >                                 NFS_CAP_OWNER_GROUP|NFS_CAP_ATIME|
> >                                 NFS_CAP_CTIME|NFS_CAP_MTIME|
> > -                               NFS_CAP_SECURITY_LABEL|
> > -                               NFS_CAP_MODE_UMASK);
> > +                               NFS_CAP_SECURITY_LABEL);
> >                 if (res.attr_bitmask[0] & FATTR4_WORD0_ACL &&
> >                                 res.acl_bitmask & ACL4_SUPPORT_ALLOW_ACL)
> >                         server->caps |= NFS_CAP_ACLS;
> > @@ -3352,8 +3351,6 @@ static int _nfs4_server_capabilities(struct nfs_server *server, struct nfs_fh *f
> >                 if (res.attr_bitmask[2] & FATTR4_WORD2_SECURITY_LABEL)
> >                         server->caps |= NFS_CAP_SECURITY_LABEL;
> >  #endif
> > -               if (res.attr_bitmask[2] & FATTR4_WORD2_MODE_UMASK)
> > -                       server->caps |= NFS_CAP_MODE_UMASK;
> >                 memcpy(server->attr_bitmask_nl, res.attr_bitmask,
> >                                 sizeof(server->attr_bitmask));
> >                 server->attr_bitmask_nl[2] &= ~FATTR4_WORD2_SECURITY_LABEL;
> > @@ -3969,7 +3966,7 @@ nfs4_proc_create(struct inode *dir, struct dentry *dentry, struct iattr *sattr,
> >
> >         ilabel = nfs4_label_init_security(dir, dentry, sattr, &l);
> >
> > -       if (!(server->caps & NFS_CAP_MODE_UMASK))
> > +       if (!(server->attr_bitmask_nl[2] & FATTR4_WORD2_MODE_UMASK))
> >                 sattr->ia_mode &= ~current_umask();
> >         state = nfs4_do_open(dir, ctx, flags, sattr, ilabel, NULL);
> >         if (IS_ERR(state)) {
> > @@ -4283,7 +4280,7 @@ static int nfs4_proc_mkdir(struct inode *dir, struct dentry *dentry,
> >
> >         label = nfs4_label_init_security(dir, dentry, sattr, &l);
> >
> > -       if (!(server->caps & NFS_CAP_MODE_UMASK))
> > +       if (!(server->attr_bitmask_nl[2] & FATTR4_WORD2_MODE_UMASK))
> >                 sattr->ia_mode &= ~current_umask();
> >         do {
> >                 err = _nfs4_proc_mkdir(dir, dentry, sattr, label);
> > @@ -4394,7 +4391,7 @@ static int nfs4_proc_mknod(struct inode *dir, struct dentry *dentry,
> >
> >         label = nfs4_label_init_security(dir, dentry, sattr, &l);
> >
> > -       if (!(server->caps & NFS_CAP_MODE_UMASK))
> > +       if (!(server->attr_bitmask_nl[2] & FATTR4_WORD2_MODE_UMASK))
> >                 sattr->ia_mode &= ~current_umask();
> >         do {
> >                 err = _nfs4_proc_mknod(dir, dentry, sattr, label, rdev);
> > diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
> > index 420d27865504..54714ce5dbd1 100644
> > --- a/fs/nfs/nfs4xdr.c
> > +++ b/fs/nfs/nfs4xdr.c
> > @@ -1023,7 +1023,7 @@ static void encode_attrs(struct xdr_stream *xdr, const struct iattr *iap,
> >                 bmval[0] |= FATTR4_WORD0_SIZE;
> >                 len += 8;
> >         }
> > -       if (!(server->caps & NFS_CAP_MODE_UMASK))
> > +       if (!(server->attr_bitmask_nl[2] & FATTR4_WORD2_MODE_UMASK))
> >                 umask = NULL;
> >         if (iap->ia_valid & ATTR_MODE) {
> >                 if (umask) {
> > diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
> > index 87ab710772b3..b34097c67848 100644
> > --- a/include/linux/nfs_fs_sb.h
> > +++ b/include/linux/nfs_fs_sb.h
> > @@ -250,6 +250,5 @@ struct nfs_server {
> >  #define NFS_CAP_LAYOUTSTATS    (1U << 22)
> >  #define NFS_CAP_CLONE          (1U << 23)
> >  #define NFS_CAP_COPY           (1U << 24)
> > -#define NFS_CAP_MODE_UMASK     (1U << 25)
> >
> >  #endif
> 
> Thanks,
> Andreas

[PATCH 1/2] nfs: add support for the umask attribute

[J. Bruce Fields]

From: Andreas Gruenbacher <agruenba@redhat.com>

Clients can set the umask attribute when creating files to cause the
server to apply it always except when inheriting permissions from the
parent directory.  That way, the new files will end up with the same
permissions as files created locally.

See https://tools.ietf.org/html/draft-ietf-nfsv4-umask-02 for more details.

Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/nfs/dir.c            |  7 ++++++-
 fs/nfs/nfs4proc.c       | 16 ++++++++++++----
 fs/nfs/nfs4xdr.c        | 36 ++++++++++++++++++++++++------------
 include/linux/nfs4.h    |  1 +
 include/linux/nfs_xdr.h |  2 ++
 5 files changed, 45 insertions(+), 17 deletions(-)

diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index 5f1af4cd1a33..837f12e683df 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -1535,8 +1535,13 @@ int nfs_atomic_open(struct inode *dir, struct dentry *dentry,
 		return -ENAMETOOLONG;
 
 	if (open_flags & O_CREAT) {
+		struct nfs_server *server = NFS_SERVER(dir);
+
+		if (!(server->attr_bitmask[2] & FATTR4_WORD2_MODE_UMASK))
+			mode &= ~current_umask();
+
 		attr.ia_valid |= ATTR_MODE;
-		attr.ia_mode = mode & ~current_umask();
+		attr.ia_mode = mode;
 	}
 	if (open_flags & O_TRUNC) {
 		attr.ia_valid |= ATTR_SIZE;
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 7897826d7c51..df739778e4b3 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -1239,6 +1239,7 @@ static struct nfs4_opendata *nfs4_opendata_alloc(struct dentry *dentry,
 	p->o_arg.bitmask = nfs4_bitmask(server, label);
 	p->o_arg.open_bitmap = &nfs4_fattr_bitmap[0];
 	p->o_arg.label = nfs4_label_copy(p->a_label, label);
+	p->o_arg.umask = current_umask();
 	p->o_arg.claim = nfs4_map_atomic_open_claim(server, claim);
 	switch (p->o_arg.claim) {
 	case NFS4_OPEN_CLAIM_NULL:
@@ -3277,7 +3278,7 @@ static void nfs4_close_context(struct nfs_open_context *ctx, int is_sync)
 
 #define FATTR4_WORD1_NFS40_MASK (2*FATTR4_WORD1_MOUNTED_ON_FILEID - 1UL)
 #define FATTR4_WORD2_NFS41_MASK (2*FATTR4_WORD2_SUPPATTR_EXCLCREAT - 1UL)
-#define FATTR4_WORD2_NFS42_MASK (2*FATTR4_WORD2_SECURITY_LABEL - 1UL)
+#define FATTR4_WORD2_NFS42_MASK (2*FATTR4_WORD2_MODE_UMASK - 1UL)
 
 static int _nfs4_server_capabilities(struct nfs_server *server, struct nfs_fh *fhandle)
 {
@@ -3953,6 +3954,7 @@ static int
 nfs4_proc_create(struct inode *dir, struct dentry *dentry, struct iattr *sattr,
 		 int flags)
 {
+	struct nfs_server *server = NFS_SERVER(dir);
 	struct nfs4_label l, *ilabel = NULL;
 	struct nfs_open_context *ctx;
 	struct nfs4_state *state;
@@ -3964,7 +3966,8 @@ nfs4_proc_create(struct inode *dir, struct dentry *dentry, struct iattr *sattr,
 
 	ilabel = nfs4_label_init_security(dir, dentry, sattr, &l);
 
-	sattr->ia_mode &= ~current_umask();
+	if (!(server->attr_bitmask[2] & FATTR4_WORD2_MODE_UMASK))
+		sattr->ia_mode &= ~current_umask();
 	state = nfs4_do_open(dir, ctx, flags, sattr, ilabel, NULL);
 	if (IS_ERR(state)) {
 		status = PTR_ERR(state);
@@ -4172,6 +4175,7 @@ static struct nfs4_createdata *nfs4_alloc_createdata(struct inode *dir,
 		data->arg.attrs = sattr;
 		data->arg.ftype = ftype;
 		data->arg.bitmask = nfs4_bitmask(server, data->label);
+		data->arg.umask = current_umask();
 		data->res.server = server;
 		data->res.fh = &data->fh;
 		data->res.fattr = &data->fattr;
@@ -4269,13 +4273,15 @@ static int _nfs4_proc_mkdir(struct inode *dir, struct dentry *dentry,
 static int nfs4_proc_mkdir(struct inode *dir, struct dentry *dentry,
 		struct iattr *sattr)
 {
+	struct nfs_server *server = NFS_SERVER(dir);
 	struct nfs4_exception exception = { };
 	struct nfs4_label l, *label = NULL;
 	int err;
 
 	label = nfs4_label_init_security(dir, dentry, sattr, &l);
 
-	sattr->ia_mode &= ~current_umask();
+	if (!(server->attr_bitmask[2] & FATTR4_WORD2_MODE_UMASK))
+		sattr->ia_mode &= ~current_umask();
 	do {
 		err = _nfs4_proc_mkdir(dir, dentry, sattr, label);
 		trace_nfs4_mkdir(dir, &dentry->d_name, err);
@@ -4378,13 +4384,15 @@ static int _nfs4_proc_mknod(struct inode *dir, struct dentry *dentry,
 static int nfs4_proc_mknod(struct inode *dir, struct dentry *dentry,
 		struct iattr *sattr, dev_t rdev)
 {
+	struct nfs_server *server = NFS_SERVER(dir);
 	struct nfs4_exception exception = { };
 	struct nfs4_label l, *label = NULL;
 	int err;
 
 	label = nfs4_label_init_security(dir, dentry, sattr, &l);
 
-	sattr->ia_mode &= ~current_umask();
+	if (!(server->attr_bitmask[2] & FATTR4_WORD2_MODE_UMASK))
+		sattr->ia_mode &= ~current_umask();
 	do {
 		err = _nfs4_proc_mknod(dir, dentry, sattr, label, rdev);
 		trace_nfs4_mknod(dir, &dentry->d_name, err);
diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
index fc89e5ed07ee..b94706f0668b 100644
--- a/fs/nfs/nfs4xdr.c
+++ b/fs/nfs/nfs4xdr.c
@@ -52,6 +52,7 @@
 #include <linux/nfs.h>
 #include <linux/nfs4.h>
 #include <linux/nfs_fs.h>
+#include <linux/fs_struct.h>
 
 #include "nfs4_fs.h"
 #include "internal.h"
@@ -1003,7 +1004,7 @@ static void encode_nfs4_verifier(struct xdr_stream *xdr, const nfs4_verifier *ve
 static void encode_attrs(struct xdr_stream *xdr, const struct iattr *iap,
 				const struct nfs4_label *label,
 				const struct nfs_server *server,
-				bool excl_check)
+				bool excl_check, const umode_t *umask)
 {
 	char owner_name[IDMAP_NAMESZ];
 	char owner_group[IDMAP_NAMESZ];
@@ -1017,18 +1018,21 @@ static void encode_attrs(struct xdr_stream *xdr, const struct iattr *iap,
 
 	/*
 	 * We reserve enough space to write the entire attribute buffer at once.
-	 * In the worst-case, this would be
-	 * 16(bitmap) + 4(attrlen) + 8(size) + 4(mode) + 4(atime) + 4(mtime)
-	 * = 40 bytes, plus any contribution from variable-length fields
-	 *            such as owner/group.
 	 */
 	if (iap->ia_valid & ATTR_SIZE) {
 		bmval[0] |= FATTR4_WORD0_SIZE;
 		len += 8;
 	}
+	if (!(server->attr_bitmask[2] & FATTR4_WORD2_MODE_UMASK))
+		umask = NULL;
 	if (iap->ia_valid & ATTR_MODE) {
-		bmval[1] |= FATTR4_WORD1_MODE;
-		len += 4;
+		if (umask) {
+			bmval[2] |= FATTR4_WORD2_MODE_UMASK;
+			len += 8;
+		} else {
+			bmval[1] |= FATTR4_WORD1_MODE;
+			len += 4;
+		}
 	}
 	if (iap->ia_valid & ATTR_UID) {
 		owner_namelen = nfs_map_uid_to_name(server, iap->ia_uid, owner_name, IDMAP_NAMESZ);
@@ -1129,6 +1133,10 @@ static void encode_attrs(struct xdr_stream *xdr, const struct iattr *iap,
 		*p++ = cpu_to_be32(label->len);
 		p = xdr_encode_opaque_fixed(p, label->label, label->len);
 	}
+	if (bmval[2] & FATTR4_WORD2_MODE_UMASK) {
+		*p++ = cpu_to_be32(iap->ia_mode & S_IALLUGO);
+		*p++ = cpu_to_be32(*umask);
+	}
 
 /* out: */
 }
@@ -1183,7 +1191,8 @@ static void encode_create(struct xdr_stream *xdr, const struct nfs4_create_arg *
 	}
 
 	encode_string(xdr, create->name->len, create->name->name);
-	encode_attrs(xdr, create->attrs, create->label, create->server, false);
+	encode_attrs(xdr, create->attrs, create->label, create->server, false,
+		     &create->umask);
 }
 
 static void encode_getattr_one(struct xdr_stream *xdr, uint32_t bitmap, struct compound_hdr *hdr)
@@ -1403,11 +1412,13 @@ static inline void encode_createmode(struct xdr_stream *xdr, const struct nfs_op
 	switch(arg->createmode) {
 	case NFS4_CREATE_UNCHECKED:
 		*p = cpu_to_be32(NFS4_CREATE_UNCHECKED);
-		encode_attrs(xdr, arg->u.attrs, arg->label, arg->server, false);
+		encode_attrs(xdr, arg->u.attrs, arg->label, arg->server, false,
+			     &arg->umask);
 		break;
 	case NFS4_CREATE_GUARDED:
 		*p = cpu_to_be32(NFS4_CREATE_GUARDED);
-		encode_attrs(xdr, arg->u.attrs, arg->label, arg->server, false);
+		encode_attrs(xdr, arg->u.attrs, arg->label, arg->server, false,
+			     &arg->umask);
 		break;
 	case NFS4_CREATE_EXCLUSIVE:
 		*p = cpu_to_be32(NFS4_CREATE_EXCLUSIVE);
@@ -1416,7 +1427,8 @@ static inline void encode_createmode(struct xdr_stream *xdr, const struct nfs_op
 	case NFS4_CREATE_EXCLUSIVE4_1:
 		*p = cpu_to_be32(NFS4_CREATE_EXCLUSIVE4_1);
 		encode_nfs4_verifier(xdr, &arg->u.verifier);
-		encode_attrs(xdr, arg->u.attrs, arg->label, arg->server, true);
+		encode_attrs(xdr, arg->u.attrs, arg->label, arg->server, true,
+			     &arg->umask);
 	}
 }
 
@@ -1672,7 +1684,7 @@ static void encode_setattr(struct xdr_stream *xdr, const struct nfs_setattrargs
 {
 	encode_op_hdr(xdr, OP_SETATTR, decode_setattr_maxsz, hdr);
 	encode_nfs4_stateid(xdr, &arg->stateid);
-	encode_attrs(xdr, arg->iap, arg->label, server, false);
+	encode_attrs(xdr, arg->iap, arg->label, server, false, NULL);
 }
 
 static void encode_setclientid(struct xdr_stream *xdr, const struct nfs4_setclientid *setclientid, struct compound_hdr *hdr)
diff --git a/include/linux/nfs4.h b/include/linux/nfs4.h
index 9094faf0699d..bca536341d1a 100644
--- a/include/linux/nfs4.h
+++ b/include/linux/nfs4.h
@@ -440,6 +440,7 @@ enum lock_type4 {
 #define FATTR4_WORD2_MDSTHRESHOLD       (1UL << 4)
 #define FATTR4_WORD2_CLONE_BLKSIZE	(1UL << 13)
 #define FATTR4_WORD2_SECURITY_LABEL     (1UL << 16)
+#define FATTR4_WORD2_MODE_UMASK		(1UL << 17)
 
 /* MDS threshold bitmap bits */
 #define THRESHOLD_RD                    (1UL << 0)
diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
index beb1e10f446e..ff82f42da656 100644
--- a/include/linux/nfs_xdr.h
+++ b/include/linux/nfs_xdr.h
@@ -418,6 +418,7 @@ struct nfs_openargs {
 	enum open_claim_type4	claim;
 	enum createmode4	createmode;
 	const struct nfs4_label *label;
+	umode_t			umask;
 };
 
 struct nfs_openres {
@@ -937,6 +938,7 @@ struct nfs4_create_arg {
 	const struct nfs_fh *		dir_fh;
 	const u32 *			bitmask;
 	const struct nfs4_label		*label;
+	umode_t				umask;
 };
 
 struct nfs4_create_res {
-- 
2.9.3

Re: [PATCH 1/2] nfs: add support for the umask attribute

[Olga Kornievskaia]

Any plans to add wireshark support for this?

On Fri, Dec 2, 2016 at 11:47 AM, J. Bruce Fields <bfields@redhat.com> wrote:
> On Fri, Dec 02, 2016 at 02:12:26PM +0100, Andreas Gruenbacher wrote:
>> On Thu, Dec 1, 2016 at 11:07 PM, J. Bruce Fields <bfields@fieldses.org> wrote:
>> > On Wed, Nov 23, 2016 at 03:41:39PM -0500, J. Bruce Fields wrote:
>> >> From: Andreas Gruenbacher <agruenba@redhat.com>
>> >>
>> >> Clients can set the umask attribute when creating files to cause the
>> >> server to apply it always except when inheriting permissions from the
>> >> parent directory.  That way, the new files will end up with the same
>> >> permissions as files created locally.
>> >
>> > Trond asked out of band whether we could do away with the new
>> > server->caps bit and instead directly use the supported attribute
>> > bitmask (which is now also stored with the server).
>> >
>> > I haven't given this a real test yet, but it looks fine to me.
>> >
>> > If nobody sees an objection then I'll fold this into the client patch
>> > and resend.
>>
>> Sure. I'm wondering why the code isn't checking for any other
>> attributes like that.
>
> No idea.
>
>> > diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
>> > index 0a8326bcb481..32969907e218 100644
>> > --- a/fs/nfs/dir.c
>> > +++ b/fs/nfs/dir.c
>> > @@ -1537,7 +1537,7 @@ int nfs_atomic_open(struct inode *dir, struct dentry *dentry,
>> >         if (open_flags & O_CREAT) {
>> >                 struct nfs_server *server = NFS_SERVER(dir);
>> >
>> > -               if (!(server->caps & NFS_CAP_MODE_UMASK))
>> > +               if (!(server->attr_bitmask_nl[2] & FATTR4_WORD2_MODE_UMASK))
>>
>> It seems that this patch should be checking in server->attr_bitmask,
>> not server->attr_bitmask_nl.
>
> Huh, I missed that distinction.
>
> Looks like the results are the same, but, sure, attr_bitmask is probably
> better; fixed.  Thanks for taking a look.
>
> --b.
>
>>
>> >                         mode &= ~current_umask();
>> >
>> >                 attr.ia_valid |= ATTR_MODE;
>> > diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
>> > index 1fa15fbf7b48..960ba55ddde7 100644
>> > --- a/fs/nfs/nfs4proc.c
>> > +++ b/fs/nfs/nfs4proc.c
>> > @@ -3323,8 +3323,7 @@ static int _nfs4_server_capabilities(struct nfs_server *server, struct nfs_fh *f
>> >                                 NFS_CAP_MODE|NFS_CAP_NLINK|NFS_CAP_OWNER|
>> >                                 NFS_CAP_OWNER_GROUP|NFS_CAP_ATIME|
>> >                                 NFS_CAP_CTIME|NFS_CAP_MTIME|
>> > -                               NFS_CAP_SECURITY_LABEL|
>> > -                               NFS_CAP_MODE_UMASK);
>> > +                               NFS_CAP_SECURITY_LABEL);
>> >                 if (res.attr_bitmask[0] & FATTR4_WORD0_ACL &&
>> >                                 res.acl_bitmask & ACL4_SUPPORT_ALLOW_ACL)
>> >                         server->caps |= NFS_CAP_ACLS;
>> > @@ -3352,8 +3351,6 @@ static int _nfs4_server_capabilities(struct nfs_server *server, struct nfs_fh *f
>> >                 if (res.attr_bitmask[2] & FATTR4_WORD2_SECURITY_LABEL)
>> >                         server->caps |= NFS_CAP_SECURITY_LABEL;
>> >  #endif
>> > -               if (res.attr_bitmask[2] & FATTR4_WORD2_MODE_UMASK)
>> > -                       server->caps |= NFS_CAP_MODE_UMASK;
>> >                 memcpy(server->attr_bitmask_nl, res.attr_bitmask,
>> >                                 sizeof(server->attr_bitmask));
>> >                 server->attr_bitmask_nl[2] &= ~FATTR4_WORD2_SECURITY_LABEL;
>> > @@ -3969,7 +3966,7 @@ nfs4_proc_create(struct inode *dir, struct dentry *dentry, struct iattr *sattr,
>> >
>> >         ilabel = nfs4_label_init_security(dir, dentry, sattr, &l);
>> >
>> > -       if (!(server->caps & NFS_CAP_MODE_UMASK))
>> > +       if (!(server->attr_bitmask_nl[2] & FATTR4_WORD2_MODE_UMASK))
>> >                 sattr->ia_mode &= ~current_umask();
>> >         state = nfs4_do_open(dir, ctx, flags, sattr, ilabel, NULL);
>> >         if (IS_ERR(state)) {
>> > @@ -4283,7 +4280,7 @@ static int nfs4_proc_mkdir(struct inode *dir, struct dentry *dentry,
>> >
>> >         label = nfs4_label_init_security(dir, dentry, sattr, &l);
>> >
>> > -       if (!(server->caps & NFS_CAP_MODE_UMASK))
>> > +       if (!(server->attr_bitmask_nl[2] & FATTR4_WORD2_MODE_UMASK))
>> >                 sattr->ia_mode &= ~current_umask();
>> >         do {
>> >                 err = _nfs4_proc_mkdir(dir, dentry, sattr, label);
>> > @@ -4394,7 +4391,7 @@ static int nfs4_proc_mknod(struct inode *dir, struct dentry *dentry,
>> >
>> >         label = nfs4_label_init_security(dir, dentry, sattr, &l);
>> >
>> > -       if (!(server->caps & NFS_CAP_MODE_UMASK))
>> > +       if (!(server->attr_bitmask_nl[2] & FATTR4_WORD2_MODE_UMASK))
>> >                 sattr->ia_mode &= ~current_umask();
>> >         do {
>> >                 err = _nfs4_proc_mknod(dir, dentry, sattr, label, rdev);
>> > diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
>> > index 420d27865504..54714ce5dbd1 100644
>> > --- a/fs/nfs/nfs4xdr.c
>> > +++ b/fs/nfs/nfs4xdr.c
>> > @@ -1023,7 +1023,7 @@ static void encode_attrs(struct xdr_stream *xdr, const struct iattr *iap,
>> >                 bmval[0] |= FATTR4_WORD0_SIZE;
>> >                 len += 8;
>> >         }
>> > -       if (!(server->caps & NFS_CAP_MODE_UMASK))
>> > +       if (!(server->attr_bitmask_nl[2] & FATTR4_WORD2_MODE_UMASK))
>> >                 umask = NULL;
>> >         if (iap->ia_valid & ATTR_MODE) {
>> >                 if (umask) {
>> > diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
>> > index 87ab710772b3..b34097c67848 100644
>> > --- a/include/linux/nfs_fs_sb.h
>> > +++ b/include/linux/nfs_fs_sb.h
>> > @@ -250,6 +250,5 @@ struct nfs_server {
>> >  #define NFS_CAP_LAYOUTSTATS    (1U << 22)
>> >  #define NFS_CAP_CLONE          (1U << 23)
>> >  #define NFS_CAP_COPY           (1U << 24)
>> > -#define NFS_CAP_MODE_UMASK     (1U << 25)
>> >
>> >  #endif
>>
>> Thanks,
>> Andreas
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] nfs: add support for the umask attribute

[J. Bruce Fields]

On Wed, Feb 01, 2017 at 04:31:25PM -0500, Olga Kornievskaia wrote:
> Any plans to add wireshark support for this?

Digging around, all I can find is this, which is outdated (it's for the
umask not the umask+mode attribute), and never went upstream.

I'll try to get back to this soon.  (But if somebody else can fist,
great.)

--b.

commit ed7c3053a99e
Author: Andreas Gruenbacher <agruenba@redhat.com>
Date:   Thu Jan 14 21:20:38 2016 +0100

    NFS: Add support for the proposed umask attribute
    
    Clients can set the umask attribute when creating files to cause the
    server to apply it always except when inheriting permissions from the
    parent directory.
    
    Change-Id: Id2a23c6839021107fdb32252be4a689b6125222c
    Signed-off-by: J. Bruce Fields <bfields@fieldses.org>

diff --git a/epan/dissectors/packet-nfs.c b/epan/dissectors/packet-nfs.c
index 010db6e96761..bf458d4bccf4 100644
--- a/epan/dissectors/packet-nfs.c
+++ b/epan/dissectors/packet-nfs.c
@@ -420,6 +420,7 @@ static int hf_nfs4_fattr_layout_blksize = -1;
 static int hf_nfs4_fattr_security_label_lfs = -1;
 static int hf_nfs4_fattr_security_label_pi = -1;
 static int hf_nfs4_fattr_security_label_context = -1;
+static int hf_nfs4_fattr_umask_mask = -1;
 static int hf_nfs4_who = -1;
 static int hf_nfs4_server = -1;
 static int hf_nfs4_fslocation = -1;
@@ -6134,6 +6135,8 @@ static const value_string fattr4_names[] = {
 	{	FATTR4_CHANGE_ATTR_TYPE,   "Change_Attr_Type"		},
 #define FATTR4_SECURITY_LABEL      80
 	{	FATTR4_SECURITY_LABEL,     "Security_Label"		},
+#define FATTR4_UMASK               81
+	{	FATTR4_UMASK,              "Umask"			},
 	{	0,	NULL	}
 };
 static value_string_ext fattr4_names_ext = VALUE_STRING_EXT_INIT(fattr4_names);
@@ -6718,6 +6721,14 @@ dissect_nfs4_security_label(tvbuff_t *tvb, proto_tree *tree, int offset)
 	return offset;
 }
 
+static int
+dissect_nfs4_umask(tvbuff_t *tvb, proto_tree *tree, int offset)
+{
+	offset = dissect_nfs4_mode(tvb, offset, tree);
+	offset = dissect_rpc_uint32(tvb, tree, hf_nfs4_fattr_umask_mask, offset);
+	return offset;
+}
+
 #define FATTR4_BITMAP_ONLY 0
 #define FATTR4_DISSECT_VALUES 1
 
@@ -7120,6 +7131,10 @@ dissect_nfs4_fattrs(tvbuff_t *tvb, int offset, packet_info *pinfo, proto_tree *t
 						offset = dissect_nfs4_security_label(tvb, attr_tree, offset);
 						break;
 
+					case FATTR4_UMASK:
+						offset = dissect_nfs4_umask(tvb, attr_tree, offset);
+						break;
+
 					default:
 						break;
 					}
@@ -12510,6 +12525,10 @@ proto_register_nfs(void)
 			"label_format", "nfs.fattr4.security_label.lfs", FT_UINT32, BASE_DEC,
 			NULL, 0, NULL, HFILL }},
 
+		{ &hf_nfs4_fattr_umask, {
+			"umask", "nfs.fattr4.umask", FT_UINT32, BASE_OCT,
+			NULL, 0, NULL, HFILL }},
+
 		{ &hf_nfs4_fattr_security_label_pi, {
 			"policy_id", "nfs.fattr4.security_label.pi", FT_UINT32, BASE_DEC,
 			NULL, 0, NULL, HFILL }},

Re: [PATCH 1/2] nfs: add support for the umask attribute

[Andreas Gruenbacher]

From: "J. Bruce Fields" <bfields@fieldses.org>

On Wed, Feb 1, 2017 at 10:31 PM, Olga Kornievskaia <aglo@umich.edu> wrote:
> Any plans to add wireshark support for this?

We did, yes.  Bruce had posted that together with the very first version.  I
couldn't find the wireshark patch for the current version of the proposal in
the mailing list archive, so here's that.

Andreas

--

NFSv4.2 umask support

---
 epan/dissectors/packet-nfs.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/epan/dissectors/packet-nfs.c b/epan/dissectors/packet-nfs.c
index 6d1dd3b..5f2ce42 100644
--- a/epan/dissectors/packet-nfs.c
+++ b/epan/dissectors/packet-nfs.c
@@ -420,6 +420,7 @@ static int hf_nfs4_fattr_layout_blksize = -1;
 static int hf_nfs4_fattr_security_label_lfs = -1;
 static int hf_nfs4_fattr_security_label_pi = -1;
 static int hf_nfs4_fattr_security_label_context = -1;
+static int hf_nfs4_fattr_umask_mask = -1;
 static int hf_nfs4_who = -1;
 static int hf_nfs4_server = -1;
 static int hf_nfs4_fslocation = -1;
@@ -6133,6 +6134,8 @@ static const value_string fattr4_names[] = {
 	{	FATTR4_CHANGE_ATTR_TYPE,   "Change_Attr_Type"		},
 #define FATTR4_SECURITY_LABEL      80
 	{	FATTR4_SECURITY_LABEL,     "Security_Label"		},
+#define FATTR4_MODE_UMASK          81
+	{	FATTR4_MODE_UMASK,         "Mode_Umask"			},
 	{	0,	NULL	}
 };
 static value_string_ext fattr4_names_ext = VALUE_STRING_EXT_INIT(fattr4_names);
@@ -6717,6 +6720,14 @@ dissect_nfs4_security_label(tvbuff_t *tvb, proto_tree *tree, int offset)
 	return offset;
 }
 
+static int
+dissect_nfs4_mode_umask(tvbuff_t *tvb, proto_tree *tree, int offset)
+{
+	offset = dissect_nfs4_mode(tvb, offset, tree);
+	offset = dissect_rpc_uint32(tvb, tree, hf_nfs4_fattr_umask_mask, offset);
+	return offset;
+}
+
 #define FATTR4_BITMAP_ONLY 0
 #define FATTR4_DISSECT_VALUES 1
 
@@ -7119,6 +7130,10 @@ dissect_nfs4_fattrs(tvbuff_t *tvb, int offset, packet_info *pinfo, proto_tree *t
 						offset = dissect_nfs4_security_label(tvb, attr_tree, offset);
 						break;
 
+					case FATTR4_MODE_UMASK:
+						offset = dissect_nfs4_mode_umask(tvb, attr_tree, offset);
+						break;
+
 					default:
 						break;
 					}
@@ -12509,6 +12524,10 @@ proto_register_nfs(void)
 			"label_format", "nfs.fattr4.security_label.lfs", FT_UINT32, BASE_DEC,
 			NULL, 0, NULL, HFILL }},
 
+		{ &hf_nfs4_fattr_umask_mask, {
+			"umask", "nfs.fattr4.umask", FT_UINT32, BASE_OCT,
+			NULL, 0, NULL, HFILL }},
+
 		{ &hf_nfs4_fattr_security_label_pi, {
 			"policy_id", "nfs.fattr4.security_label.pi", FT_UINT32, BASE_DEC,
 			NULL, 0, NULL, HFILL }},
-- 
2.7.4

Re: [PATCH 1/2] nfs: add support for the umask attribute

[Olga Kornievskaia]

On Wed, Feb 1, 2017 at 5:44 PM, Andreas Gruenbacher <agruenba@redhat.com> wrote:
> From: "J. Bruce Fields" <bfields@fieldses.org>
>
> On Wed, Feb 1, 2017 at 10:31 PM, Olga Kornievskaia <aglo@umich.edu> wrote:
>> Any plans to add wireshark support for this?
>
> We did, yes.  Bruce had posted that together with the very first version.  I
> couldn't find the wireshark patch for the current version of the proposal in
> the mailing list archive, so here's that.
>
> Andreas
>
> --
>
> NFSv4.2 umask support
>
> ---
>  epan/dissectors/packet-nfs.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
>
> diff --git a/epan/dissectors/packet-nfs.c b/epan/dissectors/packet-nfs.c
> index 6d1dd3b..5f2ce42 100644
> --- a/epan/dissectors/packet-nfs.c
> +++ b/epan/dissectors/packet-nfs.c
> @@ -420,6 +420,7 @@ static int hf_nfs4_fattr_layout_blksize = -1;
>  static int hf_nfs4_fattr_security_label_lfs = -1;
>  static int hf_nfs4_fattr_security_label_pi = -1;
>  static int hf_nfs4_fattr_security_label_context = -1;
> +static int hf_nfs4_fattr_umask_mask = -1;
>  static int hf_nfs4_who = -1;
>  static int hf_nfs4_server = -1;
>  static int hf_nfs4_fslocation = -1;
> @@ -6133,6 +6134,8 @@ static const value_string fattr4_names[] = {
>         {       FATTR4_CHANGE_ATTR_TYPE,   "Change_Attr_Type"           },
>  #define FATTR4_SECURITY_LABEL      80
>         {       FATTR4_SECURITY_LABEL,     "Security_Label"             },
> +#define FATTR4_MODE_UMASK          81
> +       {       FATTR4_MODE_UMASK,         "Mode_Umask"                 },
>         {       0,      NULL    }
>  };
>  static value_string_ext fattr4_names_ext = VALUE_STRING_EXT_INIT(fattr4_names);
> @@ -6717,6 +6720,14 @@ dissect_nfs4_security_label(tvbuff_t *tvb, proto_tree *tree, int offset)
>         return offset;
>  }
>
> +static int
> +dissect_nfs4_mode_umask(tvbuff_t *tvb, proto_tree *tree, int offset)
> +{
> +       offset = dissect_nfs4_mode(tvb, offset, tree);
> +       offset = dissect_rpc_uint32(tvb, tree, hf_nfs4_fattr_umask_mask, offset);
> +       return offset;
> +}
> +
>  #define FATTR4_BITMAP_ONLY 0
>  #define FATTR4_DISSECT_VALUES 1
>
> @@ -7119,6 +7130,10 @@ dissect_nfs4_fattrs(tvbuff_t *tvb, int offset, packet_info *pinfo, proto_tree *t
>                                                 offset = dissect_nfs4_security_label(tvb, attr_tree, offset);
>                                                 break;
>
> +                                       case FATTR4_MODE_UMASK:
> +                                               offset = dissect_nfs4_mode_umask(tvb, attr_tree, offset);
> +                                               break;
> +
>                                         default:
>                                                 break;
>                                         }
> @@ -12509,6 +12524,10 @@ proto_register_nfs(void)
>                         "label_format", "nfs.fattr4.security_label.lfs", FT_UINT32, BASE_DEC,
>                         NULL, 0, NULL, HFILL }},
>
> +               { &hf_nfs4_fattr_umask_mask, {
> +                       "umask", "nfs.fattr4.umask", FT_UINT32, BASE_OCT,
> +                       NULL, 0, NULL, HFILL }},
> +
>                 { &hf_nfs4_fattr_security_label_pi, {
>                         "policy_id", "nfs.fattr4.security_label.pi", FT_UINT32, BASE_DEC,
>                         NULL, 0, NULL, HFILL }},
> --
> 2.7.4


Thank you Andreas. I have tried this patch and it decodes the OPEN
compounds ok. Previously it's been garbage past the unknown attribute
with value 81.