[pve-devel] [PATCH kvm] Fix CVE-2015-8817 and CVE-2015-8818

Wolfgang Bumiller w.bumiller at proxmox.com
Wed Mar 2 09:03:09 CET 2016


And two non-security relevant functionality fixes from the
same series:

CVE-2015-8817: exec: Respect as_tranlsate_internal length clamp
exec: do not clamp accesses to MMIO regions
CVE-2015-8818: exec: skip MMIO regions correctly in cpu_physical_memory_write_rom_internal
exec: clamp accesses against the MemoryRegionSection
---
 ...xec-do-not-clamp-accesses-to-MMIO-regions.patch | 53 +++++++++++++++
 ...-accesses-against-the-MemoryRegionSection.patch | 39 +++++++++++
 ...espect-as_tranlsate_internal-length-clamp.patch | 57 ++++++++++++++++
 ...MIO-regions-correctly-in-cpu_physical_mem.patch | 76 ++++++++++++++++++++++
 debian/patches/series                              |  4 ++
 5 files changed, 229 insertions(+)
 create mode 100644 debian/patches/0002-exec-do-not-clamp-accesses-to-MMIO-regions.patch
 create mode 100644 debian/patches/0004-exec-clamp-accesses-against-the-MemoryRegionSection.patch
 create mode 100644 debian/patches/CVE-2015-8817-exec-Respect-as_tranlsate_internal-length-clamp.patch
 create mode 100644 debian/patches/CVE-2015-8818-exec-skip-MMIO-regions-correctly-in-cpu_physical_mem.patch

