From: Marc-André Lureau <marcandre.lureau@redhat.com>

ramfb_create_display_surface() passes the guest-controlled address
straight to physical_memory_map() and only checks whether the returned
length matches the requested size.  Two problems follow.

An address backed by a non-directly-mappable region (device MMIO, an
unassigned hole, ...) is bounced through a temporary host buffer.
Bouncing is pointless for a framebuffer that is only ever read, and it
consumes the limited address_space_memory bounce buffer budget.

When address_space_map() cannot obtain bounce buffer space it returns
NULL with *plen set to 0.  The size check then fails and the function
calls physical_memory_unmap(NULL, 0, ...).  address_space_unmap() treats
the NULL pointer as a BounceBuffer and dereferences its magic field,
crashing QEMU.

A guest can trigger the latter by writing the same configuration twice
via etc/ramfb: the first 16x64 XRGB8888 surface at an unmapped GPA
consumes the entire 4096 byte bounce buffer allowance, and the second
mapping fails because that surface is still being displayed.

Validate up front that the whole framebuffer range is backed by
directly accessible memory, and still return early if the mapping
fails, leaving the previous surface in place.

Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4740
Reported-by: Karl Mehltretter <https://gitlab.com/kmehltretter>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 hw/display/ramfb.c      | 38 ++++++++++++++++++++++++++++++++++++--
 hw/display/trace-events |  4 ++++
 2 files changed, 40 insertions(+), 2 deletions(-)

diff --git a/hw/display/ramfb.c b/hw/display/ramfb.c
index f477bdcc2186..8e2be33b0fcc 100644
--- a/hw/display/ramfb.c
+++ b/hw/display/ramfb.c
@@ -17,8 +17,10 @@
 #include "hw/display/ramfb.h"
 #include "hw/display/bochs-vbe.h" /* for limits */
 #include "ui/console.h"
+#include "system/memory.h"
 #include "system/physmem.h"
 #include "system/reset.h"
+#include "trace.h"
 #include "exec/cpu-common.h"
 
 struct QEMU_PACKED RAMFBCfg {
@@ -46,6 +48,31 @@ static void ramfb_unmap_display_surface(pixman_image_t *image, void *unused)
     physical_memory_unmap(data, size, 0, 0);
 }
 
+/*
+ * A framebuffer must live in directly mappable host memory. Reject
+ * addresses backed by unrelated regions (MMIO, unassigned holes, ...)
+ */
+static bool ramfb_range_is_direct_access(hwaddr addr, hwaddr size)
+{
+    WITH_RCU_READ_LOCK_GUARD() {
+        while (size > 0) {
+            MemoryRegion *mr;
+            hwaddr len = size, xlat;
+
+            mr = address_space_translate(&address_space_memory, addr,
+                                         &xlat, &len, false,
+                                         MEMTXATTRS_UNSPECIFIED);
+            if (len == 0 || !memory_region_supports_direct_access(mr)) {
+                return false;
+            }
+            addr += len;
+            size -= len;
+        }
+    }
+
+    return true;
+}
+
 static DisplaySurface *ramfb_create_display_surface(int width, int height,
                                                     pixman_format_code_t format,
                                                     hwaddr stride, hwaddr addr)
@@ -65,9 +92,16 @@ static DisplaySurface *ramfb_create_display_surface(int width, int height,
     }
 
     mapsize = size = stride * (height - 1) + linesize;
+    if (!ramfb_range_is_direct_access(addr, size)) {
+        trace_ramfb_bad_range(addr, size);
+        return NULL;
+    }
     data = physical_memory_map(addr, &mapsize, false);
-    if (size != mapsize) {
-        physical_memory_unmap(data, mapsize, 0, 0);
+    if (!data || size != mapsize) {
+        trace_ramfb_map_failed(addr, size, mapsize);
+        if (data) {
+            physical_memory_unmap(data, mapsize, 0, 0);
+        }
         return NULL;
     }
 
diff --git a/hw/display/trace-events b/hw/display/trace-events
index c5e7e42af23b..bc1822b6b023 100644
--- a/hw/display/trace-events
+++ b/hw/display/trace-events
@@ -184,6 +184,10 @@ macfb_sense_read(uint32_t value) "video sense: 0x%"PRIx32
 macfb_sense_write(uint32_t value) "video sense: 0x%"PRIx32
 macfb_update_mode(uint32_t width, uint32_t height, uint8_t depth) "setting mode to width %"PRId32 " height %"PRId32 " size %d"
 
+# ramfb.c
+ramfb_bad_range(uint64_t addr, uint64_t size) "addr=0x%"PRIx64" size=0x%"PRIx64
+ramfb_map_failed(uint64_t addr, uint64_t size, uint64_t mapsize) "addr=0x%"PRIx64" size=0x%"PRIx64" mapped=0x%"PRIx64
+
 # dm163.c
 dm163_redraw(uint8_t redraw) "0x%02x"
 dm163_dck(unsigned new_state) "dck : %u"
-- 
2.56.0.116.g6de20f6092dc