* [PATCH v2] nfsd/nfsd3_proc_readdir: fix buffer count and page pointers
@ 2019-04-04 6:57 Murphy Zhou
2019-04-04 22:47 ` NeilBrown
0 siblings, 1 reply; 4+ messages in thread
From: Murphy Zhou @ 2019-04-04 6:57 UTC (permalink / raw)
To: linux-nfs, neilb; +Cc: bfields, jlayton, Murphy Zhou
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;
+ 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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2] nfsd/nfsd3_proc_readdir: fix buffer count and page pointers
2019-04-04 6:57 [PATCH v2] nfsd/nfsd3_proc_readdir: fix buffer count and page pointers Murphy Zhou
@ 2019-04-04 22:47 ` NeilBrown
[not found] ` <CADJHv_t_Cqav7Wr8dHo0kBBX3-ws0qVDcYi4HZt26Dq8jO+DGQ@mail.gmail.com>
0 siblings, 1 reply; 4+ messages in thread
From: NeilBrown @ 2019-04-04 22:47 UTC (permalink / raw)
To: Murphy Zhou, linux-nfs; +Cc: bfields, jlayton, Murphy Zhou
[-- Attachment #1: Type: text/plain, Size: 3294 bytes --]
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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] nfsd/nfsd3_proc_readdir: fix buffer count and page pointers
[not found] ` <CADJHv_u-F+D-p-uhJzxKTwQijZHe8wHfcyC97tjAgOO_ToJYHw@mail.gmail.com>
@ 2019-04-05 6:44 ` Murphy Zhou
2019-04-05 23:56 ` J. Bruce Fields
1 sibling, 0 replies; 4+ messages in thread
From: Murphy Zhou @ 2019-04-05 6:44 UTC (permalink / raw)
To: Murphy Zhou, linux-nfs
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] nfsd/nfsd3_proc_readdir: fix buffer count and page pointers
[not found] ` <CADJHv_u-F+D-p-uhJzxKTwQijZHe8wHfcyC97tjAgOO_ToJYHw@mail.gmail.com>
2019-04-05 6:44 ` Murphy Zhou
@ 2019-04-05 23:56 ` J. Bruce Fields
1 sibling, 0 replies; 4+ messages in thread
From: J. Bruce Fields @ 2019-04-05 23:56 UTC (permalink / raw)
To: Murphy Zhou; +Cc: NeilBrown, jlayton, linux-nfs
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
> >>
> >
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-04-05 23:56 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-04 6:57 [PATCH v2] nfsd/nfsd3_proc_readdir: fix buffer count and page pointers Murphy Zhou
2019-04-04 22:47 ` NeilBrown
[not found] ` <CADJHv_t_Cqav7Wr8dHo0kBBX3-ws0qVDcYi4HZt26Dq8jO+DGQ@mail.gmail.com>
[not found] ` <CADJHv_u-F+D-p-uhJzxKTwQijZHe8wHfcyC97tjAgOO_ToJYHw@mail.gmail.com>
2019-04-05 6:44 ` Murphy Zhou
2019-04-05 23:56 ` J. Bruce Fields
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).