diff --git a/debian/patches/0002-exec-do-not-clamp-accesses-to-MMIO-regions.patch b/debian/patches/0002-exec-do-not-clamp-accesses-to-MMIO-regions.patch
new file mode 100644
index 0000000..256970e
--- /dev/null
+++ b/debian/patches/0002-exec-do-not-clamp-accesses-to-MMIO-regions.patch
@@ -0,0 +1,53 @@
+From 83c9a2ae066a3dd968e774a96f90a239adc7f082 Mon Sep 17 00:00:00 2001
+From: Paolo Bonzini <pbonzini at redhat.com>
+Date: Wed, 17 Jun 2015 10:40:27 +0200
+Subject: [PATCH 2/4] exec: do not clamp accesses to MMIO regions
+MIME-Version: 1.0
+Content-Type: text/plain; charset=UTF-8
+Content-Transfer-Encoding: 8bit
+
+It is common for MMIO registers to overlap, for example a 4 byte register
+at 0xcf8 (totally random choice... :)) and a 1 byte register at 0xcf9.
+If these registers are implemented via separate MemoryRegions, it is
+wrong to clamp the accesses as the value written would be truncated.
+
+Hence for these regions the effects of commit 23820db (exec: Respect
+as_translate_internal length clamp, 2015-03-16, previously applied as
+commit c3c1bb99) must be skipped.
+
+Tested-by: Hervé Poussineau <hpoussin at reactos.org>
+Tested-by: Mark Cave-Ayland <mark.cave-ayland at ilande.co.uk>
+Signed-off-by: Paolo Bonzini <pbonzini at redhat.com>
+---
+ exec.c | 8 ++++++--
+ 1 file changed, 6 insertions(+), 2 deletions(-)
+
+diff --git a/exec.c b/exec.c
+index 1c3d210..03c9995 100644
+--- a/exec.c
++++ b/exec.c
+@@ -330,6 +330,7 @@ address_space_translate_internal(AddressSpaceDispatch *d, hwaddr addr, hwaddr *x
+                                  hwaddr *plen, bool resolve_subpage)
+ {
+     MemoryRegionSection *section;
++    MemoryRegion *mr;
+     Int128 diff;
+ 
+     section = address_space_lookup_region(d, addr, resolve_subpage);
+@@ -339,8 +340,11 @@ address_space_translate_internal(AddressSpaceDispatch *d, hwaddr addr, hwaddr *x
+     /* Compute offset within MemoryRegion */
+     *xlat = addr + section->offset_within_region;
+ 
+-    diff = int128_sub(section->mr->size, int128_make64(addr));
+-    *plen = int128_get64(int128_min(diff, int128_make64(*plen)));
++    mr = section->mr;
++    if (memory_region_is_ram(mr)) {
++        diff = int128_sub(mr->size, int128_make64(addr));
++        *plen = int128_get64(int128_min(diff, int128_make64(*plen)));
++    }
+     return section;
+ }
+ 
+-- 
+2.1.4
+
diff --git a/debian/patches/0004-exec-clamp-accesses-against-the-MemoryRegionSection.patch b/debian/patches/0004-exec-clamp-accesses-against-the-MemoryRegionSection.patch
new file mode 100644
index 0000000..61ec45e
--- /dev/null
+++ b/debian/patches/0004-exec-clamp-accesses-against-the-MemoryRegionSection.patch
@@ -0,0 +1,39 @@
+From 4b5d6bca704a8fba4d00e28ac7678639c1434a95 Mon Sep 17 00:00:00 2001
+From: Paolo Bonzini <pbonzini at redhat.com>
+Date: Wed, 17 Jun 2015 10:36:54 +0200
+Subject: [PATCH 4/4] exec: clamp accesses against the MemoryRegionSection
+MIME-Version: 1.0
+Content-Type: text/plain; charset=UTF-8
+Content-Transfer-Encoding: 8bit
+
+Because the clamping was done against the MemoryRegion,
+address_space_rw was effectively broken if a write spanned
+multiple sections that are not linear in underlying memory
+(with the memory not being under an IOMMU).
+
+This is visible with the MIPS rc4030 IOMMU, which is implemented
+as a series of alias memory regions that point to the actual RAM.
+
+Tested-by: Hervé Poussineau <hpoussin at reactos.org>
+Tested-by: Mark Cave-Ayland <mark.cave-ayland at ilande.co.uk>
+Signed-off-by: Paolo Bonzini <pbonzini at redhat.com>
+---
+ exec.c | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+diff --git a/exec.c b/exec.c
+index 80c9d79..2d7b62f 100644
+--- a/exec.c
++++ b/exec.c
+@@ -354,7 +354,7 @@ address_space_translate_internal(AddressSpaceDispatch *d, hwaddr addr, hwaddr *x
+      * the caller really has to do the clamping through memory_access_size.
+      */
+     if (memory_region_is_ram(mr)) {
+-        diff = int128_sub(mr->size, int128_make64(addr));
++        diff = int128_sub(section->size, int128_make64(addr));
+         *plen = int128_get64(int128_min(diff, int128_make64(*plen)));
+     }
+     return section;
+-- 
+2.1.4
+
diff --git a/debian/patches/CVE-2015-8817-exec-Respect-as_tranlsate_internal-length-clamp.patch b/debian/patches/CVE-2015-8817-exec-Respect-as_tranlsate_internal-length-clamp.patch
new file mode 100644
index 0000000..e2c7049
--- /dev/null
+++ b/debian/patches/CVE-2015-8817-exec-Respect-as_tranlsate_internal-length-clamp.patch
@@ -0,0 +1,57 @@
+From 5f918b05bc7d2c6b9c3b60f01c8ee0446736f8de Mon Sep 17 00:00:00 2001
+From: Peter Crosthwaite <peter.crosthwaite at xilinx.com>
+Date: Mon, 16 Mar 2015 22:35:54 -0700
+Subject: [PATCH 1/4] exec: Respect as_tranlsate_internal length clamp
+
+address_space_translate_internal will clamp the *plen length argument
+based on the size of the memory region being queried. The iommu walker
+logic in addresss_space_translate was ignoring this by discarding the
+post fn call value of *plen. Fix by just always using *plen as the
+length argument throughout the fn, removing the len local variable.
+
+This fixes a bootloader bug when a single elf section spans multiple
+QEMU memory regions.
+
+Signed-off-by: Peter Crosthwaite <peter.crosthwaite at xilinx.com>
+Message-Id: <1426570554-15940-1-git-send-email-peter.crosthwaite at xilinx.com>
+Signed-off-by: Paolo Bonzini <pbonzini at redhat.com>
+---
+ exec.c | 6 ++----
+ 1 file changed, 2 insertions(+), 4 deletions(-)
+
+diff --git a/exec.c b/exec.c
+index 46fe70e..1c3d210 100644
+--- a/exec.c
++++ b/exec.c
+@@ -363,7 +363,6 @@ MemoryRegion *address_space_translate(AddressSpace *as, hwaddr addr,
+     IOMMUTLBEntry iotlb;
+     MemoryRegionSection *section;
+     MemoryRegion *mr;
+-    hwaddr len = *plen;
+ 
+     for (;;) {
+         section = address_space_translate_internal(as->dispatch, addr, &addr, plen, true);
+@@ -376,7 +375,7 @@ MemoryRegion *address_space_translate(AddressSpace *as, hwaddr addr,
+         iotlb = mr->iommu_ops->translate(mr, addr, is_write);
+         addr = ((iotlb.translated_addr & ~iotlb.addr_mask)
+                 | (addr & iotlb.addr_mask));
+-        len = MIN(len, (addr | iotlb.addr_mask) - addr + 1);
++        *plen = MIN(*plen, (addr | iotlb.addr_mask) - addr + 1);
+         if (!(iotlb.perm & (1 << is_write))) {
+             mr = &io_mem_unassigned;
+             break;
+@@ -387,10 +386,9 @@ MemoryRegion *address_space_translate(AddressSpace *as, hwaddr addr,
+ 
+     if (xen_enabled() && memory_access_is_direct(mr, is_write)) {
+         hwaddr page = ((addr & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE) - addr;
+-        len = MIN(page, len);
++        *plen = MIN(page, *plen);
+     }
+ 
+-    *plen = len;
+     *xlat = addr;
+     return mr;
+ }
+-- 
+2.1.4
+
diff --git a/debian/patches/CVE-2015-8818-exec-skip-MMIO-regions-correctly-in-cpu_physical_mem.patch b/debian/patches/CVE-2015-8818-exec-skip-MMIO-regions-correctly-in-cpu_physical_mem.patch
new file mode 100644
index 0000000..8958352
--- /dev/null
+++ b/debian/patches/CVE-2015-8818-exec-skip-MMIO-regions-correctly-in-cpu_physical_mem.patch
@@ -0,0 +1,76 @@
+From 56daf5912f942155224af873b056f981fca2de8b Mon Sep 17 00:00:00 2001
+From: Paolo Bonzini <pbonzini at redhat.com>
+Date: Sat, 4 Jul 2015 00:24:51 +0200
+Subject: [PATCH 3/4] exec: skip MMIO regions correctly in
+ cpu_physical_memory_write_rom_internal
+
+Loading the BIOS in the mac99 machine is interesting, because there is a
+PROM in the middle of the BIOS region (from 16K to 32K).  Before memory
+region accesses were clamped, when QEMU was asked to load a BIOS from
+0xfff00000 to 0xffffffff it would put even those 16K from the BIOS file
+into the region.  This is weird because those 16K were not actually
+visible between 0xfff04000 and 0xfff07fff.  However, it worked.
+
+After clamping was added, this also worked.  In this case, the
+cpu_physical_memory_write_rom_internal function split the write in
+three parts: the first 16K were copied, the PROM area (second 16K) were
+ignored, then the rest was copied.
+
+Problems then started with commit 965eb2f (exec: do not clamp accesses
+to MMIO regions, 2015-06-17).  Clamping accesses is not done for MMIO
+regions because they can overlap wildly, and MMIO registers can be
+expected to perform full-width accesses based only on their address
+(with no respect for adjacent registers that could decode to completely
+different MemoryRegions).  However, this lack of clamping also applied
+to the PROM area!  cpu_physical_memory_write_rom_internal thus failed
+to copy the third range above, i.e. only copied the first 16K of the BIOS.
+
+In effect, address_space_translate is expecting _something else_ to do
+the clamping for MMIO regions if the incoming length is large.  This
+"something else" is memory_access_size in the case of address_space_rw,
+so use the same logic in cpu_physical_memory_write_rom_internal.
+
+Reported-by: Alexander Graf <agraf at redhat.com>
+Reviewed-by: Laurent Vivier <lvivier at redhat.com>
+Tested-by: Laurent Vivier <lvivier at redhat.com>
+Fixes: 965eb2f
+Signed-off-by: Paolo Bonzini <pbonzini at redhat.com>
+---
+ exec.c | 14 +++++++++++++-
+ 1 file changed, 13 insertions(+), 1 deletion(-)
+
+diff --git a/exec.c b/exec.c
+index 03c9995..80c9d79 100644
+--- a/exec.c
++++ b/exec.c
+@@ -341,6 +341,18 @@ address_space_translate_internal(AddressSpaceDispatch *d, hwaddr addr, hwaddr *x
+     *xlat = addr + section->offset_within_region;
+ 
+     mr = section->mr;
++
++    /* MMIO registers can be expected to perform full-width accesses based only
++     * on their address, without considering adjacent registers that could
++     * decode to completely different MemoryRegions.  When such registers
++     * exist (e.g. I/O ports 0xcf8 and 0xcf9 on most PC chipsets), MMIO
++     * regions overlap wildly.  For this reason we cannot clamp the accesses
++     * here.
++     *
++     * If the length is small (as is the case for address_space_ldl/stl),
++     * everything works fine.  If the incoming length is large, however,
++     * the caller really has to do the clamping through memory_access_size.
++     */
+     if (memory_region_is_ram(mr)) {
+         diff = int128_sub(mr->size, int128_make64(addr));
+         *plen = int128_get64(int128_min(diff, int128_make64(*plen)));
+@@ -2236,7 +2248,7 @@ static inline void cpu_physical_memory_write_rom_internal(AddressSpace *as,
+ 
+         if (!(memory_region_is_ram(mr) ||
+               memory_region_is_romd(mr))) {
+-            /* do nothing */
++            l = memory_access_size(mr, l, addr1);
+         } else {
+             addr1 += memory_region_get_ram_addr(mr);
+             /* ROM/RAM case */
+-- 
+2.1.4
+
diff --git a/debian/patches/series b/debian/patches/series
index 33d19da..f8e218f 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -62,3 +62,7 @@ CVE-2015-7295-virtio-introduce-virtqueue_unmap_sg.patch
 CVE-2016-2391-usb-ohci-avoid-multiple-eof-timers.patch
 CVE-2016-2392-check-USB-configuration-descriptor-object.patch
 CVE-2016-2538-usb-check-RNDIS-message-length.patch
+CVE-2015-8817-exec-Respect-as_tranlsate_internal-length-clamp.patch
+0002-exec-do-not-clamp-accesses-to-MMIO-regions.patch
+CVE-2015-8818-exec-skip-MMIO-regions-correctly-in-cpu_physical_mem.patch
+0004-exec-clamp-accesses-against-the-MemoryRegionSection.patch
-- 
2.1.4





More information about the pve-devel mailing list