Blame SOURCES/kvm-memory-Revert-memory-accept-mismatching-sizes-in-mem.patch

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