Re: [Qemu-devel] [PULL 23/35] x86: ioapic: dump version for "info ioapic"

[Peter Xu]

On Mon, Jan 30, 2017 at 02:33:32PM -0500, Paolo Bonzini wrote:
> 
> 
> On 30/01/2017 09:07, Peter Maydell wrote:
> > On 20 January 2017 at 13:31, Paolo Bonzini <address@hidden> wrote:
> >> From: Peter Xu <address@hidden>
> >>
> >> Signed-off-by: Peter Xu <address@hidden>
> >> Message-Id: <address@hidden>
> >> Signed-off-by: Paolo Bonzini <address@hidden>
> >> ---
> >>  hw/intc/ioapic_common.c | 3 ++-
> >>  1 file changed, 2 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/hw/intc/ioapic_common.c b/hw/intc/ioapic_common.c
> >> index 1b7ec5e..97c4f9c 100644
> >> --- a/hw/intc/ioapic_common.c
> >> +++ b/hw/intc/ioapic_common.c
> >> @@ -58,7 +58,8 @@ void ioapic_print_redtbl(Monitor *mon, IOAPICCommonState 
> >> *s)
> >>      uint32_t remote_irr = 0;
> >>      int i;
> >>
> >> -    monitor_printf(mon, "ioapic id=0x%02x sel=0x%02x", s->id, 
> >> s->ioregsel);
> >> +    monitor_printf(mon, "ioapic ver=0x%x id=0x%02x sel=0x%02x",
> >> +                   s->version, s->id, s->ioregsel);
> >>      if (s->ioregsel) {
> >>          monitor_printf(mon, " (redir[%u])\n",
> >>                         (s->ioregsel - IOAPIC_REG_REDTBL_BASE) >> 1);
> > 
> > Coverity points out (CID 1369422) that this is a use of a possibly
> > uninitialized field. In kvm_ioapic_dump_state() we do:
> > 
> >     IOAPICCommonState s;
> >     kvm_ioapic_get(&s);
> >     ioapic_print_redtbl(mon, &s);
> > 
> > and kvm_ioapic_get() doesn't initialize s->version, so when we
> > come to print it in ioapic_print_redtbl() it's uninitialized.
> > 
> > The easy fix is to initialize version to something. The
> > underlying problem here I think is that we're manufacturing
> > a fake IOAPICCommonState rather than finding the one that
> > corresponds to the actual IOAPIC device in the system...
> 
> Right, we can probably use object_resolve_path to get one.

I'll use this and post a fix for it. Thanks!

-- peterx