* Re: + kvm-ia64-dereference-of-null-pointer-in-set_pal_result.patch
@ 2010-01-13 9:22 Avi Kivity
2010-01-13 9:32 ` Andrew Morton
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Avi Kivity @ 2010-01-13 9:22 UTC (permalink / raw)
To: kvm-ia64
On 01/13/2010 12:11 AM, akpm@linux-foundation.org wrote:
> Subject: kvm/ia64: dereference of NULL pointer in set_pal_result()
> From: Roel Kluin<roel.kluin@gmail.com>
>
> Do not dereference a NULL pointer
>
> diff -puN arch/ia64/kvm/kvm_fw.c~kvm-ia64-dereference-of-null-pointer-in-set_pal_result arch/ia64/kvm/kvm_fw.c
> --- a/arch/ia64/kvm/kvm_fw.c~kvm-ia64-dereference-of-null-pointer-in-set_pal_result
> +++ a/arch/ia64/kvm/kvm_fw.c
> @@ -75,9 +75,11 @@ static void set_pal_result(struct kvm_vc
> struct exit_ctl_data *p;
>
> p = kvm_get_exit_data(vcpu);
> - if (p&& p->exit_reason = EXIT_REASON_PAL_CALL) {
> + if (!p)
> + return;
> + if (p->exit_reason = EXIT_REASON_PAL_CALL) {
> p->u.pal_data.ret = result;
> - return ;
> + return;
> }
> INIT_PAL_STATUS_UNIMPLEMENTED(p->u.pal_data.ret);
> }
>
kvm_get_exit_data() cannot return a NULL pointer.
Where did this come from?
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: + kvm-ia64-dereference-of-null-pointer-in-set_pal_result.patch
2010-01-13 9:22 + kvm-ia64-dereference-of-null-pointer-in-set_pal_result.patch Avi Kivity
@ 2010-01-13 9:32 ` Andrew Morton
2010-01-13 9:37 ` Avi Kivity
2010-01-13 9:59 ` Zhang, Xiantao
2 siblings, 0 replies; 4+ messages in thread
From: Andrew Morton @ 2010-01-13 9:32 UTC (permalink / raw)
To: kvm-ia64
On Wed, 13 Jan 2010 11:22:39 +0200 Avi Kivity <avi@redhat.com> wrote:
> On 01/13/2010 12:11 AM, akpm@linux-foundation.org wrote:
> > Subject: kvm/ia64: dereference of NULL pointer in set_pal_result()
> > From: Roel Kluin<roel.kluin@gmail.com>
> >
> > Do not dereference a NULL pointer
> >
> > diff -puN arch/ia64/kvm/kvm_fw.c~kvm-ia64-dereference-of-null-pointer-in-set_pal_result arch/ia64/kvm/kvm_fw.c
> > --- a/arch/ia64/kvm/kvm_fw.c~kvm-ia64-dereference-of-null-pointer-in-set_pal_result
> > +++ a/arch/ia64/kvm/kvm_fw.c
> > @@ -75,9 +75,11 @@ static void set_pal_result(struct kvm_vc
> > struct exit_ctl_data *p;
> >
> > p = kvm_get_exit_data(vcpu);
> > - if (p&& p->exit_reason = EXIT_REASON_PAL_CALL) {
> > + if (!p)
> > + return;
> > + if (p->exit_reason = EXIT_REASON_PAL_CALL) {
> > p->u.pal_data.ret = result;
> > - return ;
> > + return;
> > }
> > INIT_PAL_STATUS_UNIMPLEMENTED(p->u.pal_data.ret);
> > }
> >
>
>
> kvm_get_exit_data() cannot return a NULL pointer.
In that case set_pal_result() doesn't need to test for that.
Roel looks for code along the lines of
if (p)
...
*p;
> Where did this come from?
I got it off linux-kernel.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: + kvm-ia64-dereference-of-null-pointer-in-set_pal_result.patch
2010-01-13 9:22 + kvm-ia64-dereference-of-null-pointer-in-set_pal_result.patch Avi Kivity
2010-01-13 9:32 ` Andrew Morton
@ 2010-01-13 9:37 ` Avi Kivity
2010-01-13 9:59 ` Zhang, Xiantao
2 siblings, 0 replies; 4+ messages in thread
From: Avi Kivity @ 2010-01-13 9:37 UTC (permalink / raw)
To: kvm-ia64
On 01/13/2010 11:32 AM, Andrew Morton wrote:
> On Wed, 13 Jan 2010 11:22:39 +0200 Avi Kivity<avi@redhat.com> wrote:
>
>
>> On 01/13/2010 12:11 AM, akpm@linux-foundation.org wrote:
>>
>>> Subject: kvm/ia64: dereference of NULL pointer in set_pal_result()
>>> From: Roel Kluin<roel.kluin@gmail.com>
>>>
>>> Do not dereference a NULL pointer
>>>
>>> diff -puN arch/ia64/kvm/kvm_fw.c~kvm-ia64-dereference-of-null-pointer-in-set_pal_result arch/ia64/kvm/kvm_fw.c
>>> --- a/arch/ia64/kvm/kvm_fw.c~kvm-ia64-dereference-of-null-pointer-in-set_pal_result
>>> +++ a/arch/ia64/kvm/kvm_fw.c
>>> @@ -75,9 +75,11 @@ static void set_pal_result(struct kvm_vc
>>> struct exit_ctl_data *p;
>>>
>>> p = kvm_get_exit_data(vcpu);
>>> - if (p&& p->exit_reason = EXIT_REASON_PAL_CALL) {
>>> + if (!p)
>>> + return;
>>> + if (p->exit_reason = EXIT_REASON_PAL_CALL) {
>>> p->u.pal_data.ret = result;
>>> - return ;
>>> + return;
>>> }
>>> INIT_PAL_STATUS_UNIMPLEMENTED(p->u.pal_data.ret);
>>> }
>>>
>>>
>>
>> kvm_get_exit_data() cannot return a NULL pointer.
>>
> In that case set_pal_result() doesn't need to test for that.
>
> Roel looks for code along the lines of
>
> if (p)
> ...
>
> *p;
>
I see. I think it's worthwhile to look deeper rather than converting
things mechanically. After all, the code may have actually worked
before the patch.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: + kvm-ia64-dereference-of-null-pointer-in-set_pal_result.patch
2010-01-13 9:22 + kvm-ia64-dereference-of-null-pointer-in-set_pal_result.patch Avi Kivity
2010-01-13 9:32 ` Andrew Morton
2010-01-13 9:37 ` Avi Kivity
@ 2010-01-13 9:59 ` Zhang, Xiantao
2 siblings, 0 replies; 4+ messages in thread
From: Zhang, Xiantao @ 2010-01-13 9:59 UTC (permalink / raw)
To: kvm-ia64
Avi Kivity wrote:
> On 01/13/2010 11:32 AM, Andrew Morton wrote:
>> On Wed, 13 Jan 2010 11:22:39 +0200 Avi Kivity<avi@redhat.com> wrote:
>>
>>
>>> On 01/13/2010 12:11 AM, akpm@linux-foundation.org wrote:
>>>
>>>> Subject: kvm/ia64: dereference of NULL pointer in set_pal_result()
>>>> From: Roel Kluin<roel.kluin@gmail.com>
>>>>
>>>> Do not dereference a NULL pointer
>>>>
>>>> diff -puN
>>>> arch/ia64/kvm/kvm_fw.c~kvm-ia64-dereference-of-null-pointer-in-set_pal_result
>>>> arch/ia64/kvm/kvm_fw.c ---
>>>> a/arch/ia64/kvm/kvm_fw.c~kvm-ia64-dereference-of-null-pointer-in-set_pal_result
>>>> +++ a/arch/ia64/kvm/kvm_fw.c @@ -75,9 +75,11 @@ static void
>>>> set_pal_result(struct kvm_vc struct exit_ctl_data *p;
>>>>
>>>> p = kvm_get_exit_data(vcpu);
>>>> - if (p&& p->exit_reason = EXIT_REASON_PAL_CALL) { + if (!p)
>>>> + return;
>>>> + if (p->exit_reason = EXIT_REASON_PAL_CALL) {
>>>> p->u.pal_data.ret = result;
>>>> - return ;
>>>> + return;
>>>> }
>>>> INIT_PAL_STATUS_UNIMPLEMENTED(p->u.pal_data.ret); }
>>>>
>>>>
>>>
>>> kvm_get_exit_data() cannot return a NULL pointer.
>>>
>> In that case set_pal_result() doesn't need to test for that.
>>
>> Roel looks for code along the lines of
>>
>> if (p)
>> ...
>>
>> *p;
>>
>
> I see. I think it's worthwhile to look deeper rather than converting
> things mechanically. After all, the code may have actually worked
> before the patch.
Originally, we want to dynamically allocate the exit_data, so needs the check. But now, it is allocated statically, so the check is unnecessary.
Xiantao
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2010-01-13 9:59 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-01-13 9:22 + kvm-ia64-dereference-of-null-pointer-in-set_pal_result.patch Avi Kivity
2010-01-13 9:32 ` Andrew Morton
2010-01-13 9:37 ` Avi Kivity
2010-01-13 9:59 ` Zhang, Xiantao
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox