qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] util: secure memfd_create fallback mechanism
@ 2016-09-27  3:19 Rafael David Tinoco
  2016-09-27  4:20 ` no-reply
  0 siblings, 1 reply; 10+ messages in thread
From: Rafael David Tinoco @ 2016-09-27  3:19 UTC (permalink / raw)
  To: mst; +Cc: qemu-devel, 1626972, rafael.tinoco, marcandre.lureau

Commit: 35f9b6ef3acc9d0546c395a566b04e63ca84e302 added a fallback
mechanism for systems not supporting memfd_create syscall (started
being supported since 3.17).

Backporting memfd_create might not be accepted for distros relying
on older kernels. Nowadays there is no way for security driver
to discover memfd filename to be created: <tmpdir>/memfd-XXXXXX.

It is more appropriate to include UUID and/or VM names in the
temporary filename, allowing security driver rules to be applied
while maintaining the required unpredictability with mkstemp.

This change will allow libvirt to know exact memfd file to be created
for vhost log AND to create appropriate security rules to allow access
per instance (instead of a opened rule like <tmpdir>/memfd-*).

Example of apparmor deny messages with this change:

Per VM UUID (preferred, generated automatically by libvirt):

kernel: [26632.154856] type=1400 audit(1474945148.633:78): apparmor=
"DENIED" operation="mknod" profile="libvirt-0b96011f-0dc0-44a3-92c3-
196de2efab6d" name="/tmp/memfd-0b96011f-0dc0-44a3-92c3-196de2efab6d-
qeHrBV" pid=75161 comm="qemu-system-x86" requested_mask="c" denied_
mask="c" fsuid=107 ouid=107

Per VM name (if no UUID is specified):

kernel: [26447.505653] type=1400 audit(1474944963.985:72): apparmor=
"DENIED" operation="mknod" profile="libvirt-00000000-0000-0000-0000-
000000000000" name="/tmp/memfd-instance-teste-osYpHh" pid=74648
comm="qemu-system-x86" requested_mask="c" denied_mask="c" fsuid=107
ouid=107

Signed-off-by: Rafael David Tinoco <rafael.tinoco@canonical.com>
---
 util/memfd.c | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/util/memfd.c b/util/memfd.c
index 4571d1a..4b715ac 100644
--- a/util/memfd.c
+++ b/util/memfd.c
@@ -30,6 +30,9 @@
 #include <glib/gprintf.h>
 
 #include "qemu/memfd.h"
+#include "qmp-commands.h"
+#include "qemu-common.h"
+#include "sysemu/sysemu.h"
 
 #ifdef CONFIG_MEMFD
 #include <sys/memfd.h>
@@ -94,11 +97,32 @@ void *qemu_memfd_alloc(const char *name, size_t size, unsigned int seals,
             return NULL;
         }
     } else {
+        int ret = 0;
         const char *tmpdir = g_get_tmp_dir();
+        UuidInfo *uinfo;
+        NameInfo *ninfo;
         gchar *fname;
 
-        fname = g_strdup_printf("%s/memfd-XXXXXX", tmpdir);
+        uinfo = qmp_query_uuid(NULL);
+
+        ret = strcmp(uinfo->UUID, UUID_NONE);
+        if (ret == 0) {
+            ninfo = qmp_query_name(NULL);
+            if (ninfo->has_name) {
+                fname = g_strdup_printf("%s/memfd-%s-XXXXXX", tmpdir,
+                                        ninfo->name);
+            } else {
+                fname = g_strdup_printf("%s/memfd-XXXXXX", tmpdir);
+            }
+            qapi_free_NameInfo(ninfo);
+        } else {
+            fname = g_strdup_printf("%s/memfd-%s-XXXXXX", tmpdir,
+                                    uinfo->UUID);
+        }
+
         mfd = mkstemp(fname);
+
+        qapi_free_UuidInfo(uinfo);
         unlink(fname);
         g_free(fname);
 
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH] util: secure memfd_create fallback mechanism
@ 2016-09-27  3:06 Rafael David Tinoco
  2016-09-27  8:36 ` Daniel P. Berrange
  0 siblings, 1 reply; 10+ messages in thread
From: Rafael David Tinoco @ 2016-09-27  3:06 UTC (permalink / raw)
  To: qemu-devel; +Cc: 1626972, mst, marcandre.lureau

