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

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

[Li Zhang]

The variables msi, route in kvm_irqchip_send_msi may be uninitialised
values in some cases. It's necessary to clear them.

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;
         msi.address_lo = (uint32_t)msg.address;
         msi.address_hi = msg.address >> 32;
         msi.data = le32_to_cpu(msg.data);
@@ -1926,6 +1924,7 @@ int kvm_irqchip_send_msi(KVMState *s, MSIMessage msg)
         return kvm_vm_ioctl(s, KVM_SIGNAL_MSI, &msi);
     }
 
+    KVMMSIRoute *route;
     route = kvm_lookup_msi_route(s, msg);
     if (!route) {
         int virq;
-- 
2.31.1

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

[Daniel P. Berrangé]

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....

> 
> 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.

> @@ -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
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

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