aboutsummaryrefslogtreecommitdiff
path: root/hw/acpi
diff options
context:
space:
mode:
authorDavid Hildenbrand <david@redhat.com>2023-09-06 14:04:53 +0200
committerDavid Hildenbrand <david@redhat.com>2023-09-19 10:23:21 +0200
commit3a1258399bdf4d4412cbfde36d0d94965eec87b6 (patch)
tree651d96d9bdeb511144c73e85cc54cf6839313d29 /hw/acpi
parent13d6b1608160de40ec65ae4c32419e56714bbadf (diff)
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>
Diffstat (limited to 'hw/acpi')
-rw-r--r--hw/acpi/nvdimm.c11
1 files changed, 8 insertions, 3 deletions
diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
index a3b25a92f3..3cbd41629d 100644
--- a/hw/acpi/nvdimm.c
+++ b/hw/acpi/nvdimm.c
@@ -670,7 +670,8 @@ static void nvdimm_dsm_label_size(NVDIMMDevice *nvdimm, hwaddr dsm_mem_addr)
}
static uint32_t nvdimm_rw_label_data_check(NVDIMMDevice *nvdimm,
- uint32_t offset, uint32_t length)
+ uint32_t offset, uint32_t length,
+ bool is_write)
{
uint32_t ret = NVDIMM_DSM_RET_STATUS_INVALID;
@@ -690,6 +691,10 @@ static uint32_t nvdimm_rw_label_data_check(NVDIMMDevice *nvdimm,
return ret;
}
+ if (is_write && nvdimm->readonly) {
+ return NVDIMM_DSM_RET_STATUS_UNSUPPORT;
+ }
+
return NVDIMM_DSM_RET_STATUS_SUCCESS;
}
@@ -713,7 +718,7 @@ static void nvdimm_dsm_get_label_data(NVDIMMDevice *nvdimm, NvdimmDsmIn *in,
get_label_data->length);
status = nvdimm_rw_label_data_check(nvdimm, get_label_data->offset,
- get_label_data->length);
+ get_label_data->length, false);
if (status != NVDIMM_DSM_RET_STATUS_SUCCESS) {
nvdimm_dsm_no_payload(status, dsm_mem_addr);
return;
@@ -752,7 +757,7 @@ static void nvdimm_dsm_set_label_data(NVDIMMDevice *nvdimm, NvdimmDsmIn *in,
set_label_data->length);
status = nvdimm_rw_label_data_check(nvdimm, set_label_data->offset,
- set_label_data->length);
+ set_label_data->length, true);
if (status != NVDIMM_DSM_RET_STATUS_SUCCESS) {
nvdimm_dsm_no_payload(status, dsm_mem_addr);
return;