Skip to contentSkip to conversation

kvmvapic: Prevent reading beyond the end of guest RAM

PATCH Applied Jan Kiszka 3 participants 3 messages Sep 30, 2013 – Oct 4, 2013 KVM

lore
Share & tools
lore

Jan Kiszka

author
#0 lore raw

PATCH 1/1 kvmvapic: Prevent reading beyond the end of guest RAM

rom_state_paddr is guest provided (caller address of outw(VAPIC_PORT) + writen 16-bit value) and can be influenced to point beyond the end of the host memory backing the guest's RAM. Make sure we do not use this pointer to actually read beyond the limits.

Reading arbitrary guest bytes is harmless, the guest kernel has to manage access to this I/O port anyway.

Signed-off-by Jan Kiszka <[email protected]>
---
 hw/i386/kvmvapic.c | 3 +++
 1 file changed, 3 insertions(+)
hw/i386/kvmvapic.c +3 -0
index 1c2dbf5..2d87600 100644
@@ -596,6 +596,9 @@ static int vapic_map_rom_writable(VAPICROMState *s)
     section = memory_region_find(as, 0, 1);
 
     /* read ROM size from RAM region */
+    if (rom_paddr + 2 >= memory_region_size(section.mr)) {
+        return -1;
+    }
     ram = memory_region_get_ram_ptr(section.mr);
     rom_size = ram[rom_paddr + 2] * ROM_BLOCK_SIZE;
     if (rom_size == 0) {
Signature · 1 line
1.8.1.1.298.ge7eed54
Acked-by on v1 1/1
#1 lore raw
Quoted 9 lines · Jan Kiszka

rom_state_paddr is guest provided (caller address of outw(VAPIC_PORT) + writen 16-bit value) and can be influenced to point beyond the end of the host memory backing the guest's RAM. Make sure we do not use this pointer to actually read beyond the limits.

Reading arbitrary guest bytes is harmless, the guest kernel has to manage access to this I/O port anyway.

Signed-off-by Jan Kiszka <[email protected]>
Acked-by Michael S. Tsirkin <[email protected]>
Quoted 20 lines
---
 hw/i386/kvmvapic.c | 3 +++
 1 file changed, 3 insertions(+)
hw/i386/kvmvapic.c +3 -0
index 1c2dbf5..2d87600 100644
@@ -596,6 +596,9 @@ static int vapic_map_rom_writable(VAPICROMState *s)
     section = memory_region_find(as, 0, 1);
 
     /* read ROM size from RAM region */
+    if (rom_paddr + 2 >= memory_region_size(section.mr)) {
+        return -1;
+    }
     ram = memory_region_get_ram_ptr(section.mr);
     rom_size = ram[rom_paddr + 2] * ROM_BLOCK_SIZE;
     if (rom_size == 0) {
Signature · 1 line
1.8.1.1.298.ge7eed54
Applied
#2 lore raw
In reply to Jan Kiszka
Quoted 9 lines · Jan Kiszka

rom_state_paddr is guest provided (caller address of outw(VAPIC_PORT) + writen 16-bit value) and can be influenced to point beyond the end of the host memory backing the guest's RAM. Make sure we do not use this pointer to actually read beyond the limits.

Reading arbitrary guest bytes is harmless, the guest kernel has to manage access to this I/O port anyway.

Signed-off-by Jan Kiszka <[email protected]>

Applied, thanks.

Quoted 20 lines
---
 hw/i386/kvmvapic.c | 3 +++
 1 file changed, 3 insertions(+)
hw/i386/kvmvapic.c +3 -0
index 1c2dbf5..2d87600 100644
@@ -596,6 +596,9 @@ static int vapic_map_rom_writable(VAPICROMState *s)
     section = memory_region_find(as, 0, 1);
 
     /* read ROM size from RAM region */
+    if (rom_paddr + 2 >= memory_region_size(section.mr)) {
+        return -1;
+    }
     ram = memory_region_get_ram_ptr(section.mr);
     rom_size = ram[rom_paddr + 2] * ROM_BLOCK_SIZE;
     if (rom_size == 0) {
Signature · 1 line
1.8.1.1.298.ge7eed54

--
Gleb.