* [PATCH] SQUASHME: pnfs: check for read_buf error in decode_pnfs_layoutrecall_args @ 2010-06-29 10:55 Benny Halevy 2010-06-29 12:22 ` William A. (Andy) Adamson 0 siblings, 1 reply; 5+ messages in thread From: Benny Halevy @ 2010-06-29 10:55 UTC (permalink / raw) To: linux-nfs read_buf may return NULL. return NFS4ERR_RESOURCE in this case. Signed-off-by: Benny Halevy <bhalevy@panasas.com> --- fs/nfs/callback_xdr.c | 8 ++++++++ 1 files changed, 8 insertions(+), 0 deletions(-) diff --git a/fs/nfs/callback_xdr.c b/fs/nfs/callback_xdr.c index 7e34bb3..2f69f0d 100644 --- a/fs/nfs/callback_xdr.c +++ b/fs/nfs/callback_xdr.c @@ -247,6 +247,10 @@ static __be32 decode_pnfs_layoutrecall_args(struct svc_rqst *rqstp, goto out; p = read_buf(xdr, 2 * sizeof(uint64_t)); + if (unlikely(p == NULL)) { + status = htonl(NFS4ERR_RESOURCE); + goto out; + } p = xdr_decode_hyper(p, &args->cbl_seg.offset); p = xdr_decode_hyper(p, &args->cbl_seg.length); status = decode_stateid(xdr, &args->cbl_stateid); @@ -254,6 +258,10 @@ static __be32 decode_pnfs_layoutrecall_args(struct svc_rqst *rqstp, goto out; } else if (args->cbl_recall_type == RETURN_FSID) { p = read_buf(xdr, 2 * sizeof(uint64_t)); + if (unlikely(p == NULL)) { + status = htonl(NFS4ERR_RESOURCE); + goto out; + } p = xdr_decode_hyper(p, &args->cbl_fsid.major); p = xdr_decode_hyper(p, &args->cbl_fsid.minor); } -- 1.6.6.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] SQUASHME: pnfs: check for read_buf error in decode_pnfs_layoutrecall_args 2010-06-29 10:55 [PATCH] SQUASHME: pnfs: check for read_buf error in decode_pnfs_layoutrecall_args Benny Halevy @ 2010-06-29 12:22 ` William A. (Andy) Adamson [not found] ` <AANLkTiku06tjr1NCt4kJMJOlDne11zhRHKkJMc67-DvR-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 0 siblings, 1 reply; 5+ messages in thread From: William A. (Andy) Adamson @ 2010-06-29 12:22 UTC (permalink / raw) To: Benny Halevy; +Cc: linux-nfs I see that NFS4ERR_RESOURCE is returned throughout callback_xdr.c ,but it is not a legal error return for NFSv4.1. -ENOMEM would be better. -->Andy On Tue, Jun 29, 2010 at 6:55 AM, Benny Halevy <bhalevy@panasas.com> wro= te: > read_buf may return NULL. return NFS4ERR_RESOURCE in this case. > > Signed-off-by: Benny Halevy <bhalevy@panasas.com> > --- > =A0fs/nfs/callback_xdr.c | =A0 =A08 ++++++++ > =A01 files changed, 8 insertions(+), 0 deletions(-) > > diff --git a/fs/nfs/callback_xdr.c b/fs/nfs/callback_xdr.c > index 7e34bb3..2f69f0d 100644 > --- a/fs/nfs/callback_xdr.c > +++ b/fs/nfs/callback_xdr.c > @@ -247,6 +247,10 @@ static __be32 decode_pnfs_layoutrecall_args(stru= ct svc_rqst *rqstp, > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0goto out; > > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0p =3D read_buf(xdr, 2 * sizeof(uint64_= t)); > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (unlikely(p =3D=3D NULL)) { > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 status =3D htonl(NFS4ER= R_RESOURCE); > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 goto out; > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 } > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0p =3D xdr_decode_hyper(p, &args->cbl_s= eg.offset); > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0p =3D xdr_decode_hyper(p, &args->cbl_s= eg.length); > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0status =3D decode_stateid(xdr, &args->= cbl_stateid); > @@ -254,6 +258,10 @@ static __be32 decode_pnfs_layoutrecall_args(stru= ct svc_rqst *rqstp, > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0goto out; > =A0 =A0 =A0 =A0} else if (args->cbl_recall_type =3D=3D RETURN_FSID) { > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0p =3D read_buf(xdr, 2 * sizeof(uint64_= t)); > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (unlikely(p =3D=3D NULL)) { > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 status =3D htonl(NFS4ER= R_RESOURCE); > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 goto out; > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 } > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0p =3D xdr_decode_hyper(p, &args->cbl_f= sid.major); > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0p =3D xdr_decode_hyper(p, &args->cbl_f= sid.minor); > =A0 =A0 =A0 =A0} > -- > 1.6.6.1 > > -- > 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 =A0http://vger.kernel.org/majordomo-info.html > ^ permalink raw reply [flat|nested] 5+ messages in thread
[parent not found: <AANLkTiku06tjr1NCt4kJMJOlDne11zhRHKkJMc67-DvR-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH] SQUASHME: pnfs: check for read_buf error in decode_pnfs_layoutrecall_args [not found] ` <AANLkTiku06tjr1NCt4kJMJOlDne11zhRHKkJMc67-DvR-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2010-06-29 13:00 ` Benny Halevy 2010-06-29 13:19 ` Andy Adamson 0 siblings, 1 reply; 5+ messages in thread From: Benny Halevy @ 2010-06-29 13:00 UTC (permalink / raw) To: William A. (Andy) Adamson; +Cc: linux-nfs On Jun. 29, 2010, 15:22 +0300, "William A. (Andy) Adamson" <androsadamson@gmail.com> wrote: > I see that NFS4ERR_RESOURCE is returned throughout callback_xdr.c ,but > it is not a legal error return for NFSv4.1. -ENOMEM would be better. Sigh... it is indeed. You mean NFS4ERR_DELAY? Benny > > -->Andy > > On Tue, Jun 29, 2010 at 6:55 AM, Benny Halevy <bhalevy@panasas.com> wrote: >> read_buf may return NULL. return NFS4ERR_RESOURCE in this case. >> >> Signed-off-by: Benny Halevy <bhalevy@panasas.com> >> --- >> fs/nfs/callback_xdr.c | 8 ++++++++ >> 1 files changed, 8 insertions(+), 0 deletions(-) >> >> diff --git a/fs/nfs/callback_xdr.c b/fs/nfs/callback_xdr.c >> index 7e34bb3..2f69f0d 100644 >> --- a/fs/nfs/callback_xdr.c >> +++ b/fs/nfs/callback_xdr.c >> @@ -247,6 +247,10 @@ static __be32 decode_pnfs_layoutrecall_args(struct svc_rqst *rqstp, >> goto out; >> >> p = read_buf(xdr, 2 * sizeof(uint64_t)); >> + if (unlikely(p == NULL)) { >> + status = htonl(NFS4ERR_RESOURCE); >> + goto out; >> + } >> p = xdr_decode_hyper(p, &args->cbl_seg.offset); >> p = xdr_decode_hyper(p, &args->cbl_seg.length); >> status = decode_stateid(xdr, &args->cbl_stateid); >> @@ -254,6 +258,10 @@ static __be32 decode_pnfs_layoutrecall_args(struct svc_rqst *rqstp, >> goto out; >> } else if (args->cbl_recall_type == RETURN_FSID) { >> p = read_buf(xdr, 2 * sizeof(uint64_t)); >> + if (unlikely(p == NULL)) { >> + status = htonl(NFS4ERR_RESOURCE); >> + goto out; >> + } >> p = xdr_decode_hyper(p, &args->cbl_fsid.major); >> p = xdr_decode_hyper(p, &args->cbl_fsid.minor); >> } >> -- >> 1.6.6.1 >> >> -- >> 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 >> > -- > 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] SQUASHME: pnfs: check for read_buf error in decode_pnfs_layoutrecall_args 2010-06-29 13:00 ` Benny Halevy @ 2010-06-29 13:19 ` Andy Adamson 2010-06-29 13:57 ` Benny Halevy 0 siblings, 1 reply; 5+ messages in thread From: Andy Adamson @ 2010-06-29 13:19 UTC (permalink / raw) To: Benny Halevy; +Cc: William A. (Andy) Adamson, linux-nfs On Jun 29, 2010, at 9:00 AM, Benny Halevy wrote: > On Jun. 29, 2010, 15:22 +0300, "William A. (Andy) Adamson" <androsadamson@gmail.com > > wrote: >> I see that NFS4ERR_RESOURCE is returned throughout >> callback_xdr.c ,but >> it is not a legal error return for NFSv4.1. -ENOMEM would be better. > > Sigh... it is indeed. > > You mean NFS4ERR_DELAY? We code the client to have enough buffer space e.g. use the maximum possible value for all the xdr fields. So if a request overflows this buffer, I say it's NFS4ERR_BADXDR. (or NFS4ERR_BAD_CLIENT_CODE!!) In any event, NFS4ERR_DELAY will not solve the problem as the client will not increase the buffer space, and (most likely) the server will not decrease what it sends. -->Andy > > Benny > >> >> -->Andy >> >> On Tue, Jun 29, 2010 at 6:55 AM, Benny Halevy <bhalevy@panasas.com> >> wrote: >>> read_buf may return NULL. return NFS4ERR_RESOURCE in this case. >>> >>> Signed-off-by: Benny Halevy <bhalevy@panasas.com> >>> --- >>> fs/nfs/callback_xdr.c | 8 ++++++++ >>> 1 files changed, 8 insertions(+), 0 deletions(-) >>> >>> diff --git a/fs/nfs/callback_xdr.c b/fs/nfs/callback_xdr.c >>> index 7e34bb3..2f69f0d 100644 >>> --- a/fs/nfs/callback_xdr.c >>> +++ b/fs/nfs/callback_xdr.c >>> @@ -247,6 +247,10 @@ static __be32 >>> decode_pnfs_layoutrecall_args(struct svc_rqst *rqstp, >>> goto out; >>> >>> p = read_buf(xdr, 2 * sizeof(uint64_t)); >>> + if (unlikely(p == NULL)) { >>> + status = htonl(NFS4ERR_RESOURCE); >>> + goto out; >>> + } >>> p = xdr_decode_hyper(p, &args->cbl_seg.offset); >>> p = xdr_decode_hyper(p, &args->cbl_seg.length); >>> status = decode_stateid(xdr, &args->cbl_stateid); >>> @@ -254,6 +258,10 @@ static __be32 >>> decode_pnfs_layoutrecall_args(struct svc_rqst *rqstp, >>> goto out; >>> } else if (args->cbl_recall_type == RETURN_FSID) { >>> p = read_buf(xdr, 2 * sizeof(uint64_t)); >>> + if (unlikely(p == NULL)) { >>> + status = htonl(NFS4ERR_RESOURCE); >>> + goto out; >>> + } >>> p = xdr_decode_hyper(p, &args->cbl_fsid.major); >>> p = xdr_decode_hyper(p, &args->cbl_fsid.minor); >>> } >>> -- >>> 1.6.6.1 >>> >>> -- >>> 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 >>> >> -- >> 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 > > -- > 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] SQUASHME: pnfs: check for read_buf error in decode_pnfs_layoutrecall_args 2010-06-29 13:19 ` Andy Adamson @ 2010-06-29 13:57 ` Benny Halevy 0 siblings, 0 replies; 5+ messages in thread From: Benny Halevy @ 2010-06-29 13:57 UTC (permalink / raw) To: Andy Adamson; +Cc: William A. (Andy) Adamson, linux-nfs On Jun. 29, 2010, 16:19 +0300, Andy Adamson <andros@netapp.com> wrote: > > On Jun 29, 2010, at 9:00 AM, Benny Halevy wrote: > >> On Jun. 29, 2010, 15:22 +0300, "William A. (Andy) Adamson" <androsadamson@gmail.com >>> wrote: >>> I see that NFS4ERR_RESOURCE is returned throughout >>> callback_xdr.c ,but >>> it is not a legal error return for NFSv4.1. -ENOMEM would be better. >> >> Sigh... it is indeed. >> >> You mean NFS4ERR_DELAY? > > We code the client to have enough buffer space e.g. use the maximum > possible value for all the xdr fields. So if a request overflows this > buffer, I say it's NFS4ERR_BADXDR. (or NFS4ERR_BAD_CLIENT_CODE!!) In this case BADXDR seems wrong as the "allocation" is static, independent of the actual xdr code, so failure to allocate indicates more a bug on the (callback RPC) server size, hence: NFS4ERR_SERVERFAULT would be more appropriate. > > In any event, NFS4ERR_DELAY will not solve the problem as the client > will not increase the buffer space, and (most likely) the server will > not decrease what it sends. That's true. > > -->Andy > >> >> Benny >> >>> >>> -->Andy >>> >>> On Tue, Jun 29, 2010 at 6:55 AM, Benny Halevy <bhalevy@panasas.com> >>> wrote: >>>> read_buf may return NULL. return NFS4ERR_RESOURCE in this case. >>>> >>>> Signed-off-by: Benny Halevy <bhalevy@panasas.com> >>>> --- >>>> fs/nfs/callback_xdr.c | 8 ++++++++ >>>> 1 files changed, 8 insertions(+), 0 deletions(-) >>>> >>>> diff --git a/fs/nfs/callback_xdr.c b/fs/nfs/callback_xdr.c >>>> index 7e34bb3..2f69f0d 100644 >>>> --- a/fs/nfs/callback_xdr.c >>>> +++ b/fs/nfs/callback_xdr.c >>>> @@ -247,6 +247,10 @@ static __be32 >>>> decode_pnfs_layoutrecall_args(struct svc_rqst *rqstp, >>>> goto out; >>>> >>>> p = read_buf(xdr, 2 * sizeof(uint64_t)); >>>> + if (unlikely(p == NULL)) { >>>> + status = htonl(NFS4ERR_RESOURCE); >>>> + goto out; >>>> + } >>>> p = xdr_decode_hyper(p, &args->cbl_seg.offset); >>>> p = xdr_decode_hyper(p, &args->cbl_seg.length); >>>> status = decode_stateid(xdr, &args->cbl_stateid); >>>> @@ -254,6 +258,10 @@ static __be32 >>>> decode_pnfs_layoutrecall_args(struct svc_rqst *rqstp, >>>> goto out; >>>> } else if (args->cbl_recall_type == RETURN_FSID) { >>>> p = read_buf(xdr, 2 * sizeof(uint64_t)); >>>> + if (unlikely(p == NULL)) { >>>> + status = htonl(NFS4ERR_RESOURCE); >>>> + goto out; >>>> + } >>>> p = xdr_decode_hyper(p, &args->cbl_fsid.major); >>>> p = xdr_decode_hyper(p, &args->cbl_fsid.minor); >>>> } >>>> -- >>>> 1.6.6.1 >>>> >>>> -- >>>> 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 >>>> >>> -- >>> 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 >> >> -- >> 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 > > -- > 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2010-06-29 13:57 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-06-29 10:55 [PATCH] SQUASHME: pnfs: check for read_buf error in decode_pnfs_layoutrecall_args Benny Halevy
2010-06-29 12:22 ` William A. (Andy) Adamson
[not found] ` <AANLkTiku06tjr1NCt4kJMJOlDne11zhRHKkJMc67-DvR-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2010-06-29 13:00 ` Benny Halevy
2010-06-29 13:19 ` Andy Adamson
2010-06-29 13:57 ` Benny Halevy
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.