failed to set posix acl on NFSv3 mount

I found xfstests generic/053 generic/105 generic/307 and generic/319 started to fail on NFSv3 mount against Linux server running v4.12-rc1 kernel. They all try to set acl (xattr) on file/dir reside on NFSv3 mount. And this is a regression, 4.11 Linux server worked fine. A simple reproducer would be:

mount linux-server:/export /mnt/nfs
touch /mnt/nfs/testfile
setfattr -n user.test -v helloworld /mnt/nfs/testfile

The last setfattr returned EACCES

setfattr: /mnt/nfs/testfile: Operation not supported

And I bisected this to commit 51f56777

commit 51f567777799c9d85a778302b9eb61cf15214a98

Author: J. Bruce Fields <[email protected]>
Date:   Thu Apr 6 22:36:31 2017 -0400

nfsd: check for oversized NFSv2/v3 arguments

And I only need to revert this hunk to "fix" the regression:

include/linux/sunrpc/svc.h +1 -2

index e770abe..6ef19cf 100644
@@ -336,8 +336,7 @@ xdr_argsize_check(struct svc_rqst *rqstp, __be32 *p)
 {
        char *cp = (char *)p;
        struct kvec *vec = &rqstp->rq_arg.head[0];
-       return cp >= (char*)vec->iov_base
-               && cp <= (char*)vec->iov_base + vec->iov_len;
+       return cp == (char *)vec->iov_base + vec->iov_len;
 }
 
 static inline int

Debug log printing out cp, iov_base and iov_base + iov_len confirmed that cp can be greater than iov_base but smaller than iov_base + iov_len

[10868.819939] xdr_argsize_check: cp == ffff88020ea6f094, iov_base == ffff88020ea6f068, iov_base + iov_len == ffff88020ea6f104

Please help take a look. If you need more info please let me know.

Thanks,
Eryu

From: "J. Bruce Fields" <[email protected]>

This reverts commit 51f567777799 "nfsd: check for oversized NFSv2/v3 arguments", which breaks support for NFSv3 ACLs.

That patch was actually an earlier draft of a fix for the problem that was eventually fixed by e6838a29ecb "nfsd: check for oversized NFSv2/v3 arguments". But somehow I accidentally left this earlier draft in the branch that was part of my 2.12 pull request.

Reported-by Eryu Guan <[email protected]>

Cc [email protected]

Signed-off-by J. Bruce Fields <[email protected]>

---
 fs/nfsd/nfs3xdr.c          | 23 ++++++-----------------
 fs/nfsd/nfsxdr.c           | 13 +++----------
 include/linux/sunrpc/svc.h |  3 ++-
 3 files changed, 11 insertions(+), 28 deletions(-)

The stable cc may be overkill as I don't *think* the original's been backported to stable yet, but it's there just in case I miss NAK'ing the backports....

fs/nfsd/nfs3xdr.c +6 -17

index 12feac6ee2fd..452334694a5d 100644
@@ -334,11 +334,8 @@ nfs3svc_decode_readargs(struct svc_rqst *rqstp, __be32 *p,
 	if (!p)
 		return 0;
 	p = xdr_decode_hyper(p, &args->offset);
-	args->count = ntohl(*p++);
-
-	if (!xdr_argsize_check(rqstp, p))
-		return 0;
 
+	args->count = ntohl(*p++);
 	len = min(args->count, max_blocksize);
 
 	/* set up the kvec */

@@ -352,7 +349,7 @@ nfs3svc_decode_readargs(struct svc_rqst *rqstp, __be32 *p,
 		v++;
 	}
 	args->vlen = v;
-	return 1;
+	return xdr_argsize_check(rqstp, p);
 }
 
 int

@@ -544,11 +541,9 @@ nfs3svc_decode_readlinkargs(struct svc_rqst *rqstp, __be32 *p,
 	p = decode_fh(p, &args->fh);
 	if (!p)
 		return 0;
-	if (!xdr_argsize_check(rqstp, p))
-		return 0;
 	args->buffer = page_address(*(rqstp->rq_next_page++));
 
-	return 1;
+	return xdr_argsize_check(rqstp, p);
 }
 
 int

