diff mbox

nfsd: Revert "nfsd: check for oversized NFSv2/v3 arguments"

Message ID 20170516201944.GC28537@parsley.fieldses.org (mailing list archive)
State New, archived
Headers show

Commit Message

Bruce Fields May 16, 2017, 8:19 p.m. UTC 
From: "J. Bruce Fields" <bfields@redhat.com>

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 <eguan@redhat.com>
Cc: stable@vger.kernel.org
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 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....

Comments

Eryu Guan May 17, 2017, 12:55 p.m. UTC | #1 
On Tue, May 16, 2017 at 04:19:44PM -0400, J. Bruce Fields wrote:
> From: "J. Bruce Fields" <bfields@redhat.com>
> 
> 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 <eguan@redhat.com>
> Cc: stable@vger.kernel.org
> Signed-off-by: J. Bruce Fields <bfields@redhat.com>

Tested with generic/053 generic/105 generic/307 generic/319 and all
passed with NFSv3. Thanks!

Tested-by: Eryu Guan <eguan@redhat.com>

> ---
>  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....
> 
> diff --git a/fs/nfsd/nfs3xdr.c b/fs/nfsd/nfs3xdr.c
> index 12feac6ee2fd..452334694a5d 100644
> --- a/fs/nfsd/nfs3xdr.c
> +++ b/fs/nfsd/nfs3xdr.c
> @@ -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
> diff --git a/fs/nfsd/nfsxdr.c b/fs/nfsd/nfsxdr.c
> index 6a4947a3f4fa..de07ff625777 100644
> --- a/fs/nfsd/nfsxdr.c
> +++ b/fs/nfsd/nfsxdr.c
> @@ -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);
>  }
>  
>  /*
> diff --git a/include/linux/sunrpc/svc.h b/include/linux/sunrpc/svc.h
> index 94631026f79c..11cef5a7bc87 100644
> --- a/include/linux/sunrpc/svc.h
> +++ b/include/linux/sunrpc/svc.h
> @@ -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
> -- 
> 2.9.3
> 
--
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
diff mbox

Patch

diff --git a/fs/nfsd/nfs3xdr.c b/fs/nfsd/nfs3xdr.c
index 12feac6ee2fd..452334694a5d 100644
--- a/fs/nfsd/nfs3xdr.c
+++ b/fs/nfsd/nfs3xdr.c
@@ -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
diff --git a/fs/nfsd/nfsxdr.c b/fs/nfsd/nfsxdr.c
index 6a4947a3f4fa..de07ff625777 100644
--- a/fs/nfsd/nfsxdr.c
+++ b/fs/nfsd/nfsxdr.c
@@ -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);
 }
 
 /*
diff --git a/include/linux/sunrpc/svc.h b/include/linux/sunrpc/svc.h
index 94631026f79c..11cef5a7bc87 100644
--- a/include/linux/sunrpc/svc.h
+++ b/include/linux/sunrpc/svc.h
@@ -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