[uq/master][PATCH 0/3] Fix initialization bugs in kvmvapic

All of lore.kernel.org
 help / color / mirror / Atom feed

* [uq/master][PATCH 0/3] Fix initialization bugs in kvmvapic
@ 2013-09-03 16:08 ` Jan Kiszka
  0 siblings, 0 replies; 10+ messages in thread
From: Jan Kiszka @ 2013-09-03 16:08 UTC (permalink / raw)
  To: Paolo Bonzini, Gleb Natapov
  Cc: qemu-devel, kvm, Zhanghaoyu (A), Huangweidong (C), Luonengjun,
	Michael S. Tsirkin, Eric Blake, qemu-stable

Addresses the issue Daniel reported in
http://thread.gmane.org/gmane.comp.emulators.qemu/231577


CC: qemu-stable@nongnu.org

Jan Kiszka (3):
  kvmvapic: Catch invalid ROM size
  kvmvapic: Enter inactive state on hardware reset
  kvmvapic: Clear also physical ROM address when entering INACTIVE state

 hw/i386/kvmvapic.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

-- 
1.8.1.1.298.ge7eed54


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [Qemu-devel] [uq/master][PATCH 0/3] Fix initialization bugs in kvmvapic
@ 2013-09-03 16:08 ` Jan Kiszka
  0 siblings, 0 replies; 10+ messages in thread
From: Jan Kiszka @ 2013-09-03 16:08 UTC (permalink / raw)
  To: Paolo Bonzini, Gleb Natapov
  Cc: Huangweidong (C), kvm, Michael S. Tsirkin, Zhanghaoyu (A),
	Luonengjun, qemu-devel, qemu-stable

Addresses the issue Daniel reported in
http://thread.gmane.org/gmane.comp.emulators.qemu/231577


CC: qemu-stable@nongnu.org

Jan Kiszka (3):
  kvmvapic: Catch invalid ROM size
  kvmvapic: Enter inactive state on hardware reset
  kvmvapic: Clear also physical ROM address when entering INACTIVE state

 hw/i386/kvmvapic.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

-- 
1.8.1.1.298.ge7eed54

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [uq/master][PATCH 1/3] kvmvapic: Catch invalid ROM size
  2013-09-03 16:08 ` [Qemu-devel] " Jan Kiszka
@ 2013-09-03 16:08   ` Jan Kiszka
  -1 siblings, 0 replies; 10+ messages in thread
From: Jan Kiszka @ 2013-09-03 16:08 UTC (permalink / raw)
  To: Paolo Bonzini, Gleb Natapov
  Cc: qemu-devel, kvm, Zhanghaoyu (A), Huangweidong (C), Luonengjun,
	Michael S. Tsirkin, Eric Blake, qemu-stable

If not caught early, a zero-length ROM will cause a NULL-pointer access
later on in patch_hypercalls when allocating a zero-length ROM copy and
trying to read from it.

CC: qemu-stable@nongnu.org
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 hw/i386/kvmvapic.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/hw/i386/kvmvapic.c b/hw/i386/kvmvapic.c
index 15beb80..7ac0fe1 100644
--- a/hw/i386/kvmvapic.c
+++ b/hw/i386/kvmvapic.c
@@ -578,7 +578,7 @@ static int patch_hypercalls(VAPICROMState *s)
  * enable write access to the option ROM so that variables can be updated by
  * the guest.
  */
