[RFC PATCH] accel/kvm: respect section RO flag when mapping phys mem

[RFC PATCH] accel/kvm: respect section RO flag when mapping phys mem

[Evgeny Yakovlev]

Currently kvm_set_phys_mem looks at section's underlying memory region
to determine whether mapping is going to be RW or RO. This seems wrong.
For example, when x86 firmware attempts to reprogram q35 PAM registers
to mark bios shadow copy in RAM as RO. In that case we see section->mr
to be writable (pc.ram), but overriding section to be readonly.

This change enforces section's RO to be a priority if underlying memory
region is writable but specific section is not. But not the other way
around, elevating access rights through RW section over RO region
should not be allowed.

Signed-off-by: Evgeny Yakovlev <wrfsh@yandex-team.ru>
---
 accel/kvm/kvm-all.c | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index d2d96d7..6f9ed24 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -407,9 +407,16 @@ err:
  * dirty pages logging control
  */
 
-static int kvm_mem_flags(MemoryRegion *mr)
+static inline bool kvm_is_mem_readonly(MemoryRegionSection *section)
 {
-    bool readonly = mr->readonly || memory_region_is_romd(mr);
+    MemoryRegion *mr = section->mr;
+    return mr->readonly || memory_region_is_romd(mr) || section->readonly;
+}
+
+static int kvm_mem_flags(MemoryRegionSection *section)
+{
+    MemoryRegion *mr = section->mr;
+    bool readonly = kvm_is_mem_readonly(section);
     int flags = 0;
 
     if (memory_region_get_dirty_log_mask(mr) != 0) {
@@ -423,9 +430,9 @@ static int kvm_mem_flags(MemoryRegion *mr)
 
 /* Called with KVMMemoryListener.slots_lock held */
 static int kvm_slot_update_flags(KVMMemoryListener *kml, KVMSlot *mem,
-                                 MemoryRegion *mr)
+                                 MemoryRegionSection *section)
 {
-    mem->flags = kvm_mem_flags(mr);
+    mem->flags = kvm_mem_flags(section);
 
     /* If nothing changed effectively, no need to issue ioctl */
     if (mem->flags == mem->old_flags) {
@@ -457,7 +464,7 @@ static int kvm_section_update_flags(KVMMemoryListener *kml,
             goto out;
         }
 
-        ret = kvm_slot_update_flags(kml, mem, section->mr);
+        ret = kvm_slot_update_flags(kml, mem, section);
         start_addr += slot_size;
         size -= slot_size;
     }
@@ -1002,7 +1009,7 @@ static void kvm_set_phys_mem(KVMMemoryListener *kml,
     KVMSlot *mem;
     int err;
     MemoryRegion *mr = section->mr;
-    bool writeable = !mr->readonly && !mr->rom_device;
+    bool writeable = !kvm_is_mem_readonly(section);
     hwaddr start_addr, size, slot_size;
     void *ram;
 
@@ -1062,7 +1069,7 @@ static void kvm_set_phys_mem(KVMMemoryListener *kml,
         mem->memory_size = slot_size;
         mem->start_addr = start_addr;
         mem->ram = ram;
-        mem->flags = kvm_mem_flags(mr);
+        mem->flags = kvm_mem_flags(section);
 
         err = kvm_set_user_memory_region(kml, mem, true);
         if (err) {
-- 
2.7.4