Skip to contentSkip to conversation

NFSD: some bugfixes and updates

PATCH In review Kinglong Mee 3 participants 8 messages Jul 30, 2015 – Aug 10, 2015

Subsystems (2)
NFS client NFS server
lore
Share & tools
lore

Kinglong Mee

author
#0 lore raw

cover NFSD: some bugfixes and updates

The first three are bugfixes, the last two are updates for EXCLUSIVE4_1.

Kinglong Mee (5):
NFSD: Store parent's stat in a sperate value
NFSD: Fix a encode bug of FS_LAYOUT_TYPES and LAYOUT_TYPES
NFSD: SUPPATTR_EXCLCREAT must be encoded before SECURITY_LABEL.
NFSD: Set the attributes used to store the verifier for EXCLUSIVE4_1
NFSD: Return word2 bitmask if setting security label in OPEN/CREATE

 fs/nfsd/nfs4proc.c |  12 ++--
 fs/nfsd/nfs4xdr.c  | 158 +++++++++++++++++++++++++++++++++--------------------
 fs/nfsd/vfs.c      |   6 --
 fs/nfsd/vfs.h      |   6 ++
 4 files changed, 112 insertions(+), 70 deletions(-)
Signature · 1 line
2.4.3
author
#1 lore raw

PATCH 1/5 NFSD: Store parent's stat in a sperate value

Commit ae7095a7c4 (nfsd4: helper function for getting mounted_on ino) don't process the return value from get_parent_attributes().

Also, the following FATTR4_WORD2_LAYOUT_BLKSIZE will using stat.blksize, using an independent value for parent's attributes.

Signed-off-by Kinglong Mee <[email protected]>
---
 fs/nfsd/nfs4xdr.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)
fs/nfsd/nfs4xdr.c +10 -3
index 5463385..f11003c 100644
@@ -2674,6 +2674,9 @@ out_acl:
 		*p++ = cpu_to_be32(stat.mtime.tv_nsec);
 	}
 	if (bmval1 & FATTR4_WORD1_MOUNTED_ON_FILEID) {
+		struct kstat parent_stat;
+		u64 ino = stat.ino;
+
 		p = xdr_reserve_space(xdr, 8);
 		if (!p)
                 	goto out_resource;

@@ -2682,9 +2685,13 @@ out_acl:
 		 * and this is the root of a cross-mounted filesystem.
 		 */
 		if (ignore_crossmnt == 0 &&
-		    dentry == exp->ex_path.mnt->mnt_root)
-			get_parent_attributes(exp, &stat);
-		p = xdr_encode_hyper(p, stat.ino);
+		    dentry == exp->ex_path.mnt->mnt_root) {
+			err = get_parent_attributes(exp, &parent_stat);
+			if (err)
+				goto out_nfserr;
+			ino = parent_stat.ino;
+		}
+		p = xdr_encode_hyper(p, ino);
 	}
 #ifdef CONFIG_NFSD_PNFS
 	if ((bmval1 & FATTR4_WORD1_FS_LAYOUT_TYPES) ||
Signature · 1 line
2.4.3
author
#2 lore raw

PATCH 2/5 nfsd: Fix a encode bug of FS_LAYOUT_TYPES and LAYOUT_TYPES

In reply to Kinglong Mee

Each bitmap needs one layout_type information,
if setting both, nfsd only return one right now.

Signed-off-by Kinglong Mee <[email protected]>
---
 fs/nfsd/nfs4xdr.c | 45 +++++++++++++++++++++++++++++++--------------
 1 file changed, 31 insertions(+), 14 deletions(-)
fs/nfsd/nfs4xdr.c +31 -14
index f11003c..76d6653 100644
@@ -2140,6 +2140,27 @@ nfsd4_encode_aclname(struct xdr_stream *xdr, struct svc_rqst *rqstp,
 		return nfsd4_encode_user(xdr, rqstp, ace->who_uid);
 }
 
+static inline __be32
+nfsd4_encode_layout_type(struct xdr_stream *xdr, enum pnfs_layouttype layout_type)
+{
+	__be32 *p;
+
+	if (layout_type) {
+		p = xdr_reserve_space(xdr, 8);
+		if (!p)
+			return nfserr_resource;
+		*p++ = cpu_to_be32(1);
+		*p++ = cpu_to_be32(layout_type);
+	} else {
+		p = xdr_reserve_space(xdr, 4);
+		if (!p)
+			return nfserr_resource;
+		*p++ = cpu_to_be32(0);
+	}
+
+	return 0;
+}
+
 #define WORD0_ABSENT_FS_ATTRS (FATTR4_WORD0_FS_LOCATIONS | FATTR4_WORD0_FSID | \
 			      FATTR4_WORD0_RDATTR_ERROR)
 #define WORD1_ABSENT_FS_ATTRS FATTR4_WORD1_MOUNTED_ON_FILEID

