read_buf may return NULL. return NFS4ERR_RESOURCE in this case.
Signed-off-by: Benny Halevy <[email protected]>
---
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
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 <[email protected]> wro=
te:
> read_buf may return NULL. return NFS4ERR_RESOURCE in this case.
>
> Signed-off-by: Benny Halevy <[email protected]>
> ---
> =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 [email protected]
> More majordomo info at =A0http://vger.kernel.org/majordomo-info.html
>
On Jun. 29, 2010, 15:22 +0300, "William A. (Andy) Adamson" <[email protected]> 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 <[email protected]> wrote:
>> read_buf may return NULL. return NFS4ERR_RESOURCE in this case.
>>
>> Signed-off-by: Benny Halevy <[email protected]>
>> ---
>> 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 [email protected]
>> 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
On Jun 29, 2010, at 9:00 AM, Benny Halevy wrote:
> On Jun. 29, 2010, 15:22 +0300, "William A. (Andy) Adamson" <[email protected]
> > 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 <[email protected]>
>> wrote:
>>> read_buf may return NULL. return NFS4ERR_RESOURCE in this case.
>>>
>>> Signed-off-by: Benny Halevy <[email protected]>
>>> ---
>>> 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 [email protected]
>>> 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 [email protected]
>> 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
On Jun. 29, 2010, 16:19 +0300, Andy Adamson <[email protected]> wrote:
>
> On Jun 29, 2010, at 9:00 AM, Benny Halevy wrote:
>
>> On Jun. 29, 2010, 15:22 +0300, "William A. (Andy) Adamson" <[email protected]
>>> 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 <[email protected]>
>>> wrote:
>>>> read_buf may return NULL. return NFS4ERR_RESOURCE in this case.
>>>>
>>>> Signed-off-by: Benny Halevy <[email protected]>
>>>> ---
>>>> 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 [email protected]
>>>> 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 [email protected]
>>> 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 [email protected]
>> 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html