| Message ID | 1344606726-28754-1-git-send-email-simon.derr@bull.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Simon Derr <simon.derr@bull.net> writes: > Hi, > > While working on a modified server I had the Linux clients crash > a few times. This lead me to find this: > > Some error codes are directly extracted from the server replies. > A malformed server reply could contain an invalid error code, with a > very large value. If this value is then passed to ERR_PTR() it will > not be properly detected as an error code by IS_ERR() and as a result > the kernel will dereference an invalid pointer. > > This patch tries to avoid this. > > Simon > > Signed-off-by: Simon Derr <simon.derr@bull.net> > --- > net/9p/client.c | 18 ++++++++++++++++-- > 1 files changed, 16 insertions(+), 2 deletions(-) > > diff --git a/net/9p/client.c b/net/9p/client.c > index a170893..d066294 100644 > --- a/net/9p/client.c > +++ b/net/9p/client.c > @@ -76,6 +76,20 @@ inline int p9_is_proto_dotu(struct p9_client *clnt) > } > EXPORT_SYMBOL(p9_is_proto_dotu); > > +/* > + * Some error codes are taken directly from the server replies, > + * make sure they are valid. > + */ > +static int safe_errno(int err) > +{ > + if ((err > 0) || (err < -MAX_ERRNO)) { > + p9_debug(P9_DEBUG_ERROR, "Invalid error code %d\n", err); > + return -EINVAL; Is there any other error value we can use. EINVAL indicate that the client used invalid arguments. But that is not really the case here > + } > + return err; > +} > + > + > /* Interpret mount option for protocol version */ > static int get_protocol_version(char *s) > { > @@ -782,7 +796,7 @@ again: > return req; > reterr: > p9_free_req(c, req); > - return ERR_PTR(err); > + return ERR_PTR(safe_errno(err)); > } > > /** > @@ -865,7 +879,7 @@ static struct p9_req_t *p9_client_zc_rpc(struct p9_client *c, int8_t type, > return req; > reterr: > p9_free_req(c, req); > - return ERR_PTR(err); > + return ERR_PTR(safe_errno(err)); > } > > static struct p9_fid *p9_fid_create(struct p9_client *clnt) > -- > 1.7.1 -aneesh ------------------------------------------------------------------------------ Live Security Virtual Conference Exclusive live event will cover all the ways today's security and threat landscape has changed and how IT managers can respond. Discussions will include endpoint security, mobile security and the latest in malware threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
Agreed, if the primary case really is a malformed server reply, the
right thing might be EIO, but that tends to be fatal, which might be a
bit extreme, maybe EPROTO?
Sorry for being radio silent for the past few weeks, transitioning
jobs has been a bit time intensive. I'm processing through the patch
queue today, gonna go for EPROTO on this one for now until someone
comes up with something better.
-eric
On Mon, Sep 3, 2012 at 5:42 AM, Aneesh Kumar K.V
<aneesh.kumar@linux.vnet.ibm.com> wrote:
> Simon Derr <simon.derr@bull.net> writes:
>
>> Hi,
>>
>> While working on a modified server I had the Linux clients crash
>> a few times. This lead me to find this:
>>
>> Some error codes are directly extracted from the server replies.
>> A malformed server reply could contain an invalid error code, with a
>> very large value. If this value is then passed to ERR_PTR() it will
>> not be properly detected as an error code by IS_ERR() and as a result
>> the kernel will dereference an invalid pointer.
>>
>> This patch tries to avoid this.
>>
>> Simon
>>
>> Signed-off-by: Simon Derr <simon.derr@bull.net>
>> ---
>> net/9p/client.c | 18 ++++++++++++++++--
>> 1 files changed, 16 insertions(+), 2 deletions(-)
>>
>> diff --git a/net/9p/client.c b/net/9p/client.c
>> index a170893..d066294 100644
>> --- a/net/9p/client.c
>> +++ b/net/9p/client.c
>> @@ -76,6 +76,20 @@ inline int p9_is_proto_dotu(struct p9_client *clnt)
>> }
>> EXPORT_SYMBOL(p9_is_proto_dotu);
>>
>> +/*
>> + * Some error codes are taken directly from the server replies,
>> + * make sure they are valid.
>> + */
>> +static int safe_errno(int err)
>> +{
>> + if ((err > 0) || (err < -MAX_ERRNO)) {
>> + p9_debug(P9_DEBUG_ERROR, "Invalid error code %d\n", err);
>> + return -EINVAL;
>
> Is there any other error value we can use. EINVAL indicate that the
> client used invalid arguments. But that is not really the case here
>
>
>> + }
>> + return err;
>> +}
>> +
>> +
>> /* Interpret mount option for protocol version */
>> static int get_protocol_version(char *s)
>> {
>> @@ -782,7 +796,7 @@ again:
>> return req;
>> reterr:
>> p9_free_req(c, req);
>> - return ERR_PTR(err);
>> + return ERR_PTR(safe_errno(err));
>> }
>>
>> /**
>> @@ -865,7 +879,7 @@ static struct p9_req_t *p9_client_zc_rpc(struct p9_client *c, int8_t type,
>> return req;
>> reterr:
>> p9_free_req(c, req);
>> - return ERR_PTR(err);
>> + return ERR_PTR(safe_errno(err));
>> }
>>
>> static struct p9_fid *p9_fid_create(struct p9_client *clnt)
>> --
>> 1.7.1
>
> -aneesh
>
>
> ------------------------------------------------------------------------------
> Live Security Virtual Conference
> Exclusive live event will cover all the ways today's security and
> threat landscape has changed and how IT managers can respond. Discussions
> will include endpoint security, mobile security and the latest in malware
> threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
> _______________________________________________
> V9fs-developer mailing list
> V9fs-developer@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/v9fs-developer
------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and
threat landscape has changed and how IT managers can respond. Discussions
will include endpoint security, mobile security and the latest in malware
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
diff --git a/net/9p/client.c b/net/9p/client.c index a170893..d066294 100644 --- a/net/9p/client.c +++ b/net/9p/client.c @@ -76,6 +76,20 @@ inline int p9_is_proto_dotu(struct p9_client *clnt) } EXPORT_SYMBOL(p9_is_proto_dotu); +/* + * Some error codes are taken directly from the server replies, + * make sure they are valid. + */ +static int safe_errno(int err) +{ + if ((err > 0) || (err < -MAX_ERRNO)) { + p9_debug(P9_DEBUG_ERROR, "Invalid error code %d\n", err); + return -EINVAL; + } + return err; +} + + /* Interpret mount option for protocol version */ static int get_protocol_version(char *s) { @@ -782,7 +796,7 @@ again: return req; reterr: p9_free_req(c, req); - return ERR_PTR(err); + return ERR_PTR(safe_errno(err)); } /** @@ -865,7 +879,7 @@ static struct p9_req_t *p9_client_zc_rpc(struct p9_client *c, int8_t type, return req; reterr: p9_free_req(c, req); - return ERR_PTR(err); + return ERR_PTR(safe_errno(err)); } static struct p9_fid *p9_fid_create(struct p9_client *clnt)
Hi, While working on a modified server I had the Linux clients crash a few times. This lead me to find this: Some error codes are directly extracted from the server replies. A malformed server reply could contain an invalid error code, with a very large value. If this value is then passed to ERR_PTR() it will not be properly detected as an error code by IS_ERR() and as a result the kernel will dereference an invalid pointer. This patch tries to avoid this. Simon Signed-off-by: Simon Derr <simon.derr@bull.net> --- net/9p/client.c | 18 ++++++++++++++++-- 1 files changed, 16 insertions(+), 2 deletions(-)