| Message ID | 20190404065711.19763-1-jencce.kernel@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] nfsd/nfsd3_proc_readdir: fix buffer count and page pointers | expand |
On Thu, Apr 04 2019, Murphy Zhou wrote: > After this commit > f875a79 nfsd: allow nfsv3 readdir request to be larger. > nfsv3 readdir request size can be larger than PAGE_SIZE. So if the > directory been read is large enough, we can use multiple pages > in rq_respages. Update buffer count and page pointers like we do > in readdirplus to make this happen. > > Now listing a directory within 3000 files will panic because we > are counting in a wrong way and would write on random page. > > Fixes: f875a79 "nfsd: allow nfsv3 readdir request to be larger" > Signed-off-by: Murphy Zhou <jencce.kernel@gmail.com> > --- > > v2: > fix nfs3svc_decode_readdirargs to set page pointers like decode_readdirplusargs > do not test pointers in encode_entry as we've fixed them when decoding > > fs/nfsd/nfs3proc.c | 17 +++++++++++++++-- > fs/nfsd/nfs3xdr.c | 11 +++++++++-- > 2 files changed, 24 insertions(+), 4 deletions(-) > > diff --git a/fs/nfsd/nfs3proc.c b/fs/nfsd/nfs3proc.c > index 8f933e8..9bc32af 100644 > --- a/fs/nfsd/nfs3proc.c > +++ b/fs/nfsd/nfs3proc.c > @@ -442,7 +442,9 @@ > struct nfsd3_readdirargs *argp = rqstp->rq_argp; > struct nfsd3_readdirres *resp = rqstp->rq_resp; > __be32 nfserr; > - int count; > + int count = 0; > + struct page **p; > + caddr_t page_addr = NULL; > > dprintk("nfsd: READDIR(3) %s %d bytes at %d\n", > SVCFH_fmt(&argp->fh), > @@ -462,7 +464,18 @@ > nfserr = nfsd_readdir(rqstp, &resp->fh, (loff_t*) &argp->cookie, > &resp->common, nfs3svc_encode_entry); > memcpy(resp->verf, argp->verf, 8); > - resp->count = resp->buffer - argp->buffer; > + count = 0; Thanks - looks good. Setting 'count' to zero a second time looks a bit clumsy, but that is a minor detail. Reviewed-by: NeilBrown <neilb@suse.com> Thanks, NeilBrown > + for (p = rqstp->rq_respages + 1; p < rqstp->rq_next_page; p++) { > + page_addr = page_address(*p); > + > + if (((caddr_t)resp->buffer >= page_addr) && > + ((caddr_t)resp->buffer < page_addr + PAGE_SIZE)) { > + count += (caddr_t)resp->buffer - page_addr; > + break; > + } > + count += PAGE_SIZE; > + } > + resp->count = count >> 2; > if (resp->offset) { > loff_t offset = argp->cookie; > > diff --git a/fs/nfsd/nfs3xdr.c b/fs/nfsd/nfs3xdr.c > index 93fea24..8d78912 100644 > --- a/fs/nfsd/nfs3xdr.c > +++ b/fs/nfsd/nfs3xdr.c > @@ -573,6 +573,7 @@ void fill_post_wcc(struct svc_fh *fhp) > nfs3svc_decode_readdirargs(struct svc_rqst *rqstp, __be32 *p) > { > struct nfsd3_readdirargs *args = rqstp->rq_argp; > + int len; > u32 max_blocksize = svc_max_payload(rqstp); > > p = decode_fh(p, &args->fh); > @@ -582,8 +583,14 @@ void fill_post_wcc(struct svc_fh *fhp) > args->verf = p; p += 2; > args->dircount = ~0; > args->count = ntohl(*p++); > - args->count = min_t(u32, args->count, max_blocksize); > - args->buffer = page_address(*(rqstp->rq_next_page++)); > + len = args->count = min_t(u32, args->count, max_blocksize); > + > + while (len > 0) { > + struct page *p = *(rqstp->rq_next_page++); > + if (!args->buffer) > + args->buffer = page_address(p); > + len -= PAGE_SIZE; > + } > > return xdr_argsize_check(rqstp, p); > } > -- > 1.8.3.1
missed nfs list. resend. On Fri, Apr 5, 2019 at 2:10 PM Murphy Zhou <jencce.kernel@gmail.com> wrote: > > > > On Fri, Apr 5, 2019 at 14:08 Murphy Zhou <jencce.kernel@gmail.com> wrote: >> >> >> >> On Fri, Apr 5, 2019 at 06:47 NeilBrown <neilb@suse.com> wrote: >>> >>> On Thu, Apr 04 2019, Murphy Zhou wrote: >>> >>> > After this commit >>> > f875a79 nfsd: allow nfsv3 readdir request to be larger. >>> > nfsv3 readdir request size can be larger than PAGE_SIZE. So if the >>> > directory been read is large enough, we can use multiple pages >>> > in rq_respages. Update buffer count and page pointers like we do >>> > in readdirplus to make this happen. >>> > >>> > Now listing a directory within 3000 files will panic because we >>> > are counting in a wrong way and would write on random page. >>> > >>> > Fixes: f875a79 "nfsd: allow nfsv3 readdir request to be larger" >>> > Signed-off-by: Murphy Zhou <jencce.kernel@gmail.com> >>> > --- >>> > >>> > v2: >>> > fix nfs3svc_decode_readdirargs to set page pointers like decode_readdirplusargs >>> > do not test pointers in encode_entry as we've fixed them when decoding >>> > >>> > fs/nfsd/nfs3proc.c | 17 +++++++++++++++-- >>> > fs/nfsd/nfs3xdr.c | 11 +++++++++-- >>> > 2 files changed, 24 insertions(+), 4 deletions(-) >>> > >>> > diff --git a/fs/nfsd/nfs3proc.c b/fs/nfsd/nfs3proc.c >>> > index 8f933e8..9bc32af 100644 >>> > --- a/fs/nfsd/nfs3proc.c >>> > +++ b/fs/nfsd/nfs3proc.c >>> > @@ -442,7 +442,9 @@ >>> > struct nfsd3_readdirargs *argp = rqstp->rq_argp; >>> > struct nfsd3_readdirres *resp = rqstp->rq_resp; >>> > __be32 nfserr; >>> > - int count; >>> > + int count = 0; >>> > + struct page **p; >>> > + caddr_t page_addr = NULL; >>> > >>> > dprintk("nfsd: READDIR(3) %s %d bytes at %d\n", >>> > SVCFH_fmt(&argp->fh), >>> > @@ -462,7 +464,18 @@ >>> > nfserr = nfsd_readdir(rqstp, &resp->fh, (loff_t*) &argp->cookie, >>> > &resp->common, nfs3svc_encode_entry); >>> > memcpy(resp->verf, argp->verf, 8); >>> > - resp->count = resp->buffer - argp->buffer; >>> > + count = 0; >>> >>> Thanks - looks good. >>> Setting 'count' to zero a second time looks a bit clumsy, but that is a >>> minor detail. > > > Save some space in the stack :) > > Thanks for your review! > > M >>> >>> >>> >>> Reviewed-by: NeilBrown <neilb@suse.com> >>> >>> Thanks, >>> NeilBrown >>> >>> >>> > + for (p = rqstp->rq_respages + 1; p < rqstp->rq_next_page; p++) { >>> > + page_addr = page_address(*p); >>> > + >>> > + if (((caddr_t)resp->buffer >= page_addr) && >>> > + ((caddr_t)resp->buffer < page_addr + PAGE_SIZE)) { >>> > + count += (caddr_t)resp->buffer - page_addr; >>> > + break; >>> > + } >>> > + count += PAGE_SIZE; >>> > + } >>> > + resp->count = count >> 2; >>> > if (resp->offset) { >>> > loff_t offset = argp->cookie; >>> > >>> > diff --git a/fs/nfsd/nfs3xdr.c b/fs/nfsd/nfs3xdr.c >>> > index 93fea24..8d78912 100644 >>> > --- a/fs/nfsd/nfs3xdr.c >>> > +++ b/fs/nfsd/nfs3xdr.c >>> > @@ -573,6 +573,7 @@ void fill_post_wcc(struct svc_fh *fhp) >>> > nfs3svc_decode_readdirargs(struct svc_rqst *rqstp, __be32 *p) >>> > { >>> > struct nfsd3_readdirargs *args = rqstp->rq_argp; >>> > + int len; >>> > u32 max_blocksize = svc_max_payload(rqstp); >>> > >>> > p = decode_fh(p, &args->fh); >>> > @@ -582,8 +583,14 @@ void fill_post_wcc(struct svc_fh *fhp) >>> > args->verf = p; p += 2; >>> > args->dircount = ~0; >>> > args->count = ntohl(*p++); >>> > - args->count = min_t(u32, args->count, max_blocksize); >>> > - args->buffer = page_address(*(rqstp->rq_next_page++)); >>> > + len = args->count = min_t(u32, args->count, max_blocksize); >>> > + >>> > + while (len > 0) { >>> > + struct page *p = *(rqstp->rq_next_page++); >>> > + if (!args->buffer) >>> > + args->buffer = page_address(p); >>> > + len -= PAGE_SIZE; >>> > + } >>> > >>> > return xdr_argsize_check(rqstp, p); >>> > } >>> > -- >>> > 1.8.3.1
On Fri, Apr 05, 2019 at 02:10:13PM +0800, Murphy Zhou wrote: > On Fri, Apr 5, 2019 at 14:08 Murphy Zhou <jencce.kernel@gmail.com> wrote: > > > > > > > On Fri, Apr 5, 2019 at 06:47 NeilBrown <neilb@suse.com> wrote: > > > >> On Thu, Apr 04 2019, Murphy Zhou wrote: > >> > >> > After this commit > >> > f875a79 nfsd: allow nfsv3 readdir request to be larger. > >> > nfsv3 readdir request size can be larger than PAGE_SIZE. So if the > >> > directory been read is large enough, we can use multiple pages > >> > in rq_respages. Update buffer count and page pointers like we do > >> > in readdirplus to make this happen. > >> > > >> > Now listing a directory within 3000 files will panic because we > >> > are counting in a wrong way and would write on random page. > >> > > >> > Fixes: f875a79 "nfsd: allow nfsv3 readdir request to be larger" > >> > Signed-off-by: Murphy Zhou <jencce.kernel@gmail.com> > >> > --- > >> > > >> > v2: > >> > fix nfs3svc_decode_readdirargs to set page pointers like > >> decode_readdirplusargs > >> > do not test pointers in encode_entry as we've fixed them when decoding > >> > > >> > fs/nfsd/nfs3proc.c | 17 +++++++++++++++-- > >> > fs/nfsd/nfs3xdr.c | 11 +++++++++-- > >> > 2 files changed, 24 insertions(+), 4 deletions(-) > >> > > >> > diff --git a/fs/nfsd/nfs3proc.c b/fs/nfsd/nfs3proc.c > >> > index 8f933e8..9bc32af 100644 > >> > --- a/fs/nfsd/nfs3proc.c > >> > +++ b/fs/nfsd/nfs3proc.c > >> > @@ -442,7 +442,9 @@ > >> > struct nfsd3_readdirargs *argp = rqstp->rq_argp; > >> > struct nfsd3_readdirres *resp = rqstp->rq_resp; > >> > __be32 nfserr; > >> > - int count; > >> > + int count = 0; > >> > + struct page **p; > >> > + caddr_t page_addr = NULL; > >> > > >> > dprintk("nfsd: READDIR(3) %s %d bytes at %d\n", > >> > SVCFH_fmt(&argp->fh), > >> > @@ -462,7 +464,18 @@ > >> > nfserr = nfsd_readdir(rqstp, &resp->fh, (loff_t*) &argp->cookie, > >> > &resp->common, > >> nfs3svc_encode_entry); > >> > memcpy(resp->verf, argp->verf, 8); > >> > - resp->count = resp->buffer - argp->buffer; > >> > + count = 0; > >> > >> Thanks - looks good. > >> Setting 'count' to zero a second time looks a bit clumsy, but that is a > >> minor detail. > > > > > Save some space in the stack :) We're not *that* desperate for stack space. But, I can live with it. Queuing up for 5.1. --b. > > Thanks for your review! > > M > > > > >> > >> Reviewed-by: NeilBrown <neilb@suse.com> > >> > >> Thanks, > >> NeilBrown > >> > >> > >> > + for (p = rqstp->rq_respages + 1; p < rqstp->rq_next_page; p++) { > >> > + page_addr = page_address(*p); > >> > + > >> > + if (((caddr_t)resp->buffer >= page_addr) && > >> > + ((caddr_t)resp->buffer < page_addr + PAGE_SIZE)) { > >> > + count += (caddr_t)resp->buffer - page_addr; > >> > + break; > >> > + } > >> > + count += PAGE_SIZE; > >> > + } > >> > + resp->count = count >> 2; > >> > if (resp->offset) { > >> > loff_t offset = argp->cookie; > >> > > >> > diff --git a/fs/nfsd/nfs3xdr.c b/fs/nfsd/nfs3xdr.c > >> > index 93fea24..8d78912 100644 > >> > --- a/fs/nfsd/nfs3xdr.c > >> > +++ b/fs/nfsd/nfs3xdr.c > >> > @@ -573,6 +573,7 @@ void fill_post_wcc(struct svc_fh *fhp) > >> > nfs3svc_decode_readdirargs(struct svc_rqst *rqstp, __be32 *p) > >> > { > >> > struct nfsd3_readdirargs *args = rqstp->rq_argp; > >> > + int len; > >> > u32 max_blocksize = svc_max_payload(rqstp); > >> > > >> > p = decode_fh(p, &args->fh); > >> > @@ -582,8 +583,14 @@ void fill_post_wcc(struct svc_fh *fhp) > >> > args->verf = p; p += 2; > >> > args->dircount = ~0; > >> > args->count = ntohl(*p++); > >> > - args->count = min_t(u32, args->count, max_blocksize); > >> > - args->buffer = page_address(*(rqstp->rq_next_page++)); > >> > + len = args->count = min_t(u32, args->count, max_blocksize); > >> > + > >> > + while (len > 0) { > >> > + struct page *p = *(rqstp->rq_next_page++); > >> > + if (!args->buffer) > >> > + args->buffer = page_address(p); > >> > + len -= PAGE_SIZE; > >> > + } > >> > > >> > return xdr_argsize_check(rqstp, p); > >> > } > >> > -- > >> > 1.8.3.1 > >> > >
diff --git a/fs/nfsd/nfs3proc.c b/fs/nfsd/nfs3proc.c index 8f933e8..9bc32af 100644 --- a/fs/nfsd/nfs3proc.c +++ b/fs/nfsd/nfs3proc.c @@ -442,7 +442,9 @@ struct nfsd3_readdirargs *argp = rqstp->rq_argp; struct nfsd3_readdirres *resp = rqstp->rq_resp; __be32 nfserr; - int count; + int count = 0; + struct page **p; + caddr_t page_addr = NULL; dprintk("nfsd: READDIR(3) %s %d bytes at %d\n", SVCFH_fmt(&argp->fh), @@ -462,7 +464,18 @@ nfserr = nfsd_readdir(rqstp, &resp->fh, (loff_t*) &argp->cookie, &resp->common, nfs3svc_encode_entry); memcpy(resp->verf, argp->verf, 8); - resp->count = resp->buffer - argp->buffer; + count = 0; + for (p = rqstp->rq_respages + 1; p < rqstp->rq_next_page; p++) { + page_addr = page_address(*p); + + if (((caddr_t)resp->buffer >= page_addr) && + ((caddr_t)resp->buffer < page_addr + PAGE_SIZE)) { + count += (caddr_t)resp->buffer - page_addr; + break; + } + count += PAGE_SIZE; + } + resp->count = count >> 2; if (resp->offset) { loff_t offset = argp->cookie; diff --git a/fs/nfsd/nfs3xdr.c b/fs/nfsd/nfs3xdr.c index 93fea24..8d78912 100644 --- a/fs/nfsd/nfs3xdr.c +++ b/fs/nfsd/nfs3xdr.c @@ -573,6 +573,7 @@ void fill_post_wcc(struct svc_fh *fhp) nfs3svc_decode_readdirargs(struct svc_rqst *rqstp, __be32 *p) { struct nfsd3_readdirargs *args = rqstp->rq_argp; + int len; u32 max_blocksize = svc_max_payload(rqstp); p = decode_fh(p, &args->fh); @@ -582,8 +583,14 @@ void fill_post_wcc(struct svc_fh *fhp) args->verf = p; p += 2; args->dircount = ~0; args->count = ntohl(*p++); - args->count = min_t(u32, args->count, max_blocksize); - args->buffer = page_address(*(rqstp->rq_next_page++)); + len = args->count = min_t(u32, args->count, max_blocksize); + + while (len > 0) { + struct page *p = *(rqstp->rq_next_page++); + if (!args->buffer) + args->buffer = page_address(p); + len -= PAGE_SIZE; + } return xdr_argsize_check(rqstp, p); }
After this commit f875a79 nfsd: allow nfsv3 readdir request to be larger. nfsv3 readdir request size can be larger than PAGE_SIZE. So if the directory been read is large enough, we can use multiple pages in rq_respages. Update buffer count and page pointers like we do in readdirplus to make this happen. Now listing a directory within 3000 files will panic because we are counting in a wrong way and would write on random page. Fixes: f875a79 "nfsd: allow nfsv3 readdir request to be larger" Signed-off-by: Murphy Zhou <jencce.kernel@gmail.com> --- v2: fix nfs3svc_decode_readdirargs to set page pointers like decode_readdirplusargs do not test pointers in encode_entry as we've fixed them when decoding fs/nfsd/nfs3proc.c | 17 +++++++++++++++-- fs/nfsd/nfs3xdr.c | 11 +++++++++-- 2 files changed, 24 insertions(+), 4 deletions(-)