[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 1/9] nvdimm: Reject writing label data to ROM instead of c
From: |
Stefan Hajnoczi |
Subject: |
Re: [PATCH v2 1/9] nvdimm: Reject writing label data to ROM instead of crashing QEMU |
Date: |
Tue, 22 Aug 2023 15:25:39 -0400 |
On Tue, Aug 22, 2023 at 01:44:49PM +0200, David Hildenbrand wrote:
> 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.
>
> Fixes: dbd730e85987 ("nvdimm: check -object memory-backend-file, readonly=on
> option")
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
> hw/acpi/nvdimm.c | 11 ++++++++---
> hw/mem/nvdimm.c | 10 +++++++---
> hw/ppc/spapr_nvdimm.c | 3 ++-
> include/hw/mem/nvdimm.h | 6 ++++++
> 4 files changed, 23 insertions(+), 7 deletions(-)
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
signature.asc
Description: PGP signature