* [PATCH] staging: lustre: include: lustre_update.h: Fix for possible null pointer dereference
@ 2015-01-29 18:47 Rickard Strandqvist
2015-01-29 19:40 ` [HPDD-discuss] " Frank Zago
0 siblings, 1 reply; 5+ messages in thread
From: Rickard Strandqvist @ 2015-01-29 18:47 UTC (permalink / raw)
To: Oleg Drokin, Andreas Dilger
Cc: Rickard Strandqvist, Greg Kroah-Hartman, HPDD-discuss, devel,
linux-kernel
Fix a possible null pointer dereference, there is
otherwise a risk of a possible null pointer dereference.
This was found using a static code analysis program called cppcheck
Signed-off-by: Rickard Strandqvist <rickard_strandqvist@spectrumdigital.se>
---
drivers/staging/lustre/lustre/include/lustre_update.h | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/lustre/lustre/include/lustre_update.h b/drivers/staging/lustre/lustre/include/lustre_update.h
index 84defce..00e1361 100644
--- a/drivers/staging/lustre/lustre/include/lustre_update.h
+++ b/drivers/staging/lustre/lustre/include/lustre_update.h
@@ -165,12 +165,14 @@ static inline int update_get_reply_buf(struct update_reply *reply, void **buf,
int result;
ptr = update_get_buf_internal(reply, index, &size);
+
+ LASSERT((ptr != NULL && size >= sizeof(int)));
+
result = *(int *)ptr;
if (result < 0)
return result;
- LASSERT((ptr != NULL && size >= sizeof(int)));
*buf = ptr + sizeof(int);
return size - sizeof(int);
}
--
1.7.10.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [HPDD-discuss] [PATCH] staging: lustre: include: lustre_update.h: Fix for possible null pointer dereference
2015-01-29 18:47 [PATCH] staging: lustre: include: lustre_update.h: Fix for possible null pointer dereference Rickard Strandqvist
@ 2015-01-29 19:40 ` Frank Zago
2015-01-29 19:47 ` Rickard Strandqvist
0 siblings, 1 reply; 5+ messages in thread
From: Frank Zago @ 2015-01-29 19:40 UTC (permalink / raw)
To: Rickard Strandqvist, Oleg Drokin, Andreas Dilger
Cc: HPDD-discuss, Greg Kroah-Hartman, devel, linux-kernel
On 01/29/2015 12:47 PM, Rickard Strandqvist wrote:
> Fix a possible null pointer dereference, there is
> otherwise a risk of a possible null pointer dereference.
>
> This was found using a static code analysis program called cppcheck
>
> Signed-off-by: Rickard Strandqvist <rickard_strandqvist@spectrumdigital.se>
> ---
> drivers/staging/lustre/lustre/include/lustre_update.h | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/staging/lustre/lustre/include/lustre_update.h b/drivers/staging/lustre/lustre/include/lustre_update.h
> index 84defce..00e1361 100644
> --- a/drivers/staging/lustre/lustre/include/lustre_update.h
> +++ b/drivers/staging/lustre/lustre/include/lustre_update.h
> @@ -165,12 +165,14 @@ static inline int update_get_reply_buf(struct update_reply *reply, void **buf,
> int result;
>
> ptr = update_get_buf_internal(reply, index, &size);
> +
> + LASSERT((ptr != NULL && size >= sizeof(int)));
Now size is tested before result. So it could assert if result < 0,
while the function would have returned before.
> +
> result = *(int *)ptr;
>
> if (result < 0)
> return result;
>
> - LASSERT((ptr != NULL && size >= sizeof(int)));
> *buf = ptr + sizeof(int);
> return size - sizeof(int);
> }
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [HPDD-discuss] [PATCH] staging: lustre: include: lustre_update.h: Fix for possible null pointer dereference
2015-01-29 19:40 ` [HPDD-discuss] " Frank Zago
@ 2015-01-29 19:47 ` Rickard Strandqvist
2015-01-29 19:49 ` Frank Zago
0 siblings, 1 reply; 5+ messages in thread
From: Rickard Strandqvist @ 2015-01-29 19:47 UTC (permalink / raw)
To: Frank Zago
Cc: Oleg Drokin, Andreas Dilger, HPDD-discuss@lists.01.org,
Greg Kroah-Hartman, devel, Linux Kernel Mailing List
2015-01-29 20:40 GMT+01:00 Frank Zago <fzago@cray.com>:
> On 01/29/2015 12:47 PM, Rickard Strandqvist wrote:
>>
>> Fix a possible null pointer dereference, there is
>> otherwise a risk of a possible null pointer dereference.
>>
>> This was found using a static code analysis program called cppcheck
>>
>> Signed-off-by: Rickard Strandqvist
>> <rickard_strandqvist@spectrumdigital.se>
>> ---
>> drivers/staging/lustre/lustre/include/lustre_update.h | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/staging/lustre/lustre/include/lustre_update.h
>> b/drivers/staging/lustre/lustre/include/lustre_update.h
>> index 84defce..00e1361 100644
>> --- a/drivers/staging/lustre/lustre/include/lustre_update.h
>> +++ b/drivers/staging/lustre/lustre/include/lustre_update.h
>> @@ -165,12 +165,14 @@ static inline int update_get_reply_buf(struct
>> update_reply *reply, void **buf,
>> int result;
>>
>> ptr = update_get_buf_internal(reply, index, &size);
>> +
>> + LASSERT((ptr != NULL && size >= sizeof(int)));
>
>
> Now size is tested before result. So it could assert if result < 0, while
> the function would have returned before.
>
>
>> +
>> result = *(int *)ptr;
>>
>> if (result < 0)
>> return result;
>>
>> - LASSERT((ptr != NULL && size >= sizeof(int)));
>> *buf = ptr + sizeof(int);
>> return size - sizeof(int);
>> }
>>
>
But if prt is null krachar on the line:
result = *(int *)ptr;
Maybe there should be two LASSERT then.
Kind regards
Rickard Strandqvist
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [HPDD-discuss] [PATCH] staging: lustre: include: lustre_update.h: Fix for possible null pointer dereference
2015-01-29 19:47 ` Rickard Strandqvist
@ 2015-01-29 19:49 ` Frank Zago
2015-01-29 20:26 ` Drokin, Oleg
0 siblings, 1 reply; 5+ messages in thread
From: Frank Zago @ 2015-01-29 19:49 UTC (permalink / raw)
To: Rickard Strandqvist
Cc: Oleg Drokin, Andreas Dilger, HPDD-discuss@lists.01.org,
Greg Kroah-Hartman, devel, Linux Kernel Mailing List
On 01/29/2015 01:47 PM, Rickard Strandqvist wrote:
> 2015-01-29 20:40 GMT+01:00 Frank Zago <fzago@cray.com>:
>> On 01/29/2015 12:47 PM, Rickard Strandqvist wrote:
>>>
>>> Fix a possible null pointer dereference, there is
>>> otherwise a risk of a possible null pointer dereference.
>>>
>>> This was found using a static code analysis program called cppcheck
>>>
>>> Signed-off-by: Rickard Strandqvist
>>> <rickard_strandqvist@spectrumdigital.se>
>>> ---
>>> drivers/staging/lustre/lustre/include/lustre_update.h | 4 +++-
>>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/staging/lustre/lustre/include/lustre_update.h
>>> b/drivers/staging/lustre/lustre/include/lustre_update.h
>>> index 84defce..00e1361 100644
>>> --- a/drivers/staging/lustre/lustre/include/lustre_update.h
>>> +++ b/drivers/staging/lustre/lustre/include/lustre_update.h
>>> @@ -165,12 +165,14 @@ static inline int update_get_reply_buf(struct
>>> update_reply *reply, void **buf,
>>> int result;
>>>
>>> ptr = update_get_buf_internal(reply, index, &size);
>>> +
>>> + LASSERT((ptr != NULL && size >= sizeof(int)));
>>
>>
>> Now size is tested before result. So it could assert if result < 0, while
>> the function would have returned before.
>>
>>
>>> +
>>> result = *(int *)ptr;
>>>
>>> if (result < 0)
>>> return result;
>>>
>>> - LASSERT((ptr != NULL && size >= sizeof(int)));
>>> *buf = ptr + sizeof(int);
>>> return size - sizeof(int);
>>> }
>>>
>>
>
>
>
> But if prt is null krachar on the line:
> result = *(int *)ptr;
>
> Maybe there should be two LASSERT then.
Yes, that would be safer.
Frank.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [HPDD-discuss] [PATCH] staging: lustre: include: lustre_update.h: Fix for possible null pointer dereference
2015-01-29 19:49 ` Frank Zago
@ 2015-01-29 20:26 ` Drokin, Oleg
0 siblings, 0 replies; 5+ messages in thread
From: Drokin, Oleg @ 2015-01-29 20:26 UTC (permalink / raw)
To: Frank Zago
Cc: Rickard Strandqvist, Dilger, Andreas, HPDD-discuss@lists.01.org,
Greg Kroah-Hartman, devel, Linux Kernel Mailing List
Hello!
On Jan 29, 2015, at 2:49 PM, Frank Zago wrote:
>>>> @@ -165,12 +165,14 @@ static inline int update_get_reply_buf(struct
>>>> update_reply *reply, void **buf,
>>>> int result;
>>>>
>>>> ptr = update_get_buf_internal(reply, index, &size);
>>>> +
>>>> + LASSERT((ptr != NULL && size >= sizeof(int)));
>>>
>>>
>>> Now size is tested before result. So it could assert if result < 0, while
>>> the function would have returned before.
>>
>> But if prt is null krachar on the line:
>> result = *(int *)ptr;
>>
>> Maybe there should be two LASSERT then.
>
>
> Yes, that would be safer.
Actually I just noticed this function does not appear to be used in the client code at all.
As such let's just remove update_get_reply_buf()?
In fat I bet this entire lustre_update.h contains server side updating code, and is unused anywhere in the client code,
so we might just be able to easily remove that.
I see the only includer is ./lustre/ptlrpc/layout.c that I don't think actually uses anything there?
Thanks.
Bye,
Oleg
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-01-29 20:26 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-29 18:47 [PATCH] staging: lustre: include: lustre_update.h: Fix for possible null pointer dereference Rickard Strandqvist
2015-01-29 19:40 ` [HPDD-discuss] " Frank Zago
2015-01-29 19:47 ` Rickard Strandqvist
2015-01-29 19:49 ` Frank Zago
2015-01-29 20:26 ` Drokin, Oleg
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.