Re: Patch "nfsd: check for oversized NFSv2/v3 arguments" has been added to the 4.11-stable tree

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



NAK.--b.

On Tue, May 23, 2017 at 08:36:51PM +0200, gregkh@xxxxxxxxxxxxxxxxxxx wrote:
> 
> This is a note to let you know that I've just added the patch titled
> 
>     nfsd: check for oversized NFSv2/v3 arguments
> 
> to the 4.11-stable tree which can be found at:
>     http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
> 
> The filename of the patch is:
>      nfsd-check-for-oversized-nfsv2-v3-arguments.patch
> and it can be found in the queue-4.11 subdirectory.
> 
> If you, or anyone else, feels it should not be added to the stable tree,
> please let <stable@xxxxxxxxxxxxxxx> know about it.
> 
> 
> From 51f567777799c9d85a778302b9eb61cf15214a98 Mon Sep 17 00:00:00 2001
> From: "J. Bruce Fields" <bfields@xxxxxxxxxx>
> Date: Thu, 6 Apr 2017 22:36:31 -0400
> Subject: nfsd: check for oversized NFSv2/v3 arguments
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
> 
> From: J. Bruce Fields <bfields@xxxxxxxxxx>
> 
> commit 51f567777799c9d85a778302b9eb61cf15214a98 upstream.
> 
> A client can append random data to the end of an NFSv2 or NFSv3 RPC call
> without our complaining; we'll just stop parsing at the end of the
> expected data and ignore the rest.
> 
> Encoded arguments and replies are stored together in an array of pages,
> and if a call is too large it could leave inadequate space for the
> reply.  This is normally OK because NFS RPC's typically have either
> short arguments and long replies (like READ) or long arguments and short
> replies (like WRITE).  But a client that sends an incorrectly long reply
> can violate those assumptions.  This was observed to cause crashes.
> 
> So, insist that the argument not be any longer than we expect.
> 
> Also, several operations increment rq_next_page in the decode routine
> before checking the argument size, which can leave rq_next_page pointing
> well past the end of the page array, causing trouble later in
> svc_free_pages.
> 
> As followup we may also want to rewrite the encoding routines to check
> more carefully that they aren't running off the end of the page array.
> 
> Reported-by: Tuomas Haanpää <thaan@xxxxxxxxxxxx>
> Reported-by: Ari Kauppi <ari@xxxxxxxxxxxx>
> Signed-off-by: J. Bruce Fields <bfields@xxxxxxxxxx>
> Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> 
> ---
>  fs/nfsd/nfs3xdr.c          |   23 +++++++++++++++++------
>  fs/nfsd/nfsxdr.c           |   13 ++++++++++---
>  include/linux/sunrpc/svc.h |    3 +--
>  3 files changed, 28 insertions(+), 11 deletions(-)
> 
> --- a/fs/nfsd/nfs3xdr.c
> +++ b/fs/nfsd/nfs3xdr.c
> @@ -334,8 +334,11 @@ nfs3svc_decode_readargs(struct svc_rqst
>  	if (!p)
>  		return 0;
>  	p = xdr_decode_hyper(p, &args->offset);
> -
>  	args->count = ntohl(*p++);
> +
> +	if (!xdr_argsize_check(rqstp, p))
> +		return 0;
> +
>  	len = min(args->count, max_blocksize);
>  
>  	/* set up the kvec */
> @@ -349,7 +352,7 @@ nfs3svc_decode_readargs(struct svc_rqst
>  		v++;
>  	}
>  	args->vlen = v;
> -	return xdr_argsize_check(rqstp, p);
> +	return 1;
>  }
>  
>  int
> @@ -541,9 +544,11 @@ nfs3svc_decode_readlinkargs(struct svc_r
>  	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 xdr_argsize_check(rqstp, p);
> +	return 1;
>  }
>  
>  int
> @@ -569,10 +574,14 @@ nfs3svc_decode_readdirargs(struct svc_rq
>  	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 xdr_argsize_check(rqstp, p);
> +	return 1;
>  }
>  
>  int
> @@ -590,6 +599,9 @@ nfs3svc_decode_readdirplusargs(struct sv
>  	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++);
> @@ -597,8 +609,7 @@ nfs3svc_decode_readdirplusargs(struct sv
>  			args->buffer = page_address(p);
>  		len -= PAGE_SIZE;
>  	}
> -
> -	return xdr_argsize_check(rqstp, p);
> +	return 1;
>  }
>  
>  int
> --- a/fs/nfsd/nfsxdr.c
> +++ b/fs/nfsd/nfsxdr.c
> @@ -257,6 +257,9 @@ nfssvc_decode_readargs(struct svc_rqst *
>  	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.
> @@ -272,7 +275,7 @@ nfssvc_decode_readargs(struct svc_rqst *
>  		v++;
>  	}
>  	args->vlen = v;
> -	return xdr_argsize_check(rqstp, p);
> +	return 1;
>  }
>  
>  int
> @@ -362,9 +365,11 @@ nfssvc_decode_readlinkargs(struct svc_rq
>  	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 xdr_argsize_check(rqstp, p);
> +	return 1;
>  }
>  
>  int
> @@ -402,9 +407,11 @@ nfssvc_decode_readdirargs(struct svc_rqs
>  	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 xdr_argsize_check(rqstp, p);
> +	return 1;
>  }
>  
>  /*
> --- a/include/linux/sunrpc/svc.h
> +++ b/include/linux/sunrpc/svc.h
> @@ -336,8 +336,7 @@ xdr_argsize_check(struct svc_rqst *rqstp
>  {
>  	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
> 
> 
> Patches currently in stable-queue which might be from bfields@xxxxxxxxxx are
> 
> queue-4.11/nfsd-check-for-oversized-nfsv2-v3-arguments.patch
> queue-4.11/nfsd-encoders-mustn-t-use-unitialized-values-in-error-cases.patch
> queue-4.11/nfsd-fix-undefined-behavior-in-nfsd4_layout_verify.patch
> queue-4.11/nfsd-fix-up-the-supattr_exclcreat-attributes.patch



[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]