[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [RFC PATCH] virtio-9p: Use clone approach to fix TOCTOU vul
From: |
M. Mohan Kumar |
Subject: |
[Qemu-devel] [RFC PATCH] virtio-9p: Use clone approach to fix TOCTOU vulnerability |
Date: |
Tue, 14 Jun 2011 13:42:45 +0530 |
User-agent: |
Mutt/1.5.19 (2009-01-05) |
[RFC PATCH] virtio-9p: Use clone approach to fix TOCTOU vulnerability
In passthrough security model, following a symbolic link in the server
side could result in TOCTTOU vulnerability.
Use clone system call to create a thread which runs in chrooted
environment. All passthrough model file operations are done from this
thread to avoid TOCTTOU vulnerability.
Signed-off-by: Venkateswararao Jujjuri <address@hidden>
Signed-off-by: M. Mohan Kumar <address@hidden>
---
fsdev/file-op-9p.h | 1 +
hw/9pfs/virtio-9p-coth.c | 105 +++++++++++++++++++++++++++++++++++++++++--
hw/9pfs/virtio-9p-coth.h | 13 +++++-
hw/9pfs/virtio-9p-device.c | 7 +++-
hw/9pfs/virtio-9p.h | 6 ++-
5 files changed, 124 insertions(+), 8 deletions(-)
diff --git a/fsdev/file-op-9p.h b/fsdev/file-op-9p.h
index 5d088d4..c54f6bf 100644
--- a/fsdev/file-op-9p.h
+++ b/fsdev/file-op-9p.h
@@ -60,6 +60,7 @@ typedef struct FsContext
SecModel fs_sm;
uid_t uid;
int open_flags;
+ int enable_chroot;
struct xattr_operations **xops;
/* fs driver specific data */
void *private;
diff --git a/hw/9pfs/virtio-9p-coth.c b/hw/9pfs/virtio-9p-coth.c
index e61b656..aa71a83 100644
--- a/hw/9pfs/virtio-9p-coth.c
+++ b/hw/9pfs/virtio-9p-coth.c
@@ -17,6 +17,8 @@
#include "qemu-thread.h"
#include "qemu-coroutine.h"
#include "virtio-9p-coth.h"
+#include <sys/syscall.h>
+#include "qemu-error.h"
/* v9fs glib thread pool */
static V9fsThPool v9fs_pool;
@@ -56,7 +58,96 @@ static void v9fs_thread_routine(gpointer data, gpointer
user_data)
} while (len == -1 && errno == EINTR);
}
-int v9fs_init_worker_threads(void)
+static int v9fs_chroot_function(void *arg)
+{
+ GError *err;
+ V9fsChrootState *csp = arg;
+ FsContext *fs_ctx = csp->fs_ctx;
+ V9fsThPool *p = &v9fs_pool;
+ int notifier_fds[2];
+
+ if (chroot(fs_ctx->fs_root) < 0) {
+ error_report("chroot");
+ goto error;
+ }
+
+ if (qemu_pipe(notifier_fds) == -1) {
+ return -1;
+ }
+ p->pool = g_thread_pool_new(v9fs_thread_routine, p, 10, TRUE, &err);
+ if (!p->pool) {
+ error_report("g_thread_pool_new");
+ goto error;
+ }
+
+ p->completed = g_async_queue_new();
+ if (!p->completed) {
+ /*
+ * We are going to terminate.
+ * So don't worry about cleanup
+ */
+ goto error;
+ }
+ p->rfd = notifier_fds[0];
+ p->wfd = notifier_fds[1];
+
+ fcntl(p->rfd, F_SETFL, O_NONBLOCK);
+ fcntl(p->wfd, F_SETFL, O_NONBLOCK);
+
+ qemu_set_fd_handler(p->rfd, v9fs_qemu_process_req_done, NULL, NULL);
+
+ /* Signal parent thread that thread pools are created */
+ g_cond_signal(csp->cond);
+ g_mutex_lock(csp->mutex_term);
+ /* TODO: Wake up cs->terminate during 9p hotplug support */
+ g_cond_wait(csp->terminate, csp->mutex);
+ g_mutex_unlock(csp->mutex_term);
+ g_mutex_free(csp->mutex);
+ g_cond_free(csp->cond);
+ g_mutex_free(csp->mutex_term);
+ g_cond_free(csp->terminate);
+ qemu_free(csp->stack);
+ qemu_free(csp);
+ return 0;
+error:
+ p->pool = NULL;
+ g_cond_signal(csp->cond);
+ return 0;
+}
+
+static int v9fs_clone_chroot_th(FsContext *fs_ctx)
+{
+ pid_t pid;
+ V9fsChrootState *cs;
+
+ cs = qemu_malloc(sizeof(*cs));
+ cs->stack = qemu_malloc(THREAD_STACK);
+ cs->mutex = g_mutex_new();
+ cs->cond = g_cond_new();
+ cs->mutex_term = g_mutex_new();
+ cs->terminate = g_cond_new();
+ cs->fs_ctx = fs_ctx;
+
+ g_mutex_lock(cs->mutex);
+ pid = clone(&v9fs_chroot_function, cs->stack + THREAD_STACK, CLONE_FILES |
+ CLONE_SIGHAND | CLONE_VM | CLONE_THREAD, cs);
+ if (pid == -1) {
+ error_report("clone");
+ g_mutex_unlock(cs->mutex);
+ g_mutex_free(cs->mutex);
+ g_cond_free(cs->cond);
+ g_mutex_free(cs->mutex_term);
+ g_cond_free(cs->terminate);
+ qemu_free(cs->stack);
+ qemu_free(cs);
+ return pid;
+ }
+ g_cond_wait(cs->cond, cs->mutex);
+ g_mutex_unlock(cs->mutex);
+ return pid;
+}
+
+int v9fs_init_worker_threads(FsContext *fs_ctx)
{
int notifier_fds[2];
V9fsThPool *p = &v9fs_pool;
@@ -66,14 +157,18 @@ int v9fs_init_worker_threads(void)
g_thread_init(NULL);
}
- if (qemu_pipe(notifier_fds) == -1) {
- return -1;
+ if (fs_ctx->enable_chroot) {
+ return v9fs_clone_chroot_th(fs_ctx);
+ } else {
+ p->pool = g_thread_pool_new(v9fs_thread_routine, p, -1, FALSE, NULL);
}
-
- p->pool = g_thread_pool_new(v9fs_thread_routine, p, -1, FALSE, NULL);
if (!p->pool) {
return -1;
}
+ if (qemu_pipe(notifier_fds) == -1) {
+ return -1;
+ }
+
p->completed = g_async_queue_new();
if (!p->completed) {
/*
diff --git a/hw/9pfs/virtio-9p-coth.h b/hw/9pfs/virtio-9p-coth.h
index 4630080..422354a 100644
--- a/hw/9pfs/virtio-9p-coth.h
+++ b/hw/9pfs/virtio-9p-coth.h
@@ -20,6 +20,8 @@
#include "virtio-9p.h"
#include <glib.h>
+#define THREAD_STACK (16 * 1024)
+
typedef struct V9fsThPool {
int rfd;
int wfd;
@@ -27,6 +29,15 @@ typedef struct V9fsThPool {
GAsyncQueue *completed;
} V9fsThPool;
+typedef struct V9fsChrootState {
+ FsContext *fs_ctx;
+ GMutex *mutex;
+ GCond *cond;
+ GMutex *mutex_term;
+ GCond *terminate;
+ void *stack;
+} V9fsChrootState;
+
/*
* we want to use bottom half because we want to make sure the below
* sequence of events.
@@ -55,7 +66,7 @@ typedef struct V9fsThPool {
} while (0)
extern void co_run_in_worker_bh(void *);
-extern int v9fs_init_worker_threads(void);
+extern int v9fs_init_worker_threads(FsContext *fs_ctx);
extern int v9fs_co_readlink(V9fsPDU *, V9fsPath *, V9fsString *);
extern int v9fs_co_readdir_r(V9fsPDU *, V9fsFidState *,
struct dirent *, struct dirent **result);
diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
index c7a7f44..5e2cc5a 100644
--- a/hw/9pfs/virtio-9p-device.c
+++ b/hw/9pfs/virtio-9p-device.c
@@ -82,10 +82,15 @@ VirtIODevice *virtio_9p_init(DeviceState *dev, V9fsConf
*conf)
exit(1);
}
+ s->ctx.enable_chroot = 0;
+
if (!strcmp(fse->security_model, "passthrough")) {
/* Files on the Fileserver set to client user credentials */
s->ctx.fs_sm = SM_PASSTHROUGH;
s->ctx.xops = passthrough_xattr_ops;
+
+ /* TODO: Add a configuration option to enable this */
+ s->ctx.enable_chroot = 1;
} else if (!strcmp(fse->security_model, "mapped")) {
/* Files on the fileserver are set to QEMU credentials.
* Client user credentials are saved in extended attributes.
@@ -146,7 +151,7 @@ VirtIODevice *virtio_9p_init(DeviceState *dev, V9fsConf
*conf)
" and export path:%s\n", conf->fsdev_id, s->ctx.fs_root);
exit(1);
}
- if (v9fs_init_worker_threads() < 0) {
+ if (v9fs_init_worker_threads(&s->ctx) < 0) {
fprintf(stderr, "worker thread initialization failed\n");
exit(1);
}
diff --git a/hw/9pfs/virtio-9p.h b/hw/9pfs/virtio-9p.h
index 9e3d4d3..ffe4f60 100644
--- a/hw/9pfs/virtio-9p.h
+++ b/hw/9pfs/virtio-9p.h
@@ -113,7 +113,11 @@ enum p9_proto_version {
#define FID_NON_RECLAIMABLE 0x2
static inline const char *rpath(FsContext *ctx, const char *path, char *buffer)
{
- snprintf(buffer, PATH_MAX, "%s/%s", ctx->fs_root, path);
+ if (ctx->enable_chroot) {
+ snprintf(buffer, PATH_MAX, "%s", path);
+ } else {
+ snprintf(buffer, PATH_MAX, "%s/%s", ctx->fs_root, path);
+ }
return buffer;
}
--
1.7.5.1
- [Qemu-devel] [RFC PATCH] virtio-9p: Use clone approach to fix TOCTOU vulnerability,
M. Mohan Kumar <=