Re: [Qemu-devel] [PATCH 1/2] kvm: zero-initialize KVM_SET_GSI_ROUTING input

[Gleb Natapov <gleb@redhat.com>]

On Tue, Jun 04, 2013 at 02:52:32PM +0300, Michael S. Tsirkin wrote:
> kvm_add_routing_entry makes an attempt to
> zero-initialize any new routing entry.
> However, it fails to initialize padding
> within the u field of the structure
> kvm_irq_routing_entry.
> 
> Other functions like kvm_irqchip_update_msi_route
> also fail to initialize the padding field in
> kvm_irq_routing_entry.
> 
> While mostly harmless, this would prevent us from
> reusing these fields for something useful in
> the future.
> 
The fact that kernel does not check them for zero value is what will
prevents us from doing so.

> It's better to just make sure all input is initialized.
> 
> Once it is, we can also drop complex field by field assignment and just
> do the simple *a = *b to update a route entry.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>  kvm-all.c | 19 +++++++------------
>  1 file changed, 7 insertions(+), 12 deletions(-)
> 
> diff --git a/kvm-all.c b/kvm-all.c
> index 405480e..f119ce1 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -1006,11 +1006,8 @@ static void kvm_add_routing_entry(KVMState *s,
>      }
>      n = s->irq_routes->nr++;
>      new = &s->irq_routes->entries[n];
> -    memset(new, 0, sizeof(*new));
> -    new->gsi = entry->gsi;
> -    new->type = entry->type;
> -    new->flags = entry->flags;
> -    new->u = entry->u;
> +
> +    *new = *entry;
>  
>      set_gsi(s, entry->gsi);
>  
> @@ -1029,9 +1026,7 @@ static int kvm_update_routing_entry(KVMState *s,
>              continue;
>          }
>  
> -        entry->type = new_entry->type;
> -        entry->flags = new_entry->flags;
> -        entry->u = new_entry->u;
> +        *entry = *new_entry;
>  
>          kvm_irqchip_commit_routes(s);
>  
> @@ -1043,7 +1038,7 @@ static int kvm_update_routing_entry(KVMState *s,
>  
>  void kvm_irqchip_add_irq_route(KVMState *s, int irq, int irqchip, int pin)
>  {
> -    struct kvm_irq_routing_entry e;
> +    struct kvm_irq_routing_entry e = {};
>  
>      assert(pin < s->gsi_count);
>  
> @@ -1156,7 +1151,7 @@ int kvm_irqchip_send_msi(KVMState *s, MSIMessage msg)
>              return virq;
>          }
>  
> -        route = g_malloc(sizeof(KVMMSIRoute));
> +        route = g_malloc0(sizeof(KVMMSIRoute));
>          route->kroute.gsi = virq;
>          route->kroute.type = KVM_IRQ_ROUTING_MSI;
>          route->kroute.flags = 0;
> @@ -1177,7 +1172,7 @@ int kvm_irqchip_send_msi(KVMState *s, MSIMessage msg)
>  
>  int kvm_irqchip_add_msi_route(KVMState *s, MSIMessage msg)
>  {
> -    struct kvm_irq_routing_entry kroute;
> +    struct kvm_irq_routing_entry kroute = {};
>      int virq;
>  
>      if (!kvm_gsi_routing_enabled()) {
> @@ -1203,7 +1198,7 @@ int kvm_irqchip_add_msi_route(KVMState *s, MSIMessage msg)
>  
>  int kvm_irqchip_update_msi_route(KVMState *s, int virq, MSIMessage msg)
>  {
> -    struct kvm_irq_routing_entry kroute;
> +    struct kvm_irq_routing_entry kroute = {};
>  
>      if (!kvm_irqchip_in_kernel()) {
>          return -ENOSYS;
> -- 
> MST

--
			Gleb.