[Qemu-devel] [RFC] memory-backend-file/nvdimm: support read-only files as memory-backends

Julio Montes posted 1 patch 4 years, 9 months ago
Test docker-clang@ubuntu passed
Test s390x passed
Test asan passed
Test docker-mingw@fedora passed
Test FreeBSD passed
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20190708211936.8037-1-julio.montes@intel.com
Maintainers: Paolo Bonzini <pbonzini@redhat.com>, Richard Henderson <rth@twiddle.net>
exec.c | 6 ++++++
1 file changed, 6 insertions(+)
[Qemu-devel] [RFC] memory-backend-file/nvdimm: support read-only files as memory-backends
Posted by Julio Montes 4 years, 9 months ago
Currently is not possible to use a file that is part of a read-only
filesystem as memory backend for nvdimm devices, even if this is not modified
in the guest. In order to improve the security of Virtual Machines that share
and do not modify the memory-backend-file, QEMU should support
read-only memory-backeds.

Use case:
* Kata Containers use a memory-backed-file as read-only rootfs, and this
  file is used to start all the virtual machines in the node.
  It would be really bad if somehow a malicious container modified it.

Signed-off-by: Julio Montes <julio.montes@intel.com>
---
 exec.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/exec.c b/exec.c
index 50ea9c5aaa..1eb170b55a 100644
--- a/exec.c
+++ b/exec.c
@@ -1852,6 +1852,12 @@ static int file_ram_open(const char *path,
                 break;
             }
             g_free(filename);
+        } else if (errno == EROFS) {
+            fd = open(path, O_RDONLY);
+            if (fd >= 0) {
+                /* @path names an existing read-only file, use it */
+                break;
+            }
         }
         if (errno != EEXIST && errno != EINTR) {
             error_setg_errno(errp, errno,
--
2.17.2

Re: [Qemu-devel] [RFC] memory-backend-file/nvdimm: support read-only files as memory-backends
Posted by Philippe Mathieu-Daudé 4 years, 9 months ago
Cc'ing Igor & Xiao.

On 7/8/19 11:19 PM, Julio Montes wrote:
> Currently is not possible to use a file that is part of a read-only
> filesystem as memory backend for nvdimm devices, even if this is not modified
> in the guest. In order to improve the security of Virtual Machines that share
> and do not modify the memory-backend-file, QEMU should support
> read-only memory-backeds.
> 
> Use case:
> * Kata Containers use a memory-backed-file as read-only rootfs, and this
>   file is used to start all the virtual machines in the node.
>   It would be really bad if somehow a malicious container modified it.
> 
> Signed-off-by: Julio Montes <julio.montes@intel.com>
> ---
>  exec.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/exec.c b/exec.c
> index 50ea9c5aaa..1eb170b55a 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -1852,6 +1852,12 @@ static int file_ram_open(const char *path,
>                  break;
>              }
>              g_free(filename);
> +        } else if (errno == EROFS) {
> +            fd = open(path, O_RDONLY);

While I can understand your use case, I'm not sure we want this silenced
as default. I'd expect an explicit command line option for that backend,
but I don't know well this area so let's wait for other to review.

> +            if (fd >= 0) {
> +                /* @path names an existing read-only file, use it */
> +                break;
> +            }
>          }
>          if (errno != EEXIST && errno != EINTR) {
>              error_setg_errno(errp, errno,
> --
> 2.17.2
> 

Re: [Qemu-devel] [RFC] memory-backend-file/nvdimm: support read-only files as memory-backends
Posted by Stefan Hajnoczi 4 years, 9 months ago
On Tue, Jul 09, 2019 at 12:53:45PM +0200, Philippe Mathieu-Daudé wrote:
> Cc'ing Igor & Xiao.
> 
> On 7/8/19 11:19 PM, Julio Montes wrote:
> > Currently is not possible to use a file that is part of a read-only
> > filesystem as memory backend for nvdimm devices, even if this is not modified
> > in the guest. In order to improve the security of Virtual Machines that share
> > and do not modify the memory-backend-file, QEMU should support
> > read-only memory-backeds.
> > 
> > Use case:
> > * Kata Containers use a memory-backed-file as read-only rootfs, and this
> >   file is used to start all the virtual machines in the node.
> >   It would be really bad if somehow a malicious container modified it.
> > 
> > Signed-off-by: Julio Montes <julio.montes@intel.com>
> > ---
> >  exec.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/exec.c b/exec.c
> > index 50ea9c5aaa..1eb170b55a 100644
> > --- a/exec.c
> > +++ b/exec.c
> > @@ -1852,6 +1852,12 @@ static int file_ram_open(const char *path,
> >                  break;
> >              }
> >              g_free(filename);
> > +        } else if (errno == EROFS) {
> > +            fd = open(path, O_RDONLY);
> 
> While I can understand your use case, I'm not sure we want this silenced
> as default. I'd expect an explicit command line option for that backend,
> but I don't know well this area so let's wait for other to review.

I also wonder whether read-only should be exposed to the guest (e.g.
ACPI NFIT SPA EFI_MEMORY_WP Address Range Memory Mapping Attribute or
ACPI NFIT NVDIMM Region Mapping Structure NVDIMM State Flags Bit 3).

Stefan