[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PATCH] vhost: secure vhost shared log files using argv par
|
From: |
Rafael David Tinoco |
|
Subject: |
[Qemu-devel] [PATCH] vhost: secure vhost shared log files using argv paremeter |
|
Date: |
Sat, 22 Oct 2016 07:00:41 +0000 |
Commit 31190ed7 added a migration blocker in vhost_dev_init() to
check if memfd would succeed. It is better if this blocker first
checks if vhost backend requires shared log. This will avoid a
situation where a blocker is added inappropriately (e.g. shared
log allocation fails when vhost backend doesn't support it).
Commit: 35f9b6e 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.
Also, because vhost log file descriptors can be passed to other
processes, after discussion, we thought it is best to back mmap by
using files that can be placed into a specific directory: this commit
creates "vhostlog" argv parameter for such purpose. This will allow
security drivers to operate on those files appropriately.
Argv examples:
-netdev tap,id=net0,vhost=on
-netdev tap,id=net0,vhost=on,vhostlog=/tmp/guest.log
-netdev tap,id=net0,vhost=on,vhostlog=/tmp
For vhost backends supporting shared logs, if vhostlog is non-existent,
or a directory, random files are going to be created in the specified
directory (or, for non-existent, in tmpdir). If vhostlog is specified,
the filepath is always used when allocating vhost log files.
Signed-off-by: Rafael David Tinoco <address@hidden>
---
hw/net/vhost_net.c | 4 +-
hw/scsi/vhost-scsi.c | 2 +-
hw/virtio/vhost-vsock.c | 2 +-
hw/virtio/vhost.c | 41 +++++++------
include/hw/virtio/vhost.h | 4 +-
include/net/vhost_net.h | 1 +
include/qemu/mmap-file.h | 10 +++
net/tap.c | 6 ++
qapi-schema.json | 3 +
qemu-options.hx | 3 +-
util/Makefile.objs | 1 +
util/mmap-file.c | 153 ++++++++++++++++++++++++++++++++++++++++++++++
12 files changed, 207 insertions(+), 23 deletions(-)
create mode 100644 include/qemu/mmap-file.h
create mode 100644 util/mmap-file.c
diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index f2d49ad..d650c92 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -171,8 +171,8 @@ struct vhost_net *vhost_net_init(VhostNetOptions *options)
net->dev.vq_index = net->nc->queue_index * net->dev.nvqs;
}
- r = vhost_dev_init(&net->dev, options->opaque,
- options->backend_type, options->busyloop_timeout);
+ r = vhost_dev_init(&net->dev, options->opaque, options->backend_type,
+ options->busyloop_timeout, options->vhostlog);
if (r < 0) {
goto fail;
}
diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c
index 5b26946..5dc3d30 100644
--- a/hw/scsi/vhost-scsi.c
+++ b/hw/scsi/vhost-scsi.c
@@ -248,7 +248,7 @@ static void vhost_scsi_realize(DeviceState *dev, Error
**errp)
s->dev.backend_features = 0;
ret = vhost_dev_init(&s->dev, (void *)(uintptr_t)vhostfd,
- VHOST_BACKEND_TYPE_KERNEL, 0);
+ VHOST_BACKEND_TYPE_KERNEL, 0, NULL);
if (ret < 0) {
error_setg(errp, "vhost-scsi: vhost initialization failed: %s",
strerror(-ret));
diff --git a/hw/virtio/vhost-vsock.c b/hw/virtio/vhost-vsock.c
index b481562..6cf6081 100644
--- a/hw/virtio/vhost-vsock.c
+++ b/hw/virtio/vhost-vsock.c
@@ -342,7 +342,7 @@ static void vhost_vsock_device_realize(DeviceState *dev,
Error **errp)
vsock->vhost_dev.nvqs = ARRAY_SIZE(vsock->vhost_vqs);
vsock->vhost_dev.vqs = vsock->vhost_vqs;
ret = vhost_dev_init(&vsock->vhost_dev, (void *)(uintptr_t)vhostfd,
- VHOST_BACKEND_TYPE_KERNEL, 0);
+ VHOST_BACKEND_TYPE_KERNEL, 0, NULL);
if (ret < 0) {
error_setg_errno(errp, -ret, "vhost-vsock: vhost_dev_init failed");
goto err_virtio;
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index bd051ab..d874ebb 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -20,7 +20,7 @@
#include "qemu/atomic.h"
#include "qemu/range.h"
#include "qemu/error-report.h"
-#include "qemu/memfd.h"
+#include "qemu/mmap-file.h"
#include <linux/vhost.h>
#include "exec/address-spaces.h"
#include "hw/virtio/virtio-bus.h"
@@ -326,7 +326,7 @@ static uint64_t vhost_get_log_size(struct vhost_dev *dev)
return log_size;
}
-static struct vhost_log *vhost_log_alloc(uint64_t size, bool share)
+static struct vhost_log *vhost_log_alloc(char *path, uint64_t size, bool share)
{
struct vhost_log *log;
uint64_t logsize = size * sizeof(*(log->log));
@@ -334,9 +334,7 @@ static struct vhost_log *vhost_log_alloc(uint64_t size,
bool share)
log = g_new0(struct vhost_log, 1);
if (share) {
- log->log = qemu_memfd_alloc("vhost-log", logsize,
- F_SEAL_GROW | F_SEAL_SHRINK | F_SEAL_SEAL,
- &fd);
+ log->log = qemu_mmap_alloc(path, logsize, &fd);
memset(log->log, 0, logsize);
} else {
log->log = g_malloc0(logsize);
@@ -349,12 +347,12 @@ static struct vhost_log *vhost_log_alloc(uint64_t size,
bool share)
return log;
}
-static struct vhost_log *vhost_log_get(uint64_t size, bool share)
+static struct vhost_log *vhost_log_get(char *path, uint64_t size, bool share)
{
struct vhost_log *log = share ? vhost_log_shm : vhost_log;
if (!log || log->size != size) {
- log = vhost_log_alloc(size, share);
+ log = vhost_log_alloc(path, size, share);
if (share) {
vhost_log_shm = log;
} else {
@@ -388,8 +386,7 @@ static void vhost_log_put(struct vhost_dev *dev, bool sync)
g_free(log->log);
vhost_log = NULL;
} else if (vhost_log_shm == log) {
- qemu_memfd_free(log->log, log->size * sizeof(*(log->log)),
- log->fd);
+ qemu_mmap_free(log->log, log->size * sizeof(*(log->log)), log->fd);
vhost_log_shm = NULL;
}
@@ -405,9 +402,12 @@ static bool vhost_dev_log_is_shared(struct vhost_dev *dev)
static inline void vhost_dev_log_resize(struct vhost_dev *dev, uint64_t size)
{
- struct vhost_log *log = vhost_log_get(size, vhost_dev_log_is_shared(dev));
- uint64_t log_base = (uintptr_t)log->log;
int r;
+ struct vhost_log *log;
+ uint64_t log_base;
+
+ log = vhost_log_get(dev->log_filename, size, vhost_dev_log_is_shared(dev));
+ log_base = (uintptr_t)log->log;
/* inform backend of log switching, this must be done before
releasing the current log, to ensure no logging is lost */
@@ -1049,7 +1049,8 @@ static void vhost_virtqueue_cleanup(struct
vhost_virtqueue *vq)
}
int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
- VhostBackendType backend_type, uint32_t busyloop_timeout)
+ VhostBackendType backend_type,
+ uint32_t busyloop_timeout, char *vhostlog)
{
uint64_t features;
int i, r, n_initialized_vqs = 0;
@@ -1118,11 +1119,18 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
.priority = 10
};
+ hdev->log = NULL;
+ hdev->log_size = 0;
+ hdev->log_enabled = false;
+ hdev->log_filename = vhostlog ? g_strdup(vhostlog) : NULL;
+ g_free(vhostlog);
+
if (hdev->migration_blocker == NULL) {
if (!(hdev->features & (0x1ULL << VHOST_F_LOG_ALL))) {
error_setg(&hdev->migration_blocker,
"Migration disabled: vhost lacks VHOST_F_LOG_ALL
feature.");
- } else if (!qemu_memfd_check()) {
+ } else if (vhost_dev_log_is_shared(hdev) &&
+ !qemu_mmap_check(hdev->log_filename)) {
error_setg(&hdev->migration_blocker,
"Migration disabled: failed to allocate shared memory");
}
@@ -1135,9 +1143,6 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
hdev->mem = g_malloc0(offsetof(struct vhost_memory, regions));
hdev->n_mem_sections = 0;
hdev->mem_sections = NULL;
- hdev->log = NULL;
- hdev->log_size = 0;
- hdev->log_enabled = false;
hdev->started = false;
hdev->memory_changed = false;
memory_listener_register(&hdev->memory_listener, &address_space_memory);
@@ -1175,6 +1180,7 @@ void vhost_dev_cleanup(struct vhost_dev *hdev)
if (hdev->vhost_ops) {
hdev->vhost_ops->vhost_backend_cleanup(hdev);
}
+ g_free(hdev->log_filename);
assert(!hdev->log);
memset(hdev, 0, sizeof(struct vhost_dev));
@@ -1335,7 +1341,8 @@ int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice
*vdev)
uint64_t log_base;
hdev->log_size = vhost_get_log_size(hdev);
- hdev->log = vhost_log_get(hdev->log_size,
+ hdev->log = vhost_log_get(hdev->log_filename,
+ hdev->log_size,
vhost_dev_log_is_shared(hdev));
log_base = (uintptr_t)hdev->log->log;
r = hdev->vhost_ops->vhost_set_log_base(hdev,
diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
index e433089..1ea4f3a 100644
--- a/include/hw/virtio/vhost.h
+++ b/include/hw/virtio/vhost.h
@@ -52,6 +52,7 @@ struct vhost_dev {
uint64_t max_queues;
bool started;
bool log_enabled;
+ char *log_filename;
uint64_t log_size;
Error *migration_blocker;
bool memory_changed;
@@ -65,7 +66,8 @@ struct vhost_dev {
int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
VhostBackendType backend_type,
- uint32_t busyloop_timeout);
+ uint32_t busyloop_timeout,
+ char *vhostlog);
void vhost_dev_cleanup(struct vhost_dev *hdev);
int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev);
void vhost_dev_stop(struct vhost_dev *hdev, VirtIODevice *vdev);
diff --git a/include/net/vhost_net.h b/include/net/vhost_net.h
index 5a08eff..94161b2 100644
--- a/include/net/vhost_net.h
+++ b/include/net/vhost_net.h
@@ -12,6 +12,7 @@ typedef struct VhostNetOptions {
NetClientState *net_backend;
uint32_t busyloop_timeout;
void *opaque;
+ char *vhostlog;
} VhostNetOptions;
uint64_t vhost_net_get_max_queues(VHostNetState *net);
diff --git a/include/qemu/mmap-file.h b/include/qemu/mmap-file.h
new file mode 100644
index 0000000..427612a
--- /dev/null
+++ b/include/qemu/mmap-file.h
@@ -0,0 +1,10 @@
+#ifndef QEMU_MMAP_FILE_H
+#define QEMU_MMAP_FILE_H
+
+#include "qemu-common.h"
+
+void *qemu_mmap_alloc(const char *path, size_t size, int *fd);
+void qemu_mmap_free(void *ptr, size_t size, int fd);
+bool qemu_mmap_check(const char *path);
+
+#endif
diff --git a/net/tap.c b/net/tap.c
index b6896a7..7b242cd 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -699,6 +699,12 @@ static void net_init_tap_one(const NetdevTapOptions *tap,
NetClientState *peer,
}
options.opaque = (void *)(uintptr_t)vhostfd;
+ if (tap->has_vhostlog) {
+ options.vhostlog = g_strdup(tap->vhostlog);
+ } else {
+ options.vhostlog = NULL;
+ }
+
s->vhost_net = vhost_net_init(&options);
if (!s->vhost_net) {
error_setg(errp,
diff --git a/qapi-schema.json b/qapi-schema.json
index 5a8ec38..72608bd 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2640,6 +2640,8 @@
#
# @vhostforce: #optional vhost on for non-MSIX virtio guests
#
+# @vhostlog: #optional file or directory for vhost backend log
+#
# @queues: #optional number of queues to be created for multiqueue capable tap
#
# @poll-us: #optional maximum number of microseconds that could
@@ -2662,6 +2664,7 @@
'*vhostfd': 'str',
'*vhostfds': 'str',
'*vhostforce': 'bool',
+ '*vhostlog': 'str',
'*queues': 'uint32',
'*poll-us': 'uint32'} }
diff --git a/qemu-options.hx b/qemu-options.hx
index b1fbdb0..5c09c09 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1599,7 +1599,7 @@ DEF("netdev", HAS_ARG, QEMU_OPTION_netdev,
#else
"-netdev
tap,id=str[,fd=h][,fds=x:y:...:z][,ifname=name][,script=file][,downscript=dfile]\n"
"
[,br=bridge][,helper=helper][,sndbuf=nbytes][,vnet_hdr=on|off][,vhost=on|off]\n"
- "
[,vhostfd=h][,vhostfds=x:y:...:z][,vhostforce=on|off][,queues=n]\n"
+ "
[,vhostfd=h][,vhostfds=x:y:...:z][,vhostforce=on|off][,vhostlog=file|dir][,queues=n]\n"
" [,poll-us=n]\n"
" configure a host TAP network backend with ID 'str'\n"
" connected to a bridge (default=" DEFAULT_BRIDGE_INTERFACE
")\n"
@@ -1618,6 +1618,7 @@ DEF("netdev", HAS_ARG, QEMU_OPTION_netdev,
" use vhost=on to enable experimental in kernel
accelerator\n"
" (only has effect for virtio guests which use MSIX)\n"
" use vhostforce=on to force vhost on for non-MSIX virtio
guests\n"
+ " use 'vhostlog=file|dir' file or directory for vhost
backend log\n"
" use 'vhostfd=h' to connect to an already opened vhost net
device\n"
" use 'vhostfds=x:y:...:z to connect to multiple already
opened vhost net devices\n"
" use 'queues=n' to specify the number of queues to be
created for multiqueue TAP\n"
diff --git a/util/Makefile.objs b/util/Makefile.objs
index 36c7dcc..69bb27a 100644
--- a/util/Makefile.objs
+++ b/util/Makefile.objs
@@ -3,6 +3,7 @@ util-obj-y += bufferiszero.o
util-obj-$(CONFIG_POSIX) += compatfd.o
util-obj-$(CONFIG_POSIX) += event_notifier-posix.o
util-obj-$(CONFIG_POSIX) += mmap-alloc.o
+util-obj-$(CONFIG_POSIX) += mmap-file.o
util-obj-$(CONFIG_POSIX) += oslib-posix.o
util-obj-$(CONFIG_POSIX) += qemu-openpty.o
util-obj-$(CONFIG_POSIX) += qemu-thread-posix.o
diff --git a/util/mmap-file.c b/util/mmap-file.c
new file mode 100644
index 0000000..ce778cf
--- /dev/null
+++ b/util/mmap-file.c
@@ -0,0 +1,153 @@
+/*
+ * Support for file backed by mmaped host memory.
+ *
+ * Authors:
+ * Rafael David Tinoco <address@hidden>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * later. See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/mmap-file.h"
+
+static char *qemu_mmap_rand_name(void)
+{
+ char *name;
+ GRand *rsufix;
+ guint32 sufix;
+
+ rsufix = g_rand_new();
+ sufix = g_rand_int(rsufix);
+ g_free(rsufix);
+ name = g_strdup_printf("mmap-%u", sufix);
+
+ return name;
+}
+
+static inline void qemu_mmap_rand_name_free(char *str)
+{
+ g_free(str);
+}
+
+static bool qemu_mmap_is(const char *path, mode_t what)
+{
+ struct stat s;
+
+ memset(&s, 0, sizeof(struct stat));
+ if (stat(path, &s)) {
+ perror("stat");
+ goto negative;
+ }
+
+ if ((s.st_mode & S_IFMT) == what) {
+ return true;
+ }
+
+negative:
+ return false;
+}
+
+static inline bool qemu_mmap_is_file(const char *path)
+{
+ return qemu_mmap_is(path, S_IFREG);
+}
+
+static inline bool qemu_mmap_is_dir(const char *path)
+{
+ return qemu_mmap_is(path, S_IFDIR);
+}
+
+static void *qemu_mmap_alloc_file(const char *filepath, size_t size, int *fd)
+{
+ void *ptr;
+ int mfd = -1;
+
+ *fd = -1;
+
+ mfd = open(filepath, O_CREAT | O_EXCL | O_RDWR, S_IRUSR | S_IWUSR);
+ if (mfd == -1) {
+ perror("open");
+ return NULL;
+ }
+
+ unlink(filepath);
+
+ if (ftruncate(mfd, size) == -1) {
+ perror("ftruncate");
+ close(mfd);
+ return NULL;
+ }
+
+ ptr = mmap(0, size, PROT_READ | PROT_WRITE, MAP_SHARED, mfd, 0);
+ if (ptr == MAP_FAILED) {
+ perror("mmap");
+ close(mfd);
+ return NULL;
+ }
+
+ *fd = mfd;
+ return ptr;
+}
+
+static void *qemu_mmap_alloc_dir(const char *dirpath, size_t size, int *fd)
+{
+ void *ptr;
+ char *file, *rand, *tmp, *dir2use = NULL;
+
+ if (dirpath && !qemu_mmap_is_dir(dirpath)) {
+ return NULL;
+ }
+
+ tmp = (char *) g_get_tmp_dir();
+ dir2use = dirpath ? (char *) dirpath : tmp;
+ rand = qemu_mmap_rand_name();
+ file = g_strdup_printf("%s/%s", dir2use, rand);
+ ptr = qemu_mmap_alloc_file(file, size, fd);
+ g_free(tmp);
+ qemu_mmap_rand_name_free(rand);
+
+ return ptr;
+}
+
+/*
+ * "path" can be:
+ *
+ * filename = full path for the file to back mmap
+ * dir path = full dir path where to create random file for mmap
+ * null = will use <tmpdir> to create random file for mmap
+ */
+void *qemu_mmap_alloc(const char *path, size_t size, int *fd)
+{
+ if (!path || qemu_mmap_is_dir(path)) {
+ return qemu_mmap_alloc_dir(path, size, fd);
+ }
+
+ return qemu_mmap_alloc_file(path, size, fd);
+}
+
+void qemu_mmap_free(void *ptr, size_t size, int fd)
+{
+ if (ptr) {
+ munmap(ptr, size);
+ }
+
+ if (fd != -1) {
+ close(fd);
+ }
+}
+
+bool qemu_mmap_check(const char *path)
+{
+ void *ptr;
+ int fd = -1;
+ bool r = true;
+
+ ptr = qemu_mmap_alloc(path, 4096, &fd);
+ if (!ptr) {
+ r = false;
+ }
+ qemu_mmap_free(ptr, 4096, fd);
+
+ return r == true ? true : false;
+}
--
2.9.3
- [Qemu-devel] [PATCH] vhost: secure vhost shared log files using argv paremeter,
Rafael David Tinoco <=