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

2013-06-05 Thread Gleb Natapov 
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 m...@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.
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2013-06-05 Thread Michael S. Tsirkin 
On Wed, Jun 05, 2013 at 04:00:20PM +0300, Gleb Natapov wrote:
 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.

Well we can not change kernel now (it would break userspace)
but we can start zeroing everything in userspace.

Also, checkers like coverity might get confused by this.

Finally, simpler assignment and comparison make it worth it,
don't they?

  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 m...@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.
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2013-06-05 Thread Gleb Natapov 
On Wed, Jun 05, 2013 at 05:02:07PM +0300, Michael S. Tsirkin wrote:
 On Wed, Jun 05, 2013 at 04:00:20PM +0300, Gleb Natapov wrote:
  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.
 
 Well we can not change kernel now (it would break userspace)
 but we can start zeroing everything in userspace.
 
 Also, checkers like coverity might get confused by this.
 
 Finally, simpler assignment and comparison make it worth it,
 don't they?
 
I am not at all against the patch! Just pointing out a mistake in the
commit message.

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

--
Gleb.
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2013-06-05 Thread Michael S. Tsirkin 
On Wed, Jun 05, 2013 at 05:04:55PM +0300, Gleb Natapov wrote:
 On Wed, Jun 05, 2013 at 05:02:07PM +0300, Michael S. Tsirkin wrote:
  On Wed, Jun 05, 2013 at 04:00:20PM +0300, Gleb Natapov wrote:
   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.
  
  Well we can not change kernel now (it would break userspace)
  but we can start zeroing everything in userspace.
  
  Also, checkers like coverity might get confused by this.
  
  Finally, simpler assignment and comparison make it worth it,
  don't they?
  
 I am not at all against the patch! Just pointing out a mistake in the
 commit message.

I think we can agree both userspace not initializing them and kernel not
checking it are mistakes?

Anyway ... could you commit this tweaking the commit
message in a way that you consider appropriate?
Or want me to repost?


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 m...@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.
 
 --
   Gleb.
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2013-06-05 Thread Gleb Natapov 
On Wed, Jun 05, 2013 at 05:11:44PM +0300, Michael S. Tsirkin wrote:
 On Wed, Jun 05, 2013 at 05:04:55PM +0300, Gleb Natapov wrote:
  On Wed, Jun 05, 2013 at 05:02:07PM +0300, Michael S. Tsirkin wrote:
   On Wed, Jun 05, 2013 at 04:00:20PM +0300, Gleb Natapov wrote:
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.
   
   Well we can not change kernel now (it would break userspace)
   but we can start zeroing everything in userspace.
   
   Also, checkers like coverity might get confused by this.
   
   Finally, simpler assignment and comparison make it worth it,
   don't they?
   
  I am not at all against the patch! Just pointing out a mistake in the
  commit message.
 
 I think we can agree both userspace not initializing them and kernel not
 checking it are mistakes?
 
Mistake that cannot be fixed at this point.

 Anyway ... could you commit this tweaking the commit
 message in a way that you consider appropriate?
 Or want me to repost?
 
No need to report.

 
 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 m...@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.
  
  --
  Gleb.

--
Gleb.
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2013-06-05 Thread Michael S. Tsirkin 
On Wed, Jun 05, 2013 at 05:12:32PM +0300, Gleb Natapov wrote:
 On Wed, Jun 05, 2013 at 05:11:44PM +0300, Michael S. Tsirkin wrote:
  On Wed, Jun 05, 2013 at 05:04:55PM +0300, Gleb Natapov wrote:
   On Wed, Jun 05, 2013 at 05:02:07PM +0300, Michael S. Tsirkin wrote:
On Wed, Jun 05, 2013 at 04:00:20PM +0300, Gleb Natapov wrote:
 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.

Well we can not change kernel now (it would break userspace)
but we can start zeroing everything in userspace.

Also, checkers like coverity might get confused by this.

Finally, simpler assignment and comparison make it worth it,
don't they?

   I am not at all against the patch! Just pointing out a mistake in the
   commit message.
  
  I think we can agree both userspace not initializing them and kernel not
  checking it are mistakes?
  
 Mistake that cannot be fixed at this point.
 
  Anyway ... could you commit this tweaking the commit
  message in a way that you consider appropriate?
  Or want me to repost?
  
 No need to report.

Thanks.

  
  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 m...@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.
   
   --
 Gleb.
 
 --
   Gleb.
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2013-06-05 Thread Gleb Natapov 
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.
 
 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 m...@redhat.com
Applied, thanks.

 ---
  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.
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2013-06-04 Thread Michael S. Tsirkin 
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.

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 m...@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
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html