@@ -2694,20 +2715,16 @@ out_acl:
 		p = xdr_encode_hyper(p, ino);
 	}
 #ifdef CONFIG_NFSD_PNFS
-	if ((bmval1 & FATTR4_WORD1_FS_LAYOUT_TYPES) ||
-	    (bmval2 & FATTR4_WORD2_LAYOUT_TYPES)) {
-		if (exp->ex_layout_type) {
-			p = xdr_reserve_space(xdr, 8);
-			if (!p)
-				goto out_resource;
-			*p++ = cpu_to_be32(1);
-			*p++ = cpu_to_be32(exp->ex_layout_type);
-		} else {
-			p = xdr_reserve_space(xdr, 4);
-			if (!p)
-				goto out_resource;
-			*p++ = cpu_to_be32(0);
-		}
+	if (bmval1 & FATTR4_WORD1_FS_LAYOUT_TYPES) {
+		status = nfsd4_encode_layout_type(xdr, exp->ex_layout_type);
+		if (status)
+			goto out;
+	}
+
+	if (bmval2 & FATTR4_WORD2_LAYOUT_TYPES) {
+		status = nfsd4_encode_layout_type(xdr, exp->ex_layout_type);
+		if (status)
+			goto out;
 	}
 
 	if (bmval2 & FATTR4_WORD2_LAYOUT_BLKSIZE) {
Signature · 1 line
2.4.3
author
#4 lore raw

PATCH 3/5 nfsd: SUPPATTR_EXCLCREAT must be encoded before SECURITY_LABEL.

In reply to Kinglong Mee

The encode order should be as the bitmask defined order.

Signed-off-by Kinglong Mee <[email protected]>
---
 fs/nfsd/nfs4xdr.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)
fs/nfsd/nfs4xdr.c +7 -6
index 76d6653..21e1263 100644
@@ -2734,12 +2734,6 @@ out_acl:
 		*p++ = cpu_to_be32(stat.blksize);
 	}
 #endif /* CONFIG_NFSD_PNFS */
-	if (bmval2 & FATTR4_WORD2_SECURITY_LABEL) {
-		status = nfsd4_encode_security_label(xdr, rqstp, context,
-								contextlen);
-		if (status)
-			goto out;
-	}
 	if (bmval2 & FATTR4_WORD2_SUPPATTR_EXCLCREAT) {
 		p = xdr_reserve_space(xdr, 16);
 		if (!p)

@@ -2750,6 +2744,13 @@ out_acl:
 		*p++ = cpu_to_be32(NFSD_SUPPATTR_EXCLCREAT_WORD2);
 	}
 
+	if (bmval2 & FATTR4_WORD2_SECURITY_LABEL) {
+		status = nfsd4_encode_security_label(xdr, rqstp, context,
+								contextlen);
+		if (status)
+			goto out;
+	}
+
 	attrlen = htonl(xdr->buf->len - attrlen_offset - 4);
 	write_bytes_to_xdr_buf(xdr->buf, attrlen_offset, &attrlen, 4);
 	status = nfs_ok;
Signature · 1 line
2.4.3
author
#5 lore raw

PATCH 4/5 NFSD: Set the attributes used to store the verifier for EXCLUSIVE4_1

In reply to Kinglong Mee

According to rfc5661 18.16.4,
"If EXCLUSIVE4_1 was used, the client determines the attributes
used for the verifier by comparing attrset with cva_attrs.attrmask;"

So, EXCLUSIVE4_1 also needs those bitmask used to store the verifier.

Signed-off-by Kinglong Mee <[email protected]>
---
 fs/nfsd/nfs4proc.c | 12 ++++++------
 fs/nfsd/vfs.c      |  6 ------
 fs/nfsd/vfs.h      |  6 ++++++
 3 files changed, 12 insertions(+), 12 deletions(-)
