| Message ID | 1336666788-30233-2-git-send-email-jim@meyering.net |
|---|---|
| State | New |
| Headers | show |
On 2012-05-10 13:19, Jim Meyering wrote: > From: Jim Meyering <meyering@redhat.com> > > kvm_put_apic_state's attempt to clear *kapic before setting its > bits cleared sizeof(void*) bytes (no more than 8) rather than the > intended 1024 (KVM_APIC_REG_SIZE) bytes. Spotted by coverity. > > Signed-off-by: Jim Meyering <meyering@redhat.com> > --- > hw/kvm/apic.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/kvm/apic.c b/hw/kvm/apic.c > index ffe7a52..a0ab503 100644 > --- a/hw/kvm/apic.c > +++ b/hw/kvm/apic.c > @@ -29,7 +29,7 @@ void kvm_put_apic_state(DeviceState *d, struct kvm_lapic_state *kapic) > APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d); > int i; > > - memset(kapic, 0, sizeof(kapic)); > + memset(kapic, 0, sizeof(*kapic)); > kvm_apic_set_reg(kapic, 0x2, s->id << 24); > kvm_apic_set_reg(kapic, 0x8, s->tpr); > kvm_apic_set_reg(kapic, 0xd, s->log_dest << 24); Yep, that's what I actually meant... Thanks, Jan
Am 10.05.2012 19:28, schrieb Jan Kiszka: > On 2012-05-10 13:19, Jim Meyering wrote: > >> From: Jim Meyering<meyering@redhat.com> >> >> kvm_put_apic_state's attempt to clear *kapic before setting its >> bits cleared sizeof(void*) bytes (no more than 8) rather than the >> intended 1024 (KVM_APIC_REG_SIZE) bytes. Spotted by coverity. >> >> Signed-off-by: Jim Meyering<meyering@redhat.com> >> --- >> hw/kvm/apic.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/hw/kvm/apic.c b/hw/kvm/apic.c >> index ffe7a52..a0ab503 100644 >> --- a/hw/kvm/apic.c >> +++ b/hw/kvm/apic.c >> @@ -29,7 +29,7 @@ void kvm_put_apic_state(DeviceState *d, struct kvm_lapic_state *kapic) >> APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d); >> int i; >> >> - memset(kapic, 0, sizeof(kapic)); >> + memset(kapic, 0, sizeof(*kapic)); >> kvm_apic_set_reg(kapic, 0x2, s->id<< 24); >> kvm_apic_set_reg(kapic, 0x8, s->tpr); >> kvm_apic_set_reg(kapic, 0xd, s->log_dest<< 24); >> > Yep, that's what I actually meant... > > Thanks, > Jan > > Reviewed-by: Stefan Weil <sw@weilnetz.de> Hello Anthony, this patch should be committed to QEMU 1.1. I had sent a patch with the same fix 6 days later. Regards, Stefan W.
On 2012-05-22 17:30, Stefan Weil wrote: > Am 10.05.2012 19:28, schrieb Jan Kiszka: >> On 2012-05-10 13:19, Jim Meyering wrote: >> >>> From: Jim Meyering<meyering@redhat.com> >>> >>> kvm_put_apic_state's attempt to clear *kapic before setting its >>> bits cleared sizeof(void*) bytes (no more than 8) rather than the >>> intended 1024 (KVM_APIC_REG_SIZE) bytes. Spotted by coverity. >>> >>> Signed-off-by: Jim Meyering<meyering@redhat.com> >>> --- >>> hw/kvm/apic.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/hw/kvm/apic.c b/hw/kvm/apic.c >>> index ffe7a52..a0ab503 100644 >>> --- a/hw/kvm/apic.c >>> +++ b/hw/kvm/apic.c >>> @@ -29,7 +29,7 @@ void kvm_put_apic_state(DeviceState *d, struct kvm_lapic_state *kapic) >>> APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d); >>> int i; >>> >>> - memset(kapic, 0, sizeof(kapic)); >>> + memset(kapic, 0, sizeof(*kapic)); >>> kvm_apic_set_reg(kapic, 0x2, s->id<< 24); >>> kvm_apic_set_reg(kapic, 0x8, s->tpr); >>> kvm_apic_set_reg(kapic, 0xd, s->log_dest<< 24); >>> >> Yep, that's what I actually meant... >> >> Thanks, >> Jan >> >> > > Reviewed-by: Stefan Weil <sw@weilnetz.de> > > Hello Anthony, > > this patch should be committed to QEMU 1.1. > I had sent a patch with the same fix 6 days later. Thanks for reminding. Yes, please merge! Jan
Am 23.05.2012 12:33, schrieb Jan Kiszka: > On 2012-05-22 17:30, Stefan Weil wrote: >> Am 10.05.2012 19:28, schrieb Jan Kiszka: >>> On 2012-05-10 13:19, Jim Meyering wrote: >>> >>>> From: Jim Meyering<meyering@redhat.com> >>>> >>>> kvm_put_apic_state's attempt to clear *kapic before setting its >>>> bits cleared sizeof(void*) bytes (no more than 8) rather than the >>>> intended 1024 (KVM_APIC_REG_SIZE) bytes. Spotted by coverity. >>>> >>>> Signed-off-by: Jim Meyering<meyering@redhat.com> >>>> --- >>>> hw/kvm/apic.c | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/hw/kvm/apic.c b/hw/kvm/apic.c >>>> index ffe7a52..a0ab503 100644 >>>> --- a/hw/kvm/apic.c >>>> +++ b/hw/kvm/apic.c >>>> @@ -29,7 +29,7 @@ void kvm_put_apic_state(DeviceState *d, struct kvm_lapic_state *kapic) >>>> APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d); >>>> int i; >>>> >>>> - memset(kapic, 0, sizeof(kapic)); >>>> + memset(kapic, 0, sizeof(*kapic)); >>>> kvm_apic_set_reg(kapic, 0x2, s->id<< 24); >>>> kvm_apic_set_reg(kapic, 0x8, s->tpr); >>>> kvm_apic_set_reg(kapic, 0xd, s->log_dest<< 24); >>>> >>> Yep, that's what I actually meant... >>> >>> Thanks, >>> Jan >>> >>> >> >> Reviewed-by: Stefan Weil<sw@weilnetz.de> >> >> Hello Anthony, >> >> this patch should be committed to QEMU 1.1. >> I had sent a patch with the same fix 6 days later. > > Thanks for reminding. Yes, please merge! > > Jan Ping? This is one of the bug fixes which is missing in QEMU 1.1, and it is also missing in latest QEMU git master. What can be done to get it committed? Cheers, Stefan W.
On 2012-06-10 22:29, Stefan Weil wrote: > Am 23.05.2012 12:33, schrieb Jan Kiszka: >> On 2012-05-22 17:30, Stefan Weil wrote: >>> Am 10.05.2012 19:28, schrieb Jan Kiszka: >>>> On 2012-05-10 13:19, Jim Meyering wrote: >>>> >>>>> From: Jim Meyering<meyering@redhat.com> >>>>> >>>>> kvm_put_apic_state's attempt to clear *kapic before setting its >>>>> bits cleared sizeof(void*) bytes (no more than 8) rather than the >>>>> intended 1024 (KVM_APIC_REG_SIZE) bytes. Spotted by coverity. >>>>> >>>>> Signed-off-by: Jim Meyering<meyering@redhat.com> >>>>> --- >>>>> hw/kvm/apic.c | 2 +- >>>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>>> >>>>> diff --git a/hw/kvm/apic.c b/hw/kvm/apic.c >>>>> index ffe7a52..a0ab503 100644 >>>>> --- a/hw/kvm/apic.c >>>>> +++ b/hw/kvm/apic.c >>>>> @@ -29,7 +29,7 @@ void kvm_put_apic_state(DeviceState *d, struct kvm_lapic_state *kapic) >>>>> APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d); >>>>> int i; >>>>> >>>>> - memset(kapic, 0, sizeof(kapic)); >>>>> + memset(kapic, 0, sizeof(*kapic)); >>>>> kvm_apic_set_reg(kapic, 0x2, s->id<< 24); >>>>> kvm_apic_set_reg(kapic, 0x8, s->tpr); >>>>> kvm_apic_set_reg(kapic, 0xd, s->log_dest<< 24); >>>>> >>>> Yep, that's what I actually meant... >>>> >>>> Thanks, >>>> Jan >>>> >>>> >>> >>> Reviewed-by: Stefan Weil<sw@weilnetz.de> >>> >>> Hello Anthony, >>> >>> this patch should be committed to QEMU 1.1. >>> I had sent a patch with the same fix 6 days later. >> >> Thanks for reminding. Yes, please merge! >> >> Jan > > > Ping? > > This is one of the bug fixes which is missing in QEMU 1.1, > and it is also missing in latest QEMU git master. > > What can be done to get it committed? Avi or Marcelo, please queue in uq/master and send a pull soon! That reminds me that [1] is still awaiting comments (and further testing). Critical for qemu-kvm 1.1 and qemu 1.1.1 as well. Jan [1] http://thread.gmane.org/gmane.comp.emulators.kvm.devel/92036
On 05/10/2012 07:19 PM, Jim Meyering wrote: > From: Jim Meyering <meyering@redhat.com> > > kvm_put_apic_state's attempt to clear *kapic before setting its > bits cleared sizeof(void*) bytes (no more than 8) rather than the > intended 1024 (KVM_APIC_REG_SIZE) bytes. Spotted by coverity. Thanks, applied to uq/master.
diff --git a/hw/kvm/apic.c b/hw/kvm/apic.c index ffe7a52..a0ab503 100644 --- a/hw/kvm/apic.c +++ b/hw/kvm/apic.c @@ -29,7 +29,7 @@ void kvm_put_apic_state(DeviceState *d, struct kvm_lapic_state *kapic) APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d); int i; - memset(kapic, 0, sizeof(kapic)); + memset(kapic, 0, sizeof(*kapic)); kvm_apic_set_reg(kapic, 0x2, s->id << 24); kvm_apic_set_reg(kapic, 0x8, s->tpr); kvm_apic_set_reg(kapic, 0xd, s->log_dest << 24);