@@ -574,14 +569,10 @@ nfs3svc_decode_readdirargs(struct svc_rqst *rqstp, __be32 *p,
 	args->verf   = p; p += 2;
 	args->dircount = ~0;
 	args->count  = ntohl(*p++);
-
-	if (!xdr_argsize_check(rqstp, p))
-		return 0;
-
 	args->count  = min_t(u32, args->count, PAGE_SIZE);
 	args->buffer = page_address(*(rqstp->rq_next_page++));
 
-	return 1;
+	return xdr_argsize_check(rqstp, p);
 }
 
 int

@@ -599,9 +590,6 @@ nfs3svc_decode_readdirplusargs(struct svc_rqst *rqstp, __be32 *p,
 	args->dircount = ntohl(*p++);
 	args->count    = ntohl(*p++);
 
-	if (!xdr_argsize_check(rqstp, p))
-		return 0;
-
 	len = args->count = min(args->count, max_blocksize);
 	while (len > 0) {
 		struct page *p = *(rqstp->rq_next_page++);

@@ -609,7 +597,8 @@ nfs3svc_decode_readdirplusargs(struct svc_rqst *rqstp, __be32 *p,
 			args->buffer = page_address(p);
 		len -= PAGE_SIZE;
 	}
-	return 1;
+
+	return xdr_argsize_check(rqstp, p);
 }
 
 int

fs/nfsd/nfsxdr.c +3 -10

index 6a4947a3f4fa..de07ff625777 100644
@@ -257,9 +257,6 @@ nfssvc_decode_readargs(struct svc_rqst *rqstp, __be32 *p,
 	len = args->count     = ntohl(*p++);
 	p++; /* totalcount - unused */
 
-	if (!xdr_argsize_check(rqstp, p))
-		return 0;
-
 	len = min_t(unsigned int, len, NFSSVC_MAXBLKSIZE_V2);
 
 	/* set up somewhere to store response.

@@ -275,7 +272,7 @@ nfssvc_decode_readargs(struct svc_rqst *rqstp, __be32 *p,
 		v++;
 	}
 	args->vlen = v;
-	return 1;
+	return xdr_argsize_check(rqstp, p);
 }
 
 int

@@ -365,11 +362,9 @@ nfssvc_decode_readlinkargs(struct svc_rqst *rqstp, __be32 *p, struct nfsd_readli
 	p = decode_fh(p, &args->fh);
 	if (!p)
 		return 0;
-	if (!xdr_argsize_check(rqstp, p))
-		return 0;
 	args->buffer = page_address(*(rqstp->rq_next_page++));
 
-	return 1;
+	return xdr_argsize_check(rqstp, p);
 }
 
 int

@@ -407,11 +402,9 @@ nfssvc_decode_readdirargs(struct svc_rqst *rqstp, __be32 *p,
 	args->cookie = ntohl(*p++);
 	args->count  = ntohl(*p++);
 	args->count  = min_t(u32, args->count, PAGE_SIZE);
-	if (!xdr_argsize_check(rqstp, p))
-		return 0;
 	args->buffer = page_address(*(rqstp->rq_next_page++));
 
-	return 1;
+	return xdr_argsize_check(rqstp, p);
 }
 
 /*

include/linux/sunrpc/svc.h +2 -1

index 94631026f79c..11cef5a7bc87 100644
@@ -336,7 +336,8 @@ xdr_argsize_check(struct svc_rqst *rqstp, __be32 *p)
 {
 	char *cp = (char *)p;
 	struct kvec *vec = &rqstp->rq_arg.head[0];
-	return cp == (char *)vec->iov_base + vec->iov_len;
+	return cp >= (char*)vec->iov_base
+		&& cp <= (char*)vec->iov_base + vec->iov_len;
 }
 
 static inline int

Signature · 1 line

2.9.3