Commit: 35f9b6ef3acc9d0546c395a566b04e63ca84e302 added a fallback
mechanism for systems not supporting memfd_create syscall (started
being supported since 3.17).

Backporting memfd_create might not be accepted for distros relying
on older kernels. Nowadays there is no way for security driver
to discover memfd filename to be created: <tmpdir>/memfd-XXXXXX.

It is more appropriate to include UUID and/or VM names in the
temporary filename, allowing security driver rules to be applied
while maintaining the required unpredictability with mkstemp.

This change will allow libvirt to know exact memfd file to be created
for vhost log AND to create appropriate security rules to allow access
per instance (instead of a opened rule like <tmpdir>/memfd-*).

Example of apparmor deny messages with this change:

Per VM UUID (preferred, generated automatically by libvirt):

kernel: [26632.154856] type=1400 audit(1474945148.633:78): apparmor=
"DENIED" operation="mknod" profile="libvirt-0b96011f-0dc0-44a3-92c3-
196de2efab6d" name="/tmp/memfd-0b96011f-0dc0-44a3-92c3-196de2efab6d-
qeHrBV" pid=75161 comm="qemu-system-x86" requested_mask="c" denied_
mask="c" fsuid=107 ouid=107

Per VM name (if no UUID is specified):

kernel: [26447.505653] type=1400 audit(1474944963.985:72): apparmor=
"DENIED" operation="mknod" profile="libvirt-00000000-0000-0000-0000-
000000000000" name="/tmp/memfd-instance-teste-osYpHh" pid=74648
comm="qemu-system-x86" requested_mask="c" denied_mask="c" fsuid=107
ouid=107

Signed-off-by: Rafael David Tinoco <rafael.tinoco@canonical.com>
---
 util/memfd.c | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/util/memfd.c b/util/memfd.c
index 4571d1a..4b715ac 100644
--- a/util/memfd.c
+++ b/util/memfd.c
@@ -30,6 +30,9 @@
 #include <glib/gprintf.h>
 
 #include "qemu/memfd.h"
+#include "qmp-commands.h"
+#include "qemu-common.h"
+#include "sysemu/sysemu.h"
 
 #ifdef CONFIG_MEMFD
 #include <sys/memfd.h>
@@ -94,11 +97,32 @@ void *qemu_memfd_alloc(const char *name, size_t size, unsigned int seals,
             return NULL;
         }
     } else {
+        int ret = 0;
         const char *tmpdir = g_get_tmp_dir();
+        UuidInfo *uinfo;
+        NameInfo *ninfo;
         gchar *fname;
 
-        fname = g_strdup_printf("%s/memfd-XXXXXX", tmpdir);
+        uinfo = qmp_query_uuid(NULL);
+
+        ret = strcmp(uinfo->UUID, UUID_NONE);
+        if (ret == 0) {
+            ninfo = qmp_query_name(NULL);
+            if (ninfo->has_name) {
+                fname = g_strdup_printf("%s/memfd-%s-XXXXXX", tmpdir,
+                                        ninfo->name);
+            } else {
+                fname = g_strdup_printf("%s/memfd-XXXXXX", tmpdir);
+            }
+            qapi_free_NameInfo(ninfo);
+        } else {
+            fname = g_strdup_printf("%s/memfd-%s-XXXXXX", tmpdir,
+                                    uinfo->UUID);
+        }
+
         mfd = mkstemp(fname);
+
+        qapi_free_UuidInfo(uinfo);
         unlink(fname);
         g_free(fname);
 
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 10+ messages in thread
end of thread, other threads:[~2016-10-03 17:39 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-09-27  3:19 [Qemu-devel] [PATCH] util: secure memfd_create fallback mechanism Rafael David Tinoco
2016-09-27  4:20 ` no-reply
  -- strict thread matches above, loose matches on Subject: below --
2016-09-27  3:06 Rafael David Tinoco
2016-09-27  8:36 ` Daniel P. Berrange
2016-09-27 11:01   ` Rafael David Tinoco
2016-09-27 11:13     ` Marc-André Lureau
2016-09-27 11:31       ` Rafael David Tinoco
2016-09-27 12:16       ` Daniel P. Berrange
2016-09-27 12:25         ` Marc-André Lureau
2016-10-03 17:38       ` Rafael David Tinoco

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).