From f324c4a6ea40a8d3ae419c5e05af487680dc3552 Mon Sep 17 00:00:00 2001 From: Wolfgang Bumiller Date: Wed, 2 Mar 2016 09:03:09 +0100 Subject: [PATCH] Fix CVE-2015-8817 and CVE-2015-8818 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 --- ...o-not-clamp-accesses-to-MMIO-regions.patch | 53 +++++++++++++ ...sses-against-the-MemoryRegionSection.patch | 39 ++++++++++ ...t-as_tranlsate_internal-length-clamp.patch | 57 ++++++++++++++ ...egions-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 +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 +Tested-by: Mark Cave-Ayland +Signed-off-by: Paolo Bonzini +--- + 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 +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 +Tested-by: Mark Cave-Ayland +Signed-off-by: Paolo Bonzini +--- + 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 +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 +Message-Id: <1426570554-15940-1-git-send-email-peter.crosthwaite@xilinx.com> +Signed-off-by: Paolo Bonzini +--- + 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 +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 +Reviewed-by: Laurent Vivier +Tested-by: Laurent Vivier +Fixes: 965eb2f +Signed-off-by: Paolo Bonzini +--- + 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.39.2