* [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
* 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.