From e4010373c72eab2342d2ba7f10c1ddf43dc618c8 Mon Sep 17 00:00:00 2001 From: Jon Maloy Date: Wed, 21 Apr 2021 22:30:03 -0400 Subject: [PATCH 4/8] memory: Revert "memory: accept mismatching sizes in memory_region_access_valid" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RH-Author: Jon Maloy Message-id: <20210421223006.19650-4-jmaloy@redhat.com> Patchwork-id: 101480 O-Subject: [RHEL-8.5.0 qemu-kvm PATCH v2 3/6] memory: Revert "memory: accept mismatching sizes in memory_region_access_valid" Bugzilla: 1944621 RH-Acked-by: Stefano Garzarella RH-Acked-by: Philippe Mathieu-Daudé RH-Acked-by: Laszlo Ersek From: "Michael S. Tsirkin" Memory API documentation documents valid .min_access_size and .max_access_size fields and explains that any access outside these boundaries is blocked. This is what devices seem to assume. However this is not what the implementation does: it simply ignores the boundaries unless there's an "accepts" callback. Naturally, this breaks a bunch of devices. Revert to the documented behaviour. Devices that want to allow any access can just drop the valid field, or add the impl field to have accesses converted to appropriate length. Cc: qemu-stable@nongnu.org Reviewed-by: Richard Henderson Fixes: CVE-2020-13754 Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1842363 Fixes: a014ed07bd5a ("memory: accept mismatching sizes in memory_region_access_valid") Signed-off-by: Michael S. Tsirkin Message-Id: <20200610134731.1514409-1-mst@redhat.com> Signed-off-by: Paolo Bonzini (cherry picked from commit 5d971f9e672507210e77d020d89e0e89165c8fc9) Signed-off-by: Jon Maloy Signed-off-by: Danilo C. L. de Paula --- memory.c | 29 +++++++++-------------------- 1 file changed, 9 insertions(+), 20 deletions(-) diff --git a/memory.c b/memory.c index 5a4a80842d..0cfcb72a5a 100644 --- a/memory.c +++ b/memory.c @@ -1351,35 +1351,24 @@ bool memory_region_access_valid(MemoryRegion *mr, bool is_write, MemTxAttrs attrs) { - int access_size_min, access_size_max; - int access_size, i; - - if (!mr->ops->valid.unaligned && (addr & (size - 1))) { + if (mr->ops->valid.accepts + && !mr->ops->valid.accepts(mr->opaque, addr, size, is_write, attrs)) { return false; } - if (!mr->ops->valid.accepts) { - return true; - } - - access_size_min = mr->ops->valid.min_access_size; - if (!mr->ops->valid.min_access_size) { - access_size_min = 1; + if (!mr->ops->valid.unaligned && (addr & (size - 1))) { + return false; } - access_size_max = mr->ops->valid.max_access_size; + /* Treat zero as compatibility all valid */ if (!mr->ops->valid.max_access_size) { - access_size_max = 4; + return true; } - access_size = MAX(MIN(size, access_size_max), access_size_min); - for (i = 0; i < size; i += access_size) { - if (!mr->ops->valid.accepts(mr->opaque, addr + i, access_size, - is_write, attrs)) { - return false; - } + if (size > mr->ops->valid.max_access_size + || size < mr->ops->valid.min_access_size) { + return false; } - return true; } -- 2.27.0