fs/nfsd/nfs4proc.c +6 -6
index 90cfda7..91f05e8 100644
@@ -276,13 +276,13 @@ do_open_lookup(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate, stru
 			nfsd4_security_inode_setsecctx(*resfh, &open->op_label, open->op_bmval);
 
 		/*
-		 * Following rfc 3530 14.2.16, use the returned bitmask
-		 * to indicate which attributes we used to store the
-		 * verifier:
+		 * Following rfc 3530 14.2.16, and rfc 5661 18.16.4
+		 * use the returned bitmask to indicate which attributes
+		 * we used to store the verifier:
 		 */
-		if (open->op_createmode == NFS4_CREATE_EXCLUSIVE && status == 0)
-			open->op_bmval[1] = (FATTR4_WORD1_TIME_ACCESS |
-							FATTR4_WORD1_TIME_MODIFY);
+		if (nfsd_create_is_exclusive(open->op_createmode) && status == 0)
+			open->op_bmval[1] |= (FATTR4_WORD1_TIME_ACCESS |
+						FATTR4_WORD1_TIME_MODIFY);
 	} else
 		/*
 		 * Note this may exit with the parent still locked.
fs/nfsd/vfs.c +0 -6
index b5e077a..45c0497 100644
@@ -1249,12 +1249,6 @@ out_nfserr:
 
 #ifdef CONFIG_NFSD_V3
 
-static inline int nfsd_create_is_exclusive(int createmode)
-{
-	return createmode == NFS3_CREATE_EXCLUSIVE
-	       || createmode == NFS4_CREATE_EXCLUSIVE4_1;
-}
-
 /*
  * NFSv3 and NFSv4 version of nfsd_create
  */
fs/nfsd/vfs.h +6 -0
index 5be875e..fee2451 100644
@@ -131,4 +131,10 @@ static inline __be32 fh_getattr(struct svc_fh *fh, struct kstat *stat)
 	return nfserrno(vfs_getattr(&p, stat));
 }
 
+static inline int nfsd_create_is_exclusive(int createmode)
+{
+	return createmode == NFS3_CREATE_EXCLUSIVE
+	       || createmode == NFS4_CREATE_EXCLUSIVE4_1;
+}
+
 #endif /* LINUX_NFSD_VFS_H */
Signature · 1 line
2.4.3
author
#6 lore raw

PATCH 5/5 NFSD: Return word2 bitmask if setting security label in OPEN/CREATE

In reply to Kinglong Mee

Security label can be set in OPEN/CRATE request, nfsd should set the bitmask in word2 if setting success.

Signed-off-by Kinglong Mee <[email protected]>
---
 fs/nfsd/nfs4xdr.c | 95 ++++++++++++++++++++++++++++++++-----------------------
 1 file changed, 56 insertions(+), 39 deletions(-)
fs/nfsd/nfs4xdr.c +56 -39
index 21e1263..0ff1abe 100644
@@ -2224,6 +2224,39 @@ static int get_parent_attributes(struct svc_export *exp, struct kstat *stat)
 	return err;
 }
 
+static __be32
+nfsd4_encode_bitmap(struct xdr_stream *xdr, u32 bmval0, u32 bmval1, u32 bmval2)
+{
+	__be32 *p;
+
+	if (bmval2) {
+		p = xdr_reserve_space(xdr, 16);
+		if (!p)
+			goto out_resource;
+		*p++ = cpu_to_be32(3);
+		*p++ = cpu_to_be32(bmval0);
+		*p++ = cpu_to_be32(bmval1);
+		*p++ = cpu_to_be32(bmval2);
+	} else if (bmval1) {
+		p = xdr_reserve_space(xdr, 12);
+		if (!p)
+			goto out_resource;
+		*p++ = cpu_to_be32(2);
+		*p++ = cpu_to_be32(bmval0);
+		*p++ = cpu_to_be32(bmval1);
+	} else {
+		p = xdr_reserve_space(xdr, 8);
+		if (!p)
+			goto out_resource;
+		*p++ = cpu_to_be32(1);
+		*p++ = cpu_to_be32(bmval0);
+	}
+
+	return 0;
+out_resource:
+	return nfserr_resource;
+}
+
 /*
  * Note: @fhp can be NULL; in this case, we might have to compose the filehandle
  * ourselves.

@@ -2321,28 +2354,9 @@ nfsd4_encode_fattr(struct xdr_stream *xdr, struct svc_fh *fhp,
 	}
 #endif /* CONFIG_NFSD_V4_SECURITY_LABEL */
 
-	if (bmval2) {
-		p = xdr_reserve_space(xdr, 16);
-		if (!p)
-			goto out_resource;
-		*p++ = cpu_to_be32(3);
-		*p++ = cpu_to_be32(bmval0);
-		*p++ = cpu_to_be32(bmval1);
-		*p++ = cpu_to_be32(bmval2);
-	} else if (bmval1) {
-		p = xdr_reserve_space(xdr, 12);
-		if (!p)
-			goto out_resource;
-		*p++ = cpu_to_be32(2);
-		*p++ = cpu_to_be32(bmval0);
-		*p++ = cpu_to_be32(bmval1);
-	} else {
-		p = xdr_reserve_space(xdr, 8);
-		if (!p)
-			goto out_resource;
-		*p++ = cpu_to_be32(1);
-		*p++ = cpu_to_be32(bmval0);
-	}
+	status = nfsd4_encode_bitmap(xdr, bmval0, bmval1, bmval2);
+	if (status)
+		goto out;
 
 	attrlen_offset = xdr->buf->len;
 	p = xdr_reserve_space(xdr, 4);

