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