nvdimm: Reject writing label data to ROM instead of crashing QEMU

Currently, when using a true R/O NVDIMM (ROM memory backend) with a label
area, the VM can easily crash QEMU by trying to write to the label area,
because the ROM memory is mmap'ed without PROT_WRITE.

    [root@vm-0 ~]# ndctl disable-region region0
    disabled 1 region
    [root@vm-0 ~]# ndctl zero-labels nmem0
    -> QEMU segfaults

Let's remember whether we have a ROM memory backend and properly
reject the write request:

    [root@vm-0 ~]# ndctl disable-region region0
    disabled 1 region
    [root@vm-0 ~]# ndctl zero-labels nmem0
    zeroed 0 nmem

In comparison, on a system with a R/W NVDIMM:

    [root@vm-0 ~]# ndctl disable-region region0
    disabled 1 region
    [root@vm-0 ~]# ndctl zero-labels nmem0
    zeroed 1 nmem

For ACPI, just return "unsupported", like if no label exists. For spapr,
return "H_P2", similar to when no label area exists.

Could we rely on the "unarmed" property? Maybe, but it looks cleaner to
only disallow what certainly cannot work.

After all "unarmed=on" primarily means: cannot accept persistent writes. In
theory, there might be setups where devices with "unarmed=on" set could
be used to host non-persistent data (temporary files, system RAM, ...); for
example, in Linux, admins can overwrite the "readonly" setting and still
write to the device -- which will work as long as we're not using ROM.
Allowing writing label data in such configurations can make sense.

Message-ID: <20230906120503.359863-2-david@redhat.com>
Fixes: dbd730e85987 ("nvdimm: check -object memory-backend-file, readonly=on option")
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: David Hildenbrand <david@redhat.com>

1 files changed, 7 insertions, 3 deletions

diff --git a/hw/mem/nvdimm.c b/hw/mem/nvdimm.c
index 31080c2..1631a7d 100644
--- a/hw/mem/nvdimm.c
+++ b/hw/mem/nvdimm.c
@@ -154,6 +154,9 @@ static void nvdimm_prepare_memory_region(NVDIMMDevice *nvdimm, Error **errp)
                    object_get_canonical_path_component(OBJECT(hostmem)));
         return;
     }
+    if (memory_region_is_rom(mr)) {
+        nvdimm->readonly = true;
+    }
 
     nvdimm->nvdimm_mr = g_new(MemoryRegion, 1);
     memory_region_init_alias(nvdimm->nvdimm_mr, OBJECT(dimm),
@@ -207,15 +210,16 @@ static void nvdimm_unrealize(PCDIMMDevice *dimm)
  * label read/write functions.
  */
 static void nvdimm_validate_rw_label_data(NVDIMMDevice *nvdimm, uint64_t size,
-                                        uint64_t offset)
+                                        uint64_t offset, bool is_write)
 {
     assert((nvdimm->label_size >= size + offset) && (offset + size > offset));
+    assert(!is_write || !nvdimm->readonly);
 }
 
 static void nvdimm_read_label_data(NVDIMMDevice *nvdimm, void *buf,
                                    uint64_t size, uint64_t offset)
 {
-    nvdimm_validate_rw_label_data(nvdimm, size, offset);
+    nvdimm_validate_rw_label_data(nvdimm, size, offset, false);
 
     memcpy(buf, nvdimm->label_data + offset, size);
 }
@@ -229,7 +233,7 @@ static void nvdimm_write_label_data(NVDIMMDevice *nvdimm, const void *buf,
                                             "pmem", NULL);
     uint64_t backend_offset;
 
-    nvdimm_validate_rw_label_data(nvdimm, size, offset);
+    nvdimm_validate_rw_label_data(nvdimm, size, offset, true);
 
     if (!is_pmem) {
         memcpy(nvdimm->label_data + offset, buf, size);