Re: [PATCH 1/1] kvm: Clear variables which may not be used

[Li Zhang]

On 12/6/21 12:39 PM, Daniel P. Berrangé wrote:
> On Mon, Dec 06, 2021 at 12:27:38PM +0100, Li Zhang wrote:
> > The variables msi, route in kvm_irqchip_send_msi may be uninitialised
> > values in some cases. It's necessary to clear them.
> You say the patch is going to 'clear them' but....

Ah, sorry for my bad. And this is not correct fix.

> > Signed-off-by: Li Zhang <lizhang@suse.de>
> > ---
> >   accel/kvm/kvm-all.c | 5 ++---
> >   1 file changed, 2 insertions(+), 3 deletions(-)
> >
> > diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
> > index eecd8031cf..bd50dc6b80 100644
> > --- a/accel/kvm/kvm-all.c
> > +++ b/accel/kvm/kvm-all.c
> > @@ -1913,10 +1913,8 @@ static KVMMSIRoute *kvm_lookup_msi_route(KVMState *s, 
> > MSIMessage msg)
> >   
> >   int kvm_irqchip_send_msi(KVMState *s, MSIMessage msg)
> >   {
> > -    struct kvm_msi msi;
> > -    KVMMSIRoute *route;
> > -
> >       if (kvm_direct_msi_allowed) {
> > +        struct kvm_msi msi;
> ...but this is still an uninitialized declaration.
>
> >           msi.address_lo = (uint32_t)msg.address;
> >           msi.address_hi = msg.address >> 32;
> >           msi.data = le32_to_cpu(msg.data);
> I guess the bug you were wanting to fix is that this code only
> initializes 5 out of 6 struct fields, before calling the
> ioctl.

Yes, you are right.

> > @@ -1926,6 +1924,7 @@ int kvm_irqchip_send_msi(KVMState *s, MSIMessage msg)
> >           return kvm_vm_ioctl(s, KVM_SIGNAL_MSI, &msi);
> >       }
> >   
> > +    KVMMSIRoute *route;
> This was initialized correctly before and didn't need moving
>
> >       route = kvm_lookup_msi_route(s, msg);
> >       if (!route) {
> >           int virq;
> Regards,
> Daniel