-static void vapic_map_rom_writable(VAPICROMState *s)
+static int vapic_map_rom_writable(VAPICROMState *s)
 {
     hwaddr rom_paddr = s->rom_state_paddr & ROM_BLOCK_MASK;
     MemoryRegionSection section;
@@ -599,6 +599,9 @@ static void vapic_map_rom_writable(VAPICROMState *s)
     /* read ROM size from RAM region */
     ram = memory_region_get_ram_ptr(section.mr);
     rom_size = ram[rom_paddr + 2] * ROM_BLOCK_SIZE;
+    if (rom_size == 0) {
+        return -1;
+    }
     s->rom_size = rom_size;
 
     /* We need to round to avoid creating subpages
@@ -612,11 +615,15 @@ static void vapic_map_rom_writable(VAPICROMState *s)
     memory_region_add_subregion_overlap(as, rom_paddr, &s->rom, 1000);
     s->rom_mapped_writable = true;
     memory_region_unref(section.mr);
+
+    return 0;
 }
 
 static int vapic_prepare(VAPICROMState *s)
 {
-    vapic_map_rom_writable(s);
+    if (vapic_map_rom_writable(s) < 0) {
+        return -1;
+    }
 
     if (patch_hypercalls(s) < 0) {
         return -1;
-- 
1.8.1.1.298.ge7eed54


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [Qemu-devel] [uq/master][PATCH 1/3] kvmvapic: Catch invalid ROM size
@ 2013-09-03 16:08   ` Jan Kiszka
  0 siblings, 0 replies; 10+ messages in thread
From: Jan Kiszka @ 2013-09-03 16:08 UTC (permalink / raw)
  To: Paolo Bonzini, Gleb Natapov
  Cc: Huangweidong (C), kvm, Michael S. Tsirkin, Zhanghaoyu (A),
	Luonengjun, qemu-devel, qemu-stable

If not caught early, a zero-length ROM will cause a NULL-pointer access
later on in patch_hypercalls when allocating a zero-length ROM copy and
trying to read from it.

CC: qemu-stable@nongnu.org
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 hw/i386/kvmvapic.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/hw/i386/kvmvapic.c b/hw/i386/kvmvapic.c
index 15beb80..7ac0fe1 100644
--- a/hw/i386/kvmvapic.c
+++ b/hw/i386/kvmvapic.c
@@ -578,7 +578,7 @@ static int patch_hypercalls(VAPICROMState *s)
  * enable write access to the option ROM so that variables can be updated by
  * the guest.
  */
-static void vapic_map_rom_writable(VAPICROMState *s)
+static int vapic_map_rom_writable(VAPICROMState *s)
 {
     hwaddr rom_paddr = s->rom_state_paddr & ROM_BLOCK_MASK;
     MemoryRegionSection section;
@@ -599,6 +599,9 @@ static void vapic_map_rom_writable(VAPICROMState *s)
     /* read ROM size from RAM region */
     ram = memory_region_get_ram_ptr(section.mr);
     rom_size = ram[rom_paddr + 2] * ROM_BLOCK_SIZE;
+    if (rom_size == 0) {
+        return -1;
+    }
     s->rom_size = rom_size;
 
     /* We need to round to avoid creating subpages
@@ -612,11 +615,15 @@ static void vapic_map_rom_writable(VAPICROMState *s)
     memory_region_add_subregion_overlap(as, rom_paddr, &s->rom, 1000);
     s->rom_mapped_writable = true;
     memory_region_unref(section.mr);
+
+    return 0;
 }
 
 static int vapic_prepare(VAPICROMState *s)
 {
-    vapic_map_rom_writable(s);
+    if (vapic_map_rom_writable(s) < 0) {
+        return -1;
+    }
 
     if (patch_hypercalls(s) < 0) {
         return -1;
-- 
1.8.1.1.298.ge7eed54

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [uq/master][PATCH 2/3] kvmvapic: Enter inactive state on hardware reset
  2013-09-03 16:08 ` [Qemu-devel] " Jan Kiszka
@ 2013-09-03 16:08   ` Jan Kiszka
  -1 siblings, 0 replies; 10+ messages in thread
From: Jan Kiszka @ 2013-09-03 16:08 UTC (permalink / raw)
  To: Paolo Bonzini, Gleb Natapov
  Cc: qemu-devel, kvm, Zhanghaoyu (A), Huangweidong (C), Luonengjun,
	Michael S. Tsirkin, Eric Blake, qemu-stable

ROM layout may change after reset of devices are hotplugged, so we have
to pick up the physical address again when the ROM is initialized. This
is best achieved by resetting the state to INACTIVE.

CC: qemu-stable@nongnu.org
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 hw/i386/kvmvapic.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/hw/i386/kvmvapic.c b/hw/i386/kvmvapic.c
index 7ac0fe1..f2e335d 100644
--- a/hw/i386/kvmvapic.c
+++ b/hw/i386/kvmvapic.c
@@ -510,9 +510,7 @@ static void vapic_reset(DeviceState *dev)
 {
     VAPICROMState *s = VAPIC(dev);
 
-    if (s->state == VAPIC_ACTIVE) {
-        s->state = VAPIC_STANDBY;
-    }
+    s->state = VAPIC_INACTIVE;
     vapic_enable_tpr_reporting(false);
 }
 
-- 
1.8.1.1.298.ge7eed54


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [Qemu-devel] [uq/master][PATCH 2/3] kvmvapic: Enter inactive state on hardware reset
@ 2013-09-03 16:08   ` Jan Kiszka
  0 siblings, 0 replies; 10+ messages in thread
From: Jan Kiszka @ 2013-09-03 16:08 UTC (permalink / raw)
  To: Paolo Bonzini, Gleb Natapov
  Cc: Huangweidong (C), kvm, Michael S. Tsirkin, Zhanghaoyu (A),
	Luonengjun, qemu-devel, qemu-stable

ROM layout may change after reset of devices are hotplugged, so we have
to pick up the physical address again when the ROM is initialized. This
is best achieved by resetting the state to INACTIVE.

CC: qemu-stable@nongnu.org
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 hw/i386/kvmvapic.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/hw/i386/kvmvapic.c b/hw/i386/kvmvapic.c
index 7ac0fe1..f2e335d 100644
--- a/hw/i386/kvmvapic.c
+++ b/hw/i386/kvmvapic.c
@@ -510,9 +510,7 @@ static void vapic_reset(DeviceState *dev)
 {
     VAPICROMState *s = VAPIC(dev);
 
-    if (s->state == VAPIC_ACTIVE) {
-        s->state = VAPIC_STANDBY;
-    }
+    s->state = VAPIC_INACTIVE;
     vapic_enable_tpr_reporting(false);
 }
 
-- 
1.8.1.1.298.ge7eed54

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [uq/master][PATCH 3/3] kvmvapic: Clear also physical ROM address when entering INACTIVE state
  2013-09-03 16:08 ` [Qemu-devel] " Jan Kiszka
@ 2013-09-03 16:08   ` Jan Kiszka
  -1 siblings, 0 replies; 10+ messages in thread
From: Jan Kiszka @ 2013-09-03 16:08 UTC (permalink / raw)
  To: Paolo Bonzini, Gleb Natapov
  Cc: qemu-devel, kvm, Zhanghaoyu (A), Huangweidong (C), Luonengjun,
	Michael S. Tsirkin, Eric Blake, qemu-stable

To avoid misinterpreting INACTIVE after migration as old qemu-kvm's
STANDBY, also clear rom_state_paddr when going back to this state.

CC: qemu-stable@nongnu.org
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 hw/i386/kvmvapic.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/i386/kvmvapic.c b/hw/i386/kvmvapic.c
index f2e335d..cf6c714 100644
--- a/hw/i386/kvmvapic.c
+++ b/hw/i386/kvmvapic.c
@@ -511,6 +511,7 @@ static void vapic_reset(DeviceState *dev)
     VAPICROMState *s = VAPIC(dev);
 
     s->state = VAPIC_INACTIVE;
+    s->rom_state_paddr = 0;
     vapic_enable_tpr_reporting(false);
 }
 
@@ -664,6 +665,7 @@ static void vapic_write(void *opaque, hwaddr addr, uint64_t data,
         }
         if (vapic_prepare(s) < 0) {
             s->state = VAPIC_INACTIVE;
+            s->rom_state_paddr = 0;
             break;
         }
         break;
-- 
1.8.1.1.298.ge7eed54


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [Qemu-devel] [uq/master][PATCH 3/3] kvmvapic: Clear also physical ROM address when entering INACTIVE state
@ 2013-09-03 16:08   ` Jan Kiszka
  0 siblings, 0 replies; 10+ messages in thread
From: Jan Kiszka @ 2013-09-03 16:08 UTC (permalink / raw)
  To: Paolo Bonzini, Gleb Natapov
  Cc: Huangweidong (C), kvm, Michael S. Tsirkin, Zhanghaoyu (A),
	Luonengjun, qemu-devel, qemu-stable

To avoid misinterpreting INACTIVE after migration as old qemu-kvm's
STANDBY, also clear rom_state_paddr when going back to this state.

CC: qemu-stable@nongnu.org
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 hw/i386/kvmvapic.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/i386/kvmvapic.c b/hw/i386/kvmvapic.c
index f2e335d..cf6c714 100644
--- a/hw/i386/kvmvapic.c
+++ b/hw/i386/kvmvapic.c
@@ -511,6 +511,7 @@ static void vapic_reset(DeviceState *dev)
     VAPICROMState *s = VAPIC(dev);
 
     s->state = VAPIC_INACTIVE;
+    s->rom_state_paddr = 0;
     vapic_enable_tpr_reporting(false);
 }
 
@@ -664,6 +665,7 @@ static void vapic_write(void *opaque, hwaddr addr, uint64_t data,
         }
         if (vapic_prepare(s) < 0) {
             s->state = VAPIC_INACTIVE;
+            s->rom_state_paddr = 0;
             break;
         }
         break;
-- 
1.8.1.1.298.ge7eed54

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [uq/master][PATCH 0/3] Fix initialization bugs in kvmvapic
  2013-09-03 16:08 ` [Qemu-devel] " Jan Kiszka
@ 2013-09-12 16:30   ` Paolo Bonzini
  -1 siblings, 0 replies; 10+ messages in thread
From: Paolo Bonzini @ 2013-09-12 16:30 UTC (permalink / raw)
  To: Jan Kiszka
  Cc: Huangweidong (C), kvm, Gleb Natapov, Michael S. Tsirkin,
	Zhanghaoyu (A), Luonengjun, qemu-devel, qemu-stable

Il 03/09/2013 18:08, Jan Kiszka ha scritto:
> Addresses the issue Daniel reported in
> http://thread.gmane.org/gmane.comp.emulators.qemu/231577
> 
> 
> CC: qemu-stable@nongnu.org
> 
> Jan Kiszka (3):
>   kvmvapic: Catch invalid ROM size
>   kvmvapic: Enter inactive state on hardware reset
>   kvmvapic: Clear also physical ROM address when entering INACTIVE state
> 
>  hw/i386/kvmvapic.c | 17 ++++++++++++-----
>  1 file changed, 12 insertions(+), 5 deletions(-)
> 

Applied to uq/master, thanks.

Paolo

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Qemu-devel] [uq/master][PATCH 0/3] Fix initialization bugs in kvmvapic
@ 2013-09-12 16:30   ` Paolo Bonzini
  0 siblings, 0 replies; 10+ messages in thread
From: Paolo Bonzini @ 2013-09-12 16:30 UTC (permalink / raw)
  To: Jan Kiszka
  Cc: Huangweidong (C), kvm, Gleb Natapov, Michael S. Tsirkin,
	Zhanghaoyu (A), Luonengjun, qemu-devel, qemu-stable

Il 03/09/2013 18:08, Jan Kiszka ha scritto:
> Addresses the issue Daniel reported in
> http://thread.gmane.org/gmane.comp.emulators.qemu/231577
> 
> 
> CC: qemu-stable@nongnu.org
> 
> Jan Kiszka (3):
>   kvmvapic: Catch invalid ROM size
>   kvmvapic: Enter inactive state on hardware reset
>   kvmvapic: Clear also physical ROM address when entering INACTIVE state
> 
>  hw/i386/kvmvapic.c | 17 ++++++++++++-----
>  1 file changed, 12 insertions(+), 5 deletions(-)
> 

Applied to uq/master, thanks.

Paolo

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2013-09-12 16:30 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-09-03 16:08 [uq/master][PATCH 0/3] Fix initialization bugs in kvmvapic Jan Kiszka
2013-09-03 16:08 ` [Qemu-devel] " Jan Kiszka
2013-09-03 16:08 ` [uq/master][PATCH 1/3] kvmvapic: Catch invalid ROM size Jan Kiszka
2013-09-03 16:08   ` [Qemu-devel] " Jan Kiszka
2013-09-03 16:08 ` [uq/master][PATCH 2/3] kvmvapic: Enter inactive state on hardware reset Jan Kiszka
2013-09-03 16:08   ` [Qemu-devel] " Jan Kiszka
2013-09-03 16:08 ` [uq/master][PATCH 3/3] kvmvapic: Clear also physical ROM address when entering INACTIVE state Jan Kiszka
2013-09-03 16:08   ` [Qemu-devel] " Jan Kiszka
2013-09-12 16:30 ` [uq/master][PATCH 0/3] Fix initialization bugs in kvmvapic Paolo Bonzini
2013-09-12 16:30   ` [Qemu-devel] " Paolo Bonzini

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.