@@ -2735,13 +2749,11 @@ out_acl:
 	}
 #endif /* CONFIG_NFSD_PNFS */
 	if (bmval2 & FATTR4_WORD2_SUPPATTR_EXCLCREAT) {
-		p = xdr_reserve_space(xdr, 16);
-		if (!p)
-			goto out_resource;
-		*p++ = cpu_to_be32(3);
-		*p++ = cpu_to_be32(NFSD_SUPPATTR_EXCLCREAT_WORD0);
-		*p++ = cpu_to_be32(NFSD_SUPPATTR_EXCLCREAT_WORD1);
-		*p++ = cpu_to_be32(NFSD_SUPPATTR_EXCLCREAT_WORD2);
+		status = nfsd4_encode_bitmap(xdr, NFSD_SUPPATTR_EXCLCREAT_WORD0,
+						  NFSD_SUPPATTR_EXCLCREAT_WORD1,
+						  NFSD_SUPPATTR_EXCLCREAT_WORD2);
+		if (status)
+			goto out;
 	}
 
 	if (bmval2 & FATTR4_WORD2_SECURITY_LABEL) {

@@ -3068,13 +3080,12 @@ nfsd4_encode_create(struct nfsd4_compoundres *resp, __be32 nfserr, struct nfsd4_
 	__be32 *p;
 
 	if (!nfserr) {
-		p = xdr_reserve_space(xdr, 32);
+		p = xdr_reserve_space(xdr, 20);
 		if (!p)
 			return nfserr_resource;
-		p = encode_cinfo(p, &create->cr_cinfo);
-		*p++ = cpu_to_be32(2);
-		*p++ = cpu_to_be32(create->cr_bmval[0]);
-		*p++ = cpu_to_be32(create->cr_bmval[1]);
+		encode_cinfo(p, &create->cr_cinfo);
+		nfserr = nfsd4_encode_bitmap(xdr, create->cr_bmval[0],
+				create->cr_bmval[1], create->cr_bmval[2]);
 	}
 	return nfserr;
 }

@@ -3214,16 +3225,22 @@ nfsd4_encode_open(struct nfsd4_compoundres *resp, __be32 nfserr, struct nfsd4_op
 	nfserr = nfsd4_encode_stateid(xdr, &open->op_stateid);
 	if (nfserr)
 		goto out;
-	p = xdr_reserve_space(xdr, 40);
+	p = xdr_reserve_space(xdr, 24);
 	if (!p)
 		return nfserr_resource;
 	p = encode_cinfo(p, &open->op_cinfo);
 	*p++ = cpu_to_be32(open->op_rflags);
-	*p++ = cpu_to_be32(2);
-	*p++ = cpu_to_be32(open->op_bmval[0]);
-	*p++ = cpu_to_be32(open->op_bmval[1]);
-	*p++ = cpu_to_be32(open->op_delegate_type);
 
+	nfserr = nfsd4_encode_bitmap(xdr, open->op_bmval[0], open->op_bmval[1],
+					open->op_bmval[2]);
+	if (nfserr)
+		goto out;
+
+	p = xdr_reserve_space(xdr, 4);
+	if (!p)
+		return nfserr_resource;
+
+	*p++ = cpu_to_be32(open->op_delegate_type);
 	switch (open->op_delegate_type) {
 	case NFS4_OPEN_DELEGATE_NONE:
 		break;
Signature · 1 line
2.4.3
#7 lore raw
In reply to Kinglong Mee
Quoted 1 line · Kinglong Mee

The first three are bugfixes, the last two are updates for EXCLUSIVE4_1.

These all look good to me, thanks, applying.--b.

Quoted 16 lines

Kinglong Mee (5):
NFSD: Store parent's stat in a sperate value
NFSD: Fix a encode bug of FS_LAYOUT_TYPES and LAYOUT_TYPES
NFSD: SUPPATTR_EXCLCREAT must be encoded before SECURITY_LABEL.
NFSD: Set the attributes used to store the verifier for EXCLUSIVE4_1
NFSD: Return word2 bitmask if setting security label in OPEN/CREATE

 fs/nfsd/nfs4proc.c |  12 ++--
 fs/nfsd/nfs4xdr.c  | 158 +++++++++++++++++++++++++++++++++--------------------
 fs/nfsd/vfs.c      |   6 --
 fs/nfsd/vfs.h      |   6 ++
 4 files changed, 112 insertions(+), 70 deletions(-)
Signature · 1 line
2.4.3