[Qemu-devel] [V10 PATCH 00/13] virtio-9p: Use chroot to safely access files in passthrough security model

[Qemu-devel] [V10 PATCH 00/13] virtio-9p: Use chroot to safely access files in passthrough security model

[M. Mohan Kumar]

In passthrough security model, following symbolic links in the server
side could result in TOCTTOU vulnerabilities.

This patchset resolves this issue by creating a dedicated process which
chroots into the share path and all file object access is done in the
chroot environment.

This patchset implements chroot enviroment, provides necessary functions
that can be used by the passthrough function calls.

Changes from version V9:
* Error handling in special file object creation in virtio-9p-local.c
* Fix memory leak issue

Changes from version V8:
* Make chmod and chown also operate under chroot process
* Check for invalid path requests, minor cleanups

Changes from version V7:
* Add two chroot methods remove and rename
* Minor cleanups like consolidating functions

Changes from version V6:
* Send only fd/errno in socket operations instead of FdInfo structure
* Minor cleanups

Changes from version V5:
* Return errno on failure instead of setting errno
* Minor cleanups like updated comments, enable CONFIG_THREAD if
  CONFIG_VIRTFS is enabled

Changes from version V4:
* Avoid using malloc/free inside chroot process
* Seperate chroot server and client functions

Changes from version V3
* Return EIO incase of socket read/write fail instead of exiting
* Changed data types as suggested by Blue Swirl
* Chroot process reports error through qemu process

Changes from version V2
* Treat socket IO errors as fatal, ie qemu will exit
* Split patchset based on chroot side (server) and qemu side(client)
  functionalities
M. Mohan Kumar (13):
  Implement qemu_read_full
  virtio-9p: Enable CONFIG_THREAD if CONFIG_VIRTFS is enabled
  virtio-9p: Provide chroot worker side interfaces
  virtio-9p: Add qemu side interfaces for chroot environment
  virtio-9p: Add support to open a file in chroot environment
  virtio-9p: Create support in chroot environment
  virtio-9p: Support for creating special files
  virtio-9p: Add support for removing file or directory
  virtio-9p: Add support to rename
  virtio-9p: Move file post creation changes to none security model
  virtio-9p: Add support for chmod
  virtio-9p: Add support for chown
  virtio-9p: Chroot environment for other functions

 Makefile.objs                     |    1 +
 configure                         |    1 +
 hw/9pfs/virtio-9p-chroot-worker.c |  342 +++++++++++++++++++++++++++++++++++++
 hw/9pfs/virtio-9p-chroot.c        |  105 +++++++++++
 hw/9pfs/virtio-9p-chroot.h        |   48 +++++
 hw/9pfs/virtio-9p-local.c         |  306 ++++++++++++++++++++++++++--------
 hw/9pfs/virtio-9p.c               |   24 +++
 hw/file-op-9p.h                   |    3 +
 osdep.c                           |   32 ++++
 qemu-common.h                     |    2 +
 10 files changed, 796 insertions(+), 68 deletions(-)
 create mode 100644 hw/9pfs/virtio-9p-chroot-worker.c
 create mode 100644 hw/9pfs/virtio-9p-chroot.c
 create mode 100644 hw/9pfs/virtio-9p-chroot.h

-- 
1.7.4

[Qemu-devel] [V10 PATCH 01/13] Implement qemu_read_full

[M. Mohan Kumar]

Add qemu_read_full function

Signed-off-by: M. Mohan Kumar <mohan@in.ibm.com>
---
 osdep.c       |   32 ++++++++++++++++++++++++++++++++
 qemu-common.h |    2 ++
 2 files changed, 34 insertions(+), 0 deletions(-)

diff --git a/osdep.c b/osdep.c
index 327583b..8d84a88 100644
--- a/osdep.c
+++ b/osdep.c
@@ -127,6 +127,38 @@ ssize_t qemu_write_full(int fd, const void *buf, size_t count)
 }
 
 /*
+ * A variant of read(2) which handles interrupted read.
+ * Simlar to qemu_write_full function
+ *
+ * Return the number of bytes read.
+ *
+ * This function does not work with non-blocking fd's.
+ * errno is set if fewer than `count' bytes are read because of any
+ * error
+ */
+ssize_t qemu_read_full(int fd, void *buf, size_t count)
+{
+    ssize_t ret = 0;
+    ssize_t total = 0;
+
+    while (count) {
+        ret = read(fd, buf, count);
+        if (ret <= 0) {
+            if (errno == EINTR) {
+                continue;
+            }
+            break;
+        }
+
+        count -= ret;
+        buf += ret;
+        total += ret;
+    }
+
+    return total;
+}
+
+/*
  * Opens a socket with FD_CLOEXEC set
  */
 int qemu_socket(int domain, int type, int protocol)
diff --git a/qemu-common.h b/qemu-common.h
index 40dad52..325b16a 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -207,6 +207,8 @@ void qemu_mutex_unlock_iothread(void);
 int qemu_open(const char *name, int flags, ...);
 ssize_t qemu_write_full(int fd, const void *buf, size_t count)
     QEMU_WARN_UNUSED_RESULT;
+ssize_t qemu_read_full(int fd, void *buf, size_t count)
+    QEMU_WARN_UNUSED_RESULT;
 void qemu_set_cloexec(int fd);
 
 #ifndef _WIN32
-- 
1.7.4

[Qemu-devel] [V10 PATCH 02/13] virtio-9p: Enable CONFIG_THREAD if CONFIG_VIRTFS is enabled

[M. Mohan Kumar]

9p Chroot environment needs APIs defined in qemu-thread.c, so enable
CONFIG_THREAD if virtfs is enabled

Signed-off-by: M. Mohan Kumar <mohan@in.ibm.com>
---
 configure |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/configure b/configure
index 4a4d085..9bfbe51 100755
--- a/configure
+++ b/configure
@@ -2769,6 +2769,7 @@ fi
 if test "$linux" = "yes" ; then
   if test "$attr" = "yes" ; then
     echo "CONFIG_VIRTFS=y" >> $config_host_mak
+    echo "CONFIG_THREAD=y" >> $config_host_mak
   fi
 fi
 if test "$blobs" = "yes" ; then
-- 
1.7.4

[Qemu-devel] [V10 PATCH 03/13] virtio-9p: Provide chroot worker side interfaces

[M. Mohan Kumar]

Implement chroot worker side interfaces like sending the file
descriptor to qemu process, reading the object request from socket etc.
Also add chroot main function and other helper routines.

Signed-off-by: M. Mohan Kumar <mohan@in.ibm.com>
---
 Makefile.objs                     |    1 +
 hw/9pfs/virtio-9p-chroot-worker.c |  183 +++++++++++++++++++++++++++++++++++++
 hw/9pfs/virtio-9p-chroot.h        |   37 ++++++++
 hw/9pfs/virtio-9p.c               |   24 +++++
 hw/file-op-9p.h                   |    3 +
 5 files changed, 248 insertions(+), 0 deletions(-)
 create mode 100644 hw/9pfs/virtio-9p-chroot-worker.c
 create mode 100644 hw/9pfs/virtio-9p-chroot.h

diff --git a/Makefile.objs b/Makefile.objs
index 761c8e8..f499c8c 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -285,6 +285,7 @@ hw-obj-$(CONFIG_SOUND) += $(sound-obj-y)
 9pfs-nested-$(CONFIG_VIRTFS) = virtio-9p-debug.o
 9pfs-nested-$(CONFIG_VIRTFS) +=  virtio-9p-local.o virtio-9p-xattr.o
 9pfs-nested-$(CONFIG_VIRTFS) +=   virtio-9p-xattr-user.o virtio-9p-posix-acl.o
+9pfs-nested-$(CONFIG_VIRTFS) +=   virtio-9p-chroot-worker.o
 
 hw-obj-$(CONFIG_REALLY_VIRTFS) += $(addprefix 9pfs/, $(9pfs-nested-y))
 $(addprefix 9pfs/, $(9pfs-nested-y)): CFLAGS +=  -I$(SRC_PATH)/hw/
diff --git a/hw/9pfs/virtio-9p-chroot-worker.c b/hw/9pfs/virtio-9p-chroot-worker.c
new file mode 100644
index 0000000..9e02eb7
--- /dev/null
+++ b/hw/9pfs/virtio-9p-chroot-worker.c
@@ -0,0 +1,183 @@
+/*
+ * Virtio 9p chroot environment for contained access to the exported path
+ * Code path handles chroot worker side interfaces
+ * Copyright IBM, Corp. 2011
+ *
+ * Authors:
+ * M. Mohan Kumar <mohan@in.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See
+ * the copying file in the top-level directory
+ *
+ */
+
+#include <sys/fsuid.h>
+#include <sys/resource.h>
+#include <signal.h>
+#include "virtio.h"
+#include "qemu_socket.h"
+#include "qemu-thread.h"
+#include "qerror.h"
+#include "virtio-9p.h"
+#include "virtio-9p-chroot.h"
+
+/* Send file descriptor and error status to qemu process */
+static void chroot_sendfd(int sockfd, int fd, int fd_valid)
+{
+    struct msghdr msg = { };
+    struct iovec iov;
+    struct cmsghdr *cmsg;
+    int retval;
+    union MsgControl msg_control;
+
+    iov.iov_base = &fd;
+    iov.iov_len = sizeof(fd);
+
+    memset(&msg, 0, sizeof(msg));
+    msg.msg_iov = &iov;
+    msg.msg_iovlen = 1;
+    /* No ancillary data on error */
+    if (fd_valid) {
+        msg.msg_control = &msg_control;
+        msg.msg_controllen = sizeof(msg_control);
+
+        cmsg = &msg_control.cmsg;
+        cmsg->cmsg_len = CMSG_LEN(sizeof(fd));
+        cmsg->cmsg_level = SOL_SOCKET;
+        cmsg->cmsg_type = SCM_RIGHTS;
+        memcpy(CMSG_DATA(cmsg), &fd, sizeof(fd));
+    }
+    do {
+        retval = sendmsg(sockfd, &msg, 0);
+    } while (retval < 0 && errno == EINTR);
+    if (retval < 0) {
+        _exit(1);
+    }
+    if (fd_valid) {
+        close(fd);
+    }
+}
+
+/* Read V9fsFileObjectRequest sent by QEMU process */
+static int chroot_read_request(int sockfd, V9fsFileObjectRequest *request)
+{
+    int retval;
+    retval = qemu_read_full(sockfd, request, sizeof(*request));
+    if (retval != sizeof(*request)) {
+        if (errno == EBADF) {
+            _exit(1);
+        }
+        return -EIO;
+    }
+    return 0;
+}
+
+/*
+ * Helper routine to open a file
+ */
+static int chroot_do_open(V9fsFileObjectRequest *request)
+{
+    int fd;
+    fd = open(request->path.path, request->data.flags);
+    if (fd < 0) {
+        fd = -errno;
+    }
+    return fd;
+}
+
+static void chroot_daemonize(int chroot_sock)
+{
+    sigset_t sigset;
+    struct rlimit nr_fd;
+    int fd;
+
+    /* Block all signals for this process */
+    sigfillset(&sigset);
+    sigprocmask(SIG_SETMASK, &sigset, NULL);
+
+    /* Close other file descriptors */
+    getrlimit(RLIMIT_NOFILE, &nr_fd);
+    for (fd = 0; fd < nr_fd.rlim_cur; fd++) {
+        if (fd != chroot_sock) {
+            close(fd);
+        }
+    }
+    chdir("/");
+    /* Create files with mode as per request */
+    umask(0);
+}
+
+/*
+ * Fork a process and chroot into the share path. Communication
+ * between qemu process and chroot process happens via socket.
+ * All file descriptors (including stdout and stderr) are closed
+ * except one socket descriptor (which is used for communicating
+ * between qemu process and chroot process).
+ * Note: To avoid errors in forked process in multi threaded environment
+ * only async-signal safe functions used. For more information see
+ * man fork(3p), signal(7)
+ */
+int v9fs_chroot(FsContext *fs_ctx)
+{
+    int fd_pair[2], chroot_sock, error;
+    V9fsFileObjectRequest request;
+    pid_t pid;
+    uint32_t code;
+    int retval, valid_fd;
+
+    if (socketpair(PF_UNIX, SOCK_STREAM, 0, fd_pair) < 0) {
+        error_report("socketpair %s", strerror(errno));
+        return -1;
+    }
+
+    pid = fork();
+    if (pid < 0) {
+        error_report("fork %s", strerror(errno));
+        return -1;
+    }
+    if (pid != 0) {
+        fs_ctx->chroot_socket = fd_pair[0];
+        close(fd_pair[1]);
+        return 0;
+    }
+
+    close(fd_pair[0]);
+    chroot_sock = fd_pair[1];
+    if (chroot(fs_ctx->fs_root) < 0) {
+        code = errno;
+        error = qemu_write_full(chroot_sock, &code, sizeof(code));
+        _exit(1);
+    }
+
+    chroot_daemonize(chroot_sock);
+
+    /*
+     * Write 0 to chroot socket to indicate chroot process creation is
+     * successful
+     */
+    code = 0;
+    if (qemu_write_full(chroot_sock, &code, sizeof(code))
+                    != sizeof(code)) {
+        _exit(1);
+    }
+    /* get the request from the socket */
+    while (1) {
+        valid_fd = 0;
+        if (chroot_read_request(chroot_sock, &request) < 0) {
+            chroot_sendfd(chroot_sock, -1, valid_fd);
+            continue;
+        }
+        switch (request.data.type) {
+        case T_OPEN:
+            retval = chroot_do_open(&request);
+            if (retval >= 0) {
+                valid_fd = 1;
+            }
+            break;
+        default:
+            retval = -1;
+            break;
+        }
+        chroot_sendfd(chroot_sock, retval, valid_fd);
+    }
+}
diff --git a/hw/9pfs/virtio-9p-chroot.h b/hw/9pfs/virtio-9p-chroot.h
new file mode 100644
index 0000000..65cc9d3
--- /dev/null
+++ b/hw/9pfs/virtio-9p-chroot.h
@@ -0,0 +1,37 @@
+#ifndef _QEMU_VIRTIO_9P_CHROOT_H
+#define _QEMU_VIRTIO_9P_CHROOT_H
+
+#include "qemu_socket.h"
+/* types for V9fsFileObjectRequest */
+#define T_OPEN      1
+
+union MsgControl {
+    struct cmsghdr cmsg;
+    char control[CMSG_SPACE(sizeof(int))];
+};
+
+struct V9fsFileObjectData
+{
+    int flags;
+    int mode;
+    uid_t uid;
+    gid_t gid;
+    dev_t dev;
+    int type;
+};
+
+struct V9fsFileObjectPath
+{
+    char path[PATH_MAX];
+    char old_path[PATH_MAX];
+};
+
+typedef struct V9fsFileObjectRequest
+{
+    struct V9fsFileObjectData data;
+    struct V9fsFileObjectPath path;
+} V9fsFileObjectRequest;
+
+int v9fs_chroot(FsContext *fs_ctx);
+
+#endif /* _QEMU_VIRTIO_9P_CHROOT_H */
diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c
index 6c06bb3..e5e1cd3 100644
--- a/hw/9pfs/virtio-9p.c
+++ b/hw/9pfs/virtio-9p.c
@@ -14,10 +14,12 @@
 #include "virtio.h"
 #include "pc.h"
 #include "qemu_socket.h"
+#include "qerror.h"
 #include "virtio-9p.h"
 #include "fsdev/qemu-fsdev.h"
 #include "virtio-9p-debug.h"
 #include "virtio-9p-xattr.h"
+#include "virtio-9p-chroot.h"
 
 int debug_9p_pdu;
 static int max_open_fd;
@@ -3931,6 +3933,28 @@ VirtIODevice *virtio_9p_init(DeviceState *dev, V9fsConf *conf)
                         s->tag_len;
     s->vdev.get_config = virtio_9p_get_config;
 
+    if (s->ctx.fs_sm == SM_PASSTHROUGH) {
+        uint32_t code;
+        qemu_mutex_init(&s->ctx.chroot_mutex);
+        if (v9fs_chroot(&s->ctx) < 0) {
+            exit(1);
+        }
+
+        /*
+         * Chroot process sends 0 to indicate chroot process creation is
+         * successful
+         */
+        if (qemu_read_full(s->ctx.chroot_socket, &code,
+                                sizeof(code)) != sizeof(code)) {
+            error_report("chroot process creation failed");
+            exit(1);
+        }
+        if (code != 0) {
+            error_report("chroot system call failed: %s", strerror(code));
+            exit(1);
+        }
+    }
+
     return &s->vdev;
 }
 
diff --git a/hw/file-op-9p.h b/hw/file-op-9p.h
index 8577020..6b2e16a 100644
--- a/hw/file-op-9p.h
+++ b/hw/file-op-9p.h
@@ -19,6 +19,7 @@
 #include <sys/stat.h>
 #include <sys/uio.h>
 #include <sys/vfs.h>
+#include "qemu-thread.h"
 #define SM_LOCAL_MODE_BITS    0600
 #define SM_LOCAL_DIR_MODE_BITS    0700
 
@@ -56,6 +57,8 @@ typedef struct FsContext
     uid_t uid;
     int open_flags;
     struct xattr_operations **xops;
+    QemuMutex chroot_mutex;
+    int chroot_socket;
 } FsContext;
 
 void cred_init(FsCred *);
-- 
1.7.4

[Qemu-devel] [V10 PATCH 04/13] virtio-9p: Add qemu side interfaces for chroot environment

[M. Mohan Kumar]

QEMU side interfaces to communicate with chroot worker process.

Signed-off-by: M. Mohan Kumar <mohan@in.ibm.com>
---
 Makefile.objs              |    2 +-
 hw/9pfs/virtio-9p-chroot.c |   92 ++++++++++++++++++++++++++++++++++++++++++++
 hw/9pfs/virtio-9p-chroot.h |    1 +
 3 files changed, 94 insertions(+), 1 deletions(-)
 create mode 100644 hw/9pfs/virtio-9p-chroot.c

diff --git a/Makefile.objs b/Makefile.objs
index f499c8c..9c984fc 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -285,7 +285,7 @@ hw-obj-$(CONFIG_SOUND) += $(sound-obj-y)
 9pfs-nested-$(CONFIG_VIRTFS) = virtio-9p-debug.o
 9pfs-nested-$(CONFIG_VIRTFS) +=  virtio-9p-local.o virtio-9p-xattr.o
 9pfs-nested-$(CONFIG_VIRTFS) +=   virtio-9p-xattr-user.o virtio-9p-posix-acl.o
-9pfs-nested-$(CONFIG_VIRTFS) +=   virtio-9p-chroot-worker.o
+9pfs-nested-$(CONFIG_VIRTFS) +=   virtio-9p-chroot-worker.o virtio-9p-chroot.o
 
 hw-obj-$(CONFIG_REALLY_VIRTFS) += $(addprefix 9pfs/, $(9pfs-nested-y))
 $(addprefix 9pfs/, $(9pfs-nested-y)): CFLAGS +=  -I$(SRC_PATH)/hw/
diff --git a/hw/9pfs/virtio-9p-chroot.c b/hw/9pfs/virtio-9p-chroot.c
new file mode 100644
index 0000000..4aa3b43
--- /dev/null
+++ b/hw/9pfs/virtio-9p-chroot.c
@@ -0,0 +1,92 @@
+/*
+ * Virtio 9p chroot environment for contained access to exported path
+ * Code handles qemu side interfaces to communicate with chroot worker process
+ * Copyright IBM, Corp. 2011
+ *
+ * Authors:
+ * M. Mohan Kumar <mohan@in.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See
+ * the copying file in the top-level directory
+ *
+ */
+
+#include <sys/fsuid.h>
+#include <sys/resource.h>
+#include <signal.h>
+#include "virtio.h"
+#include "qemu_socket.h"
+#include "qemu-thread.h"
+#include "qerror.h"
+#include "virtio-9p.h"
+#include "virtio-9p-chroot.h"
+
+/*
+ * Return received file descriptor on success and -errno on failure.
+ * sock_error is set to 1 whenever there is error in socket IO
+ */
+static int v9fs_receivefd(int sockfd, int *sock_error)
+{
+    struct msghdr msg = { };
+    struct iovec iov;
+    union MsgControl msg_control;
+    struct cmsghdr *cmsg;
+    int retval, fd;
+
+    iov.iov_base = &fd;
+    iov.iov_len = sizeof(fd);
+
+    *sock_error = 0;
+    memset(&msg, 0, sizeof(msg));
+    msg.msg_iov = &iov;
+    msg.msg_iovlen = 1;
+    msg.msg_control = &msg_control;
+    msg.msg_controllen = sizeof(msg_control);
+
+    do {
+        retval = recvmsg(sockfd, &msg, 0);
+    } while (retval < 0 && errno == EINTR);
+    if (retval <= 0) {
+        *sock_error = 1;
+        return -EIO;
+    }
+
+    if (fd < 0) {
+        return fd;
+    }
+    for (cmsg = CMSG_FIRSTHDR(&msg); cmsg; cmsg = CMSG_NXTHDR(&msg, cmsg)) {
+        if (cmsg->cmsg_len != CMSG_LEN(sizeof(int)) ||
+                cmsg->cmsg_level != SOL_SOCKET ||
+                cmsg->cmsg_type != SCM_RIGHTS) {
+            continue;
+        }
+        fd = *((int *)CMSG_DATA(cmsg));
+        return fd;
+    }
+    return fd;
+}
+
+/*
+ * V9fsFileObjectRequest is written into the socket by QEMU process.
+ * Then this request is read by chroot process using v9fs_read_request function
+ */
+static int v9fs_write_request(int sockfd, V9fsFileObjectRequest *request)
+{
+    int retval;
+    retval = qemu_write_full(sockfd, request, sizeof(*request));
+    if (retval != sizeof(*request)) {
+        return -EIO;
+    }
+    return 0;
+}
+
+/*
+ * This patch adds v9fs_receivefd and v9fs_write_request functions,
+ * but there is no callers. To avoid compiler warning message,
+ * refer these two functions
+ */
+void chroot_dummy(void)
+{
+    (void)v9fs_receivefd;
+    (void)v9fs_write_request;
+}
diff --git a/hw/9pfs/virtio-9p-chroot.h b/hw/9pfs/virtio-9p-chroot.h
index 65cc9d3..f8c6d77 100644
--- a/hw/9pfs/virtio-9p-chroot.h
+++ b/hw/9pfs/virtio-9p-chroot.h
@@ -33,5 +33,6 @@ typedef struct V9fsFileObjectRequest
 } V9fsFileObjectRequest;
 
 int v9fs_chroot(FsContext *fs_ctx);
+void chroot_dummy(void);
 
 #endif /* _QEMU_VIRTIO_9P_CHROOT_H */
-- 
1.7.4

[Qemu-devel] [V10 PATCH 05/13] virtio-9p: Add support to open a file in chroot environment

[M. Mohan Kumar]

This patch adds both chroot worker and qemu side support to open a file/
directory in the chroot environment

Signed-off-by: M. Mohan Kumar <mohan@in.ibm.com>
---
 hw/9pfs/virtio-9p-chroot.c |   29 +++++++++++----
 hw/9pfs/virtio-9p-chroot.h |    2 +-
 hw/9pfs/virtio-9p-local.c  |   80 +++++++++++++++++++++++++++++++++++++++++--
 3 files changed, 98 insertions(+), 13 deletions(-)

diff --git a/hw/9pfs/virtio-9p-chroot.c b/hw/9pfs/virtio-9p-chroot.c
index 4aa3b43..8d41087 100644
--- a/hw/9pfs/virtio-9p-chroot.c
+++ b/hw/9pfs/virtio-9p-chroot.c
@@ -80,13 +80,26 @@ static int v9fs_write_request(int sockfd, V9fsFileObjectRequest *request)
     return 0;
 }
 
-/*
- * This patch adds v9fs_receivefd and v9fs_write_request functions,
- * but there is no callers. To avoid compiler warning message,
- * refer these two functions
- */
-void chroot_dummy(void)
+/* Return opened file descriptor on success or -errno on error */
+int v9fs_request(FsContext *fs_ctx, V9fsFileObjectRequest *request)
 {
-    (void)v9fs_receivefd;
-    (void)v9fs_write_request;
+    int fd, sock_error;
+    qemu_mutex_lock(&fs_ctx->chroot_mutex);
+    if (fs_ctx->chroot_socket == -1) {
+        goto error;
+    }
+    if (v9fs_write_request(fs_ctx->chroot_socket, request) < 0) {
+        goto error;
+    }
+    fd = v9fs_receivefd(fs_ctx->chroot_socket, &sock_error);
+    if (fd < 0 && sock_error) {
+        goto error;
+    }
+    qemu_mutex_unlock(&fs_ctx->chroot_mutex);
+    return fd;
+error:
+    close(fs_ctx->chroot_socket);
+    fs_ctx->chroot_socket = -1;
+    qemu_mutex_unlock(&fs_ctx->chroot_mutex);
+    return -EIO;
 }
diff --git a/hw/9pfs/virtio-9p-chroot.h b/hw/9pfs/virtio-9p-chroot.h
index f8c6d77..ac94e59 100644
--- a/hw/9pfs/virtio-9p-chroot.h
+++ b/hw/9pfs/virtio-9p-chroot.h
@@ -33,6 +33,6 @@ typedef struct V9fsFileObjectRequest
 } V9fsFileObjectRequest;
 
 int v9fs_chroot(FsContext *fs_ctx);
-void chroot_dummy(void);
+int v9fs_request(FsContext *fs_ctx, V9fsFileObjectRequest *or);
 
 #endif /* _QEMU_VIRTIO_9P_CHROOT_H */
diff --git a/hw/9pfs/virtio-9p-local.c b/hw/9pfs/virtio-9p-local.c
index 43ff37c..2601f77 100644
--- a/hw/9pfs/virtio-9p-local.c
+++ b/hw/9pfs/virtio-9p-local.c
@@ -13,6 +13,9 @@
 #include "virtio.h"
 #include "virtio-9p.h"
 #include "virtio-9p-xattr.h"
+#include "qemu_socket.h"
+#include "fsdev/qemu-fsdev.h"
+#include "virtio-9p-chroot.h"
 #include <arpa/inet.h>
 #include <pwd.h>
 #include <grp.h>
@@ -20,6 +23,62 @@
 #include <sys/un.h>
 #include <attr/xattr.h>
 
+/* Helper routine to fill V9fsFileObjectRequest structure */
+static int fill_fileobjectrequest(V9fsFileObjectRequest *request,
+                const char *oldpath, const char *path, int flags,
+                FsCred *credp, int type)
+{
+    if (oldpath && strlen(oldpath) >= PATH_MAX) {
+        return -ENAMETOOLONG;
+    }
+    /* path can't be NULL */
+    if (!path) {
+        return -EFAULT;
+    }
+
+    if (strlen(path) >= PATH_MAX) {
+        return -ENAMETOOLONG;
+    }
+    strcpy(request->path.path, path);
+    if (oldpath) {
+        strcpy(request->path.old_path, oldpath);
+    } else {
+        request->path.old_path[0] = '\0';
+    }
+
+    memset(&request->data, 0, sizeof(request->data));
+    if (credp) {
+        request->data.mode = credp->fc_mode;
+        request->data.uid = credp->fc_uid;
+        request->data.gid = credp->fc_gid;
+        request->data.dev = credp->fc_rdev;
+    }
+
+    request->data.flags = flags;
+    request->data.type = type;
+    return 0;
+}
+
+static int passthrough_request(FsContext *fs_ctx, const char *old_path,
+                const char *path, int flags, FsCred *credp, int type)
+{
+    V9fsFileObjectRequest request;
+    int retval;
+
+    retval = fill_fileobjectrequest(&request, old_path, path, flags, credp,
+                    type);
+    if (retval < 0) {
+        errno = -retval;
+        return -1;
+    }
+
+    retval = v9fs_request(fs_ctx, &request);
+    if (retval < 0) {
+        errno = -retval;
+        retval = -1;
+    }
+    return retval;
+}
 
 static int local_lstat(FsContext *fs_ctx, const char *path, struct stat *stbuf)
 {
@@ -138,14 +197,27 @@ static int local_closedir(FsContext *ctx, DIR *dir)
     return closedir(dir);
 }
 
-static int local_open(FsContext *ctx, const char *path, int flags)
+static int local_open(FsContext *fs_ctx, const char *path, int flags)
 {
-    return open(rpath(ctx, path), flags);
+    if (fs_ctx->fs_sm == SM_PASSTHROUGH) {
+        return passthrough_request(fs_ctx, NULL, path, flags, NULL, T_OPEN);
+    } else {
+        return open(rpath(fs_ctx, path), flags);
+    }
 }
 
-static DIR *local_opendir(FsContext *ctx, const char *path)
+static DIR *local_opendir(FsContext *fs_ctx, const char *path)
 {
-    return opendir(rpath(ctx, path));
+    if (fs_ctx->fs_sm == SM_PASSTHROUGH) {
+        int fd;
+        fd = passthrough_request(fs_ctx, NULL, path, O_DIRECTORY, NULL, T_OPEN);
+        if (fd < 0) {
+            return NULL;
+        }
+        return fdopendir(fd);
+    } else {
+        return opendir(rpath(fs_ctx, path));
+    }
 }
 
 static void local_rewinddir(FsContext *ctx, DIR *dir)
-- 
1.7.4

[Qemu-devel] [V10 PATCH 06/13] virtio-9p: Create support in chroot environment

[M. Mohan Kumar]

Add both chroot worker & qemu side interfaces to create regular files in
chroot environment

Signed-off-by: M. Mohan Kumar <mohan@in.ibm.com>
---
 hw/9pfs/virtio-9p-chroot-worker.c |   36 ++++++++++++++++++++++++++++++++++++
 hw/9pfs/virtio-9p-chroot.h        |    1 +
 hw/9pfs/virtio-9p-local.c         |    5 +++--
 3 files changed, 40 insertions(+), 2 deletions(-)

diff --git a/hw/9pfs/virtio-9p-chroot-worker.c b/hw/9pfs/virtio-9p-chroot-worker.c
index 9e02eb7..2203923 100644
--- a/hw/9pfs/virtio-9p-chroot-worker.c
+++ b/hw/9pfs/virtio-9p-chroot-worker.c
@@ -85,6 +85,36 @@ static int chroot_do_open(V9fsFileObjectRequest *request)
     return fd;
 }
 
+/*
+ * Helper routine to create a file and return the file descriptor
+ */
+static int chroot_do_create(V9fsFileObjectRequest *request)
+{
+    uid_t cur_uid;
+    gid_t cur_gid;
+    int fd = -1;
+
+    cur_uid = geteuid();
+    cur_gid = getegid();
+
+    if (setfsuid(request->data.uid) < 0) {
+        return -errno;
+    }
+    if (setfsgid(request->data.gid) < 0) {
+        fd = -errno;
+        goto unset_uid;
+    }
+
+    fd = open(request->path.path, request->data.flags, request->data.mode);
+    if (fd < 0) {
+        fd = -errno;
+    }
+    setfsgid(cur_gid);
+unset_uid:
+    setfsuid(cur_uid);
+    return fd;
+}
+
 static void chroot_daemonize(int chroot_sock)
 {
     sigset_t sigset;
@@ -174,6 +204,12 @@ int v9fs_chroot(FsContext *fs_ctx)
                 valid_fd = 1;
             }
             break;
+        case T_CREATE:
+            retval = chroot_do_create(&request);
+            if (retval >= 0) {
+                valid_fd = 1;
+            }
+            break;
         default:
             retval = -1;
             break;
diff --git a/hw/9pfs/virtio-9p-chroot.h b/hw/9pfs/virtio-9p-chroot.h
index ac94e59..2e584eb 100644
--- a/hw/9pfs/virtio-9p-chroot.h
+++ b/hw/9pfs/virtio-9p-chroot.h
@@ -4,6 +4,7 @@
 #include "qemu_socket.h"
 /* types for V9fsFileObjectRequest */
 #define T_OPEN      1
+#define T_CREATE    2
 
 union MsgControl {
     struct cmsghdr cmsg;
diff --git a/hw/9pfs/virtio-9p-local.c b/hw/9pfs/virtio-9p-local.c
index 2601f77..463efe2 100644
--- a/hw/9pfs/virtio-9p-local.c
+++ b/hw/9pfs/virtio-9p-local.c
@@ -404,8 +404,7 @@ static int local_open2(FsContext *fs_ctx, const char *path, int flags,
             serrno = errno;
             goto err_end;
         }
-    } else if ((fs_ctx->fs_sm == SM_PASSTHROUGH) ||
-               (fs_ctx->fs_sm == SM_NONE)) {
+    } else if (fs_ctx->fs_sm == SM_NONE) {
         fd = open(rpath(fs_ctx, path), flags, credp->fc_mode);
         if (fd == -1) {
             return fd;
@@ -415,6 +414,8 @@ static int local_open2(FsContext *fs_ctx, const char *path, int flags,
             serrno = errno;
             goto err_end;
         }
+    } else if (fs_ctx->fs_sm == SM_PASSTHROUGH) {
+        fd = passthrough_request(fs_ctx, NULL, path, flags, credp, T_CREATE);
     }
     return fd;
 
-- 
1.7.4

[Qemu-devel] [V10 PATCH 07/13] virtio-9p: Support for creating special files

[M. Mohan Kumar]

Add both chroot worker and qemu side interfaces to create special files
(directory, device nodes, links and symbolic links)

Signed-off-by: M. Mohan Kumar <mohan@in.ibm.com>
---
 hw/9pfs/virtio-9p-chroot-worker.c |   52 +++++++++++++++++++++++++++++++++++++
 hw/9pfs/virtio-9p-chroot.h        |    5 +++
 hw/9pfs/virtio-9p-local.c         |   43 +++++++++++++-----------------
 3 files changed, 75 insertions(+), 25 deletions(-)

diff --git a/hw/9pfs/virtio-9p-chroot-worker.c b/hw/9pfs/virtio-9p-chroot-worker.c
index 2203923..fc05fd0 100644
--- a/hw/9pfs/virtio-9p-chroot-worker.c
+++ b/hw/9pfs/virtio-9p-chroot-worker.c
@@ -115,6 +115,52 @@ unset_uid:
     return fd;
 }
 
+/*
+ * Create directory, symbolic link, link, device node and regular files
+ * Similar to create, but it does not return the fd of created object
+ * Returns 0 as file descriptor on success and -errno on failure
+ */
+static int chroot_do_create_special(V9fsFileObjectRequest *request)
+{
+    int cur_uid, cur_gid;
+    int retval = -1;
+
+    cur_uid = geteuid();
+    cur_gid = getegid();
+
+    if (setfsuid(request->data.uid) < 0) {
+        return -errno;
+    }
+    if (setfsgid(request->data.gid) < 0) {
+        retval = -errno;
+        goto unset_uid;
+    }
+
+    switch (request->data.type) {
+    case T_MKDIR:
+        retval = mkdir(request->path.path, request->data.mode);
+        break;
+    case T_SYMLINK:
+        retval = symlink(request->path.old_path, request->path.path);
+        break;
+    case T_LINK:
+        retval = link(request->path.old_path, request->path.path);
+        break;
+    default:
+        retval = mknod(request->path.path, request->data.mode,
+                        request->data.dev);
+        break;
+    }
+
+    if (retval < 0) {
+        retval = -errno;
+    }
+    setfsgid(cur_gid);
+unset_uid:
+    setfsuid(cur_uid);
+    return retval;
+}
+
 static void chroot_daemonize(int chroot_sock)
 {
     sigset_t sigset;
@@ -210,6 +256,12 @@ int v9fs_chroot(FsContext *fs_ctx)
                 valid_fd = 1;
             }
             break;
+        case T_MKDIR:
+        case T_SYMLINK:
+        case T_LINK:
+        case T_MKNOD:
+            retval = chroot_do_create_special(&request);
+            break;
         default:
             retval = -1;
             break;
diff --git a/hw/9pfs/virtio-9p-chroot.h b/hw/9pfs/virtio-9p-chroot.h
index 2e584eb..2192a05 100644
--- a/hw/9pfs/virtio-9p-chroot.h
+++ b/hw/9pfs/virtio-9p-chroot.h
@@ -5,6 +5,10 @@
 /* types for V9fsFileObjectRequest */
 #define T_OPEN      1
 #define T_CREATE    2
+#define T_MKDIR     3
+#define T_MKNOD     4
+#define T_SYMLINK   5
+#define T_LINK      6
 
 union MsgControl {
     struct cmsghdr cmsg;
@@ -35,5 +39,6 @@ typedef struct V9fsFileObjectRequest
 
 int v9fs_chroot(FsContext *fs_ctx);
 int v9fs_request(FsContext *fs_ctx, V9fsFileObjectRequest *or);
+int v9fs_create_special(FsContext *fs_ctx, V9fsFileObjectRequest *request);
 
 #endif /* _QEMU_VIRTIO_9P_CHROOT_H */
diff --git a/hw/9pfs/virtio-9p-local.c b/hw/9pfs/virtio-9p-local.c
index 463efe2..2ad2f00 100644
--- a/hw/9pfs/virtio-9p-local.c
+++ b/hw/9pfs/virtio-9p-local.c
@@ -297,8 +297,7 @@ static int local_mknod(FsContext *fs_ctx, const char *path, FsCred *credp)
             serrno = errno;
             goto err_end;
         }
-    } else if ((fs_ctx->fs_sm == SM_PASSTHROUGH) ||
-               (fs_ctx->fs_sm == SM_NONE)) {
+    } else if (fs_ctx->fs_sm == SM_NONE) {
         err = mknod(rpath(fs_ctx, path), credp->fc_mode, credp->fc_rdev);
         if (err == -1) {
             return err;
@@ -308,6 +307,8 @@ static int local_mknod(FsContext *fs_ctx, const char *path, FsCred *credp)
             serrno = errno;
             goto err_end;
         }
+    } else if (fs_ctx->fs_sm == SM_PASSTHROUGH) {
+        err = passthrough_request(fs_ctx, NULL, path, 0, credp, T_MKNOD);
     }
     return err;
 
@@ -334,8 +335,7 @@ static int local_mkdir(FsContext *fs_ctx, const char *path, FsCred *credp)
             serrno = errno;
             goto err_end;
         }
-    } else if ((fs_ctx->fs_sm == SM_PASSTHROUGH) ||
-               (fs_ctx->fs_sm == SM_NONE)) {
+    } else if (fs_ctx->fs_sm == SM_NONE) {
         err = mkdir(rpath(fs_ctx, path), credp->fc_mode);
         if (err == -1) {
             return err;
@@ -345,6 +345,8 @@ static int local_mkdir(FsContext *fs_ctx, const char *path, FsCred *credp)
             serrno = errno;
             goto err_end;
         }
+    } else if (fs_ctx->fs_sm == SM_PASSTHROUGH) {
+        err = passthrough_request(fs_ctx, NULL, path, 0, credp, T_MKDIR);
     }
     return err;
 
@@ -462,24 +464,16 @@ static int local_symlink(FsContext *fs_ctx, const char *oldpath,
             serrno = errno;
             goto err_end;
         }
-    } else if ((fs_ctx->fs_sm == SM_PASSTHROUGH) ||
-               (fs_ctx->fs_sm == SM_NONE)) {
+    } else if (fs_ctx->fs_sm == SM_NONE) {
         err = symlink(oldpath, rpath(fs_ctx, newpath));
         if (err) {
             return err;
         }
         err = lchown(rpath(fs_ctx, newpath), credp->fc_uid, credp->fc_gid);
-        if (err == -1) {
-            /*
-             * If we fail to change ownership and if we are
-             * using security model none. Ignore the error
-             */
-            if (fs_ctx->fs_sm != SM_NONE) {
-                serrno = errno;
-                goto err_end;
-            } else
-                err = 0;
-        }
+        err = 0;
+    } else if (fs_ctx->fs_sm == SM_PASSTHROUGH) {
+        err = passthrough_request(fs_ctx, oldpath, newpath, 0, credp,
+                        T_SYMLINK);
     }
     return err;
 
@@ -489,26 +483,25 @@ err_end:
     return err;
 }
 
-static int local_link(FsContext *ctx, const char *oldpath, const char *newpath)
+static int local_link(FsContext *fs_ctx, const char *oldpath,
+                const char *newpath)
 {
-    char *tmp = qemu_strdup(rpath(ctx, oldpath));
     int err, serrno = 0;
+    char *tmp;
 
-    if (tmp == NULL) {
-        return -ENOMEM;
+    if (fs_ctx->fs_sm == SM_PASSTHROUGH) {
+        return passthrough_request(fs_ctx, oldpath, newpath, 0, NULL, T_LINK);
     }
 
-    err = link(tmp, rpath(ctx, newpath));
+    tmp = qemu_strdup(rpath(fs_ctx, oldpath));
+    err = link(tmp, rpath(fs_ctx, newpath));
     if (err == -1) {
         serrno = errno;
     }
-
     qemu_free(tmp);
-
     if (err == -1) {
         errno = serrno;
     }
-
     return err;
 }
 
-- 
1.7.4

[Qemu-devel] [V10 PATCH 08/13] virtio-9p: Add support for removing file or directory

[M. Mohan Kumar]

Support for removing file or directory in chroot environment. Add
interfaces to remove file/directory in chroot worker and qemu side.

Signed-off-by: M. Mohan Kumar <mohan@in.ibm.com>
---
 hw/9pfs/virtio-9p-chroot-worker.c |   18 ++++++++++++++++++
 hw/9pfs/virtio-9p-chroot.h        |    1 +
 hw/9pfs/virtio-9p-local.c         |    3 +++
 3 files changed, 22 insertions(+), 0 deletions(-)

diff --git a/hw/9pfs/virtio-9p-chroot-worker.c b/hw/9pfs/virtio-9p-chroot-worker.c
index fc05fd0..40e572c 100644
--- a/hw/9pfs/virtio-9p-chroot-worker.c
+++ b/hw/9pfs/virtio-9p-chroot-worker.c
@@ -161,6 +161,21 @@ unset_uid:
     return retval;
 }
 
+/*
+ * Remove a file object
+ * Returns 0 on success and -errno on failure
+ */
+static int chroot_do_remove(V9fsFileObjectRequest *request)
+{
+    int retval;
+
+    retval = remove(request->path.path);
+    if (retval < 0) {
+        return -errno;
+    }
+    return 0;
+}
+
 static void chroot_daemonize(int chroot_sock)
 {
     sigset_t sigset;
@@ -262,6 +277,9 @@ int v9fs_chroot(FsContext *fs_ctx)
         case T_MKNOD:
             retval = chroot_do_create_special(&request);
             break;
+        case T_REMOVE:
+            retval = chroot_do_remove(&request);
+            break;
         default:
             retval = -1;
             break;
diff --git a/hw/9pfs/virtio-9p-chroot.h b/hw/9pfs/virtio-9p-chroot.h
index 2192a05..856e9c4 100644
--- a/hw/9pfs/virtio-9p-chroot.h
+++ b/hw/9pfs/virtio-9p-chroot.h
@@ -9,6 +9,7 @@
 #define T_MKNOD     4
 #define T_SYMLINK   5
 #define T_LINK      6
+#define T_REMOVE    7
 
 union MsgControl {
     struct cmsghdr cmsg;
diff --git a/hw/9pfs/virtio-9p-local.c b/hw/9pfs/virtio-9p-local.c
index 2ad2f00..7ae21ef 100644
--- a/hw/9pfs/virtio-9p-local.c
+++ b/hw/9pfs/virtio-9p-local.c
@@ -553,6 +553,9 @@ static int local_utimensat(FsContext *s, const char *path,
 
 static int local_remove(FsContext *ctx, const char *path)
 {
+    if (ctx->fs_sm == SM_PASSTHROUGH) {
+        return passthrough_request(ctx, NULL, path, 0, NULL, T_REMOVE);
+    }
     return remove(rpath(ctx, path));
 }
 
-- 
1.7.4

[Qemu-devel] [V10 PATCH 09/13] virtio-9p: Add support to rename

[M. Mohan Kumar]

Support renaming a file or directory in chroot envirnoment. Add
interfaces for renaming in chroot worker and qemu side.

Signed-off-by: M. Mohan Kumar <mohan@in.ibm.com>
---
 hw/9pfs/virtio-9p-chroot-worker.c |   17 +++++++++++++++++
 hw/9pfs/virtio-9p-chroot.h        |    1 +
 hw/9pfs/virtio-9p-local.c         |   17 +++++++++--------
 3 files changed, 27 insertions(+), 8 deletions(-)

diff --git a/hw/9pfs/virtio-9p-chroot-worker.c b/hw/9pfs/virtio-9p-chroot-worker.c
index 40e572c..353aeed 100644
--- a/hw/9pfs/virtio-9p-chroot-worker.c
+++ b/hw/9pfs/virtio-9p-chroot-worker.c
@@ -176,6 +176,20 @@ static int chroot_do_remove(V9fsFileObjectRequest *request)
     return 0;
 }
 
+/*
+ * Rename a file object
+ */
+static int chroot_do_rename(V9fsFileObjectRequest *request)
+{
+    int retval;
+
+    retval = rename(request->path.old_path, request->path.path);
+    if (retval < 0) {
+        return -errno;
+    }
+    return 0;
+}
+
 static void chroot_daemonize(int chroot_sock)
 {
     sigset_t sigset;
@@ -280,6 +294,9 @@ int v9fs_chroot(FsContext *fs_ctx)
         case T_REMOVE:
             retval = chroot_do_remove(&request);
             break;
+        case T_RENAME:
+            retval = chroot_do_rename(&request);
+            break;
         default:
             retval = -1;
             break;
diff --git a/hw/9pfs/virtio-9p-chroot.h b/hw/9pfs/virtio-9p-chroot.h
index 856e9c4..1fa1fde 100644
--- a/hw/9pfs/virtio-9p-chroot.h
+++ b/hw/9pfs/virtio-9p-chroot.h
@@ -10,6 +10,7 @@
 #define T_SYMLINK   5
 #define T_LINK      6
 #define T_REMOVE    7
+#define T_RENAME    8
 
 union MsgControl {
     struct cmsghdr cmsg;
diff --git a/hw/9pfs/virtio-9p-local.c b/hw/9pfs/virtio-9p-local.c
index 7ae21ef..9193687 100644
--- a/hw/9pfs/virtio-9p-local.c
+++ b/hw/9pfs/virtio-9p-local.c
@@ -513,22 +513,23 @@ static int local_truncate(FsContext *ctx, const char *path, off_t size)
 static int local_rename(FsContext *ctx, const char *oldpath,
                         const char *newpath)
 {
+    int err, serrno;
     char *tmp;
-    int err;
 
-    tmp = qemu_strdup(rpath(ctx, oldpath));
+    if (ctx->fs_sm == SM_PASSTHROUGH) {
+        return passthrough_request(ctx, oldpath, newpath, 0, NULL, T_RENAME);
+    }
 
+    tmp = qemu_strdup(rpath(ctx, oldpath));
     err = rename(tmp, rpath(ctx, newpath));
     if (err == -1) {
-        int serrno = errno;
-        qemu_free(tmp);
+        serrno = errno;
+    }
+    qemu_free(tmp);
+    if (err == -1) {
         errno = serrno;
-    } else {
-        qemu_free(tmp);
     }
-
     return err;
-
 }
 
 static int local_chown(FsContext *fs_ctx, const char *path, FsCred *credp)
-- 
1.7.4

[Qemu-devel] [V10 PATCH 10/13] virtio-9p: Move file post creation changes to none security model

[M. Mohan Kumar]

After creating a file object, its permission and ownership details are updated
as per 9p client's request for both passthrough and none security model.
But with chrooted environment its not required for passthrough security model.
Move all post file creation changes to none security model.

Signed-off-by: M. Mohan Kumar <mohan@in.ibm.com>
---
 hw/9pfs/virtio-9p-local.c |   19 ++++++-------------
 1 files changed, 6 insertions(+), 13 deletions(-)

diff --git a/hw/9pfs/virtio-9p-local.c b/hw/9pfs/virtio-9p-local.c
index 9193687..863bade 100644
--- a/hw/9pfs/virtio-9p-local.c
+++ b/hw/9pfs/virtio-9p-local.c
@@ -147,21 +147,14 @@ static int local_set_xattr(const char *path, FsCred *credp)
     return 0;
 }
 
-static int local_post_create_passthrough(FsContext *fs_ctx, const char *path,
+static int local_post_create_none(FsContext *fs_ctx, const char *path,
         FsCred *credp)
 {
+    int retval;
     if (chmod(rpath(fs_ctx, path), credp->fc_mode & 07777) < 0) {
         return -1;
     }
-    if (lchown(rpath(fs_ctx, path), credp->fc_uid, credp->fc_gid) < 0) {
-        /*
-         * If we fail to change ownership and if we are
-         * using security model none. Ignore the error
-         */
-        if (fs_ctx->fs_sm != SM_NONE) {
-            return -1;
-        }
-    }
+    retval = lchown(rpath(fs_ctx, path), credp->fc_uid, credp->fc_gid);
     return 0;
 }
 
@@ -302,7 +295,7 @@ static int local_mknod(FsContext *fs_ctx, const char *path, FsCred *credp)
         if (err == -1) {
             return err;
         }
-        err = local_post_create_passthrough(fs_ctx, path, credp);
+        err = local_post_create_none(fs_ctx, path, credp);
         if (err == -1) {
             serrno = errno;
             goto err_end;
@@ -340,7 +333,7 @@ static int local_mkdir(FsContext *fs_ctx, const char *path, FsCred *credp)
         if (err == -1) {
             return err;
         }
-        err = local_post_create_passthrough(fs_ctx, path, credp);
+        err = local_post_create_none(fs_ctx, path, credp);
         if (err == -1) {
             serrno = errno;
             goto err_end;
@@ -411,7 +404,7 @@ static int local_open2(FsContext *fs_ctx, const char *path, int flags,
         if (fd == -1) {
             return fd;
         }
-        err = local_post_create_passthrough(fs_ctx, path, credp);
+        err = local_post_create_none(fs_ctx, path, credp);
         if (err == -1) {
             serrno = errno;
             goto err_end;
-- 
1.7.4

[Qemu-devel] [V10 PATCH 11/13] virtio-9p: Add support for chmod

[M. Mohan Kumar]

Add support to do chmod operation in chroot process.

Signed-off-by: M. Mohan Kumar <mohan@in.ibm.com>
---
 hw/9pfs/virtio-9p-chroot-worker.c |   18 ++++++++++++++++++
 hw/9pfs/virtio-9p-chroot.h        |    1 +
 hw/9pfs/virtio-9p-local.c         |    5 +++--
 3 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/hw/9pfs/virtio-9p-chroot-worker.c b/hw/9pfs/virtio-9p-chroot-worker.c
index 353aeed..dc92ea2 100644
--- a/hw/9pfs/virtio-9p-chroot-worker.c
+++ b/hw/9pfs/virtio-9p-chroot-worker.c
@@ -190,6 +190,21 @@ static int chroot_do_rename(V9fsFileObjectRequest *request)
     return 0;
 }
 
+/*
+ * Change permissions of a file object
+ * Returns 0 on success and -errno on failure
+ */
+static int chroot_do_chmod(V9fsFileObjectRequest *request)
+{
+    int retval;
+
+    retval = chmod(request->path.path, request->data.mode);
+    if (retval < 0) {
+        return -errno;
+    }
+    return 0;
+}
+
 static void chroot_daemonize(int chroot_sock)
 {
     sigset_t sigset;
@@ -297,6 +312,9 @@ int v9fs_chroot(FsContext *fs_ctx)
         case T_RENAME:
             retval = chroot_do_rename(&request);
             break;
+        case T_CHMOD:
+            retval = chroot_do_chmod(&request);
+            break;
         default:
             retval = -1;
             break;
diff --git a/hw/9pfs/virtio-9p-chroot.h b/hw/9pfs/virtio-9p-chroot.h
index 1fa1fde..8b89598 100644
--- a/hw/9pfs/virtio-9p-chroot.h
+++ b/hw/9pfs/virtio-9p-chroot.h
@@ -11,6 +11,7 @@
 #define T_LINK      6
 #define T_REMOVE    7
 #define T_RENAME    8
+#define T_CHMOD     9
 
 union MsgControl {
     struct cmsghdr cmsg;
diff --git a/hw/9pfs/virtio-9p-local.c b/hw/9pfs/virtio-9p-local.c
index 863bade..cf4e6ff 100644
--- a/hw/9pfs/virtio-9p-local.c
+++ b/hw/9pfs/virtio-9p-local.c
@@ -267,8 +267,9 @@ static int local_chmod(FsContext *fs_ctx, const char *path, FsCred *credp)
 {
     if (fs_ctx->fs_sm == SM_MAPPED) {
         return local_set_xattr(rpath(fs_ctx, path), credp);
-    } else if ((fs_ctx->fs_sm == SM_PASSTHROUGH) ||
-               (fs_ctx->fs_sm == SM_NONE)) {
+    } else if (fs_ctx->fs_sm == SM_PASSTHROUGH) {
+        return passthrough_request(fs_ctx, NULL, path, 0, credp, T_CHMOD);
+    } else if (fs_ctx->fs_sm == SM_NONE) {
         return chmod(rpath(fs_ctx, path), credp->fc_mode);
     }
     return -1;
-- 
1.7.4

[Qemu-devel] [V10 PATCH 12/13] virtio-9p: Add support for chown

[M. Mohan Kumar]

Add support to do chown in chroot process

Signed-off-by: M. Mohan Kumar <mohan@in.ibm.com>
---
 hw/9pfs/virtio-9p-chroot-worker.c |   18 ++++++++++++++++++
 hw/9pfs/virtio-9p-chroot.h        |    1 +
 hw/9pfs/virtio-9p-local.c         |    9 +++++----
 3 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/hw/9pfs/virtio-9p-chroot-worker.c b/hw/9pfs/virtio-9p-chroot-worker.c
index dc92ea2..7893d40 100644
--- a/hw/9pfs/virtio-9p-chroot-worker.c
+++ b/hw/9pfs/virtio-9p-chroot-worker.c
@@ -205,6 +205,21 @@ static int chroot_do_chmod(V9fsFileObjectRequest *request)
     return 0;
 }
 
+/*
+ * Change ownership of a file object
+ * Returns 0 on success and -errno on failure
+ */
+static int chroot_do_chown(V9fsFileObjectRequest *request)
+{
+    int retval;
+
+    retval = lchown(request->path.path, request->data.uid, request->data.gid);
+    if (retval < 0) {
+        return -errno;
+    }
+    return 0;
+}
+
 static void chroot_daemonize(int chroot_sock)
 {
     sigset_t sigset;
@@ -315,6 +330,9 @@ int v9fs_chroot(FsContext *fs_ctx)
         case T_CHMOD:
             retval = chroot_do_chmod(&request);
             break;
+        case T_CHOWN:
+            retval = chroot_do_chown(&request);
+            break;
         default:
             retval = -1;
             break;
diff --git a/hw/9pfs/virtio-9p-chroot.h b/hw/9pfs/virtio-9p-chroot.h
index 8b89598..001297a 100644
--- a/hw/9pfs/virtio-9p-chroot.h
+++ b/hw/9pfs/virtio-9p-chroot.h
@@ -12,6 +12,7 @@
 #define T_REMOVE    7
 #define T_RENAME    8
 #define T_CHMOD     9
+#define T_CHOWN     10
 
 union MsgControl {
     struct cmsghdr cmsg;
diff --git a/hw/9pfs/virtio-9p-local.c b/hw/9pfs/virtio-9p-local.c
index cf4e6ff..def0c62 100644
--- a/hw/9pfs/virtio-9p-local.c
+++ b/hw/9pfs/virtio-9p-local.c
@@ -528,13 +528,14 @@ static int local_rename(FsContext *ctx, const char *oldpath,
 
 static int local_chown(FsContext *fs_ctx, const char *path, FsCred *credp)
 {
-    if ((credp->fc_uid == -1 && credp->fc_gid == -1) ||
-            (fs_ctx->fs_sm == SM_PASSTHROUGH)) {
+    if (fs_ctx->fs_sm != SM_PASSTHROUGH &&
+            (credp->fc_uid == -1 && credp->fc_gid == -1)) {
         return lchown(rpath(fs_ctx, path), credp->fc_uid, credp->fc_gid);
     } else if (fs_ctx->fs_sm == SM_MAPPED) {
         return local_set_xattr(rpath(fs_ctx, path), credp);
-    } else if ((fs_ctx->fs_sm == SM_PASSTHROUGH) ||
-               (fs_ctx->fs_sm == SM_NONE)) {
+    } else if (fs_ctx->fs_sm == SM_PASSTHROUGH) {
+        return passthrough_request(fs_ctx, NULL, path, 0, credp, T_CHOWN);
+    } else if (fs_ctx->fs_sm == SM_NONE) {
         return lchown(rpath(fs_ctx, path), credp->fc_uid, credp->fc_gid);
     }
     return -1;
-- 
1.7.4

[Qemu-devel] [V10 PATCH 13/13] virtio-9p: Chroot environment for other functions

[M. Mohan Kumar]

Add chroot functionality for systemcalls that can operate on a file
using relative directory file descriptor.

Signed-off-by: M. Mohan Kumar <mohan@in.ibm.com>
---
 hw/9pfs/virtio-9p-local.c |  125 +++++++++++++++++++++++++++++++++++++++++----
 1 files changed, 115 insertions(+), 10 deletions(-)

diff --git a/hw/9pfs/virtio-9p-local.c b/hw/9pfs/virtio-9p-local.c
index def0c62..fa5c88f 100644
--- a/hw/9pfs/virtio-9p-local.c
+++ b/hw/9pfs/virtio-9p-local.c
@@ -22,6 +22,8 @@
 #include <sys/socket.h>
 #include <sys/un.h>
 #include <attr/xattr.h>
+#include <libgen.h>
+#include "qemu-error.h"
 
 /* Helper routine to fill V9fsFileObjectRequest structure */
 static int fill_fileobjectrequest(V9fsFileObjectRequest *request,
@@ -80,14 +82,42 @@ static int passthrough_request(FsContext *fs_ctx, const char *old_path,
     return retval;
 }
 
+/*
+ * Returns file descriptor of dirname(path)
+ * This fd can be used by *at functions
+ */
+static int get_dirfd(FsContext *fs_ctx, const char *path)
+{
+    int fd;
+    char *dpath;
+    char *last_component;
+
+    /* path can not contain ".." */
+    last_component = strrchr(path, '/');
+    if (last_component && !strcmp(last_component, "/..")) {
+        error_report("9p path request contains \"..\": %s\n", path);
+        errno = EFAULT;
+        return -1;
+    }
+    dpath = qemu_strdup(path);
+    fd = passthrough_request(fs_ctx, NULL, dirname(dpath), 0, NULL, T_OPEN);
+    qemu_free(dpath);
+    if (fd < 0) {
+        errno = -fd;
+        fd = -1;
+    }
+    return fd;
+}
+
 static int local_lstat(FsContext *fs_ctx, const char *path, struct stat *stbuf)
 {
     int err;
-    err =  lstat(rpath(fs_ctx, path), stbuf);
-    if (err) {
-        return err;
-    }
+
     if (fs_ctx->fs_sm == SM_MAPPED) {
+        err =  lstat(rpath(fs_ctx, path), stbuf);
+        if (err) {
+            return err;
+        }
         /* Actual credentials are part of extended attrs */
         uid_t tmp_uid;
         gid_t tmp_gid;
@@ -109,6 +139,26 @@ static int local_lstat(FsContext *fs_ctx, const char *path, struct stat *stbuf)
                         sizeof(dev_t)) > 0) {
                 stbuf->st_rdev = tmp_dev;
         }
+    } else if (fs_ctx->fs_sm == SM_PASSTHROUGH) {
+        int pfd, serrno = 0;
+        char *tmp_path;
+
+        pfd = get_dirfd(fs_ctx, path);
+        if (pfd < 0) {
+            return -1;
+        }
+        tmp_path = qemu_strdup(path);
+        err = fstatat(pfd, basename(tmp_path), stbuf, AT_SYMLINK_NOFOLLOW);
+        if (err < 0) {
+            serrno = errno;
+        }
+        close(pfd);
+        qemu_free(tmp_path);
+        if (err < 0) {
+            errno = serrno;
+        }
+    } else {
+        err =  lstat(rpath(fs_ctx, path), stbuf);
     }
     return err;
 }
@@ -173,9 +223,26 @@ static ssize_t local_readlink(FsContext *fs_ctx, const char *path,
         } while (tsize == -1 && errno == EINTR);
         close(fd);
         return tsize;
-    } else if ((fs_ctx->fs_sm == SM_PASSTHROUGH) ||
-               (fs_ctx->fs_sm == SM_NONE)) {
+    } else if (fs_ctx->fs_sm == SM_NONE) {
         tsize = readlink(rpath(fs_ctx, path), buf, bufsz);
+    } else if (fs_ctx->fs_sm == SM_PASSTHROUGH) {
+        int pfd, serrno = 0;
+        char *tmp_path;
+        pfd = get_dirfd(fs_ctx, path);
+        if (pfd < 0) {
+            return -1;
+        }
+
+        tmp_path = qemu_strdup(path);
+        tsize = readlinkat(pfd, basename(tmp_path), buf, bufsz);
+        if (tsize < 0) {
+            serrno = errno;
+        }
+        close(pfd);
+        qemu_free(tmp_path);
+        if (tsize < 0) {
+            errno = serrno;
+        }
     }
     return tsize;
 }
@@ -501,7 +568,25 @@ static int local_link(FsContext *fs_ctx, const char *oldpath,
 
 static int local_truncate(FsContext *ctx, const char *path, off_t size)
 {
-    return truncate(rpath(ctx, path), size);
+    if (ctx->fs_sm == SM_PASSTHROUGH) {
+        int fd, retval, serrno;
+        fd = passthrough_request(ctx, NULL, path, O_RDWR, NULL, T_OPEN);
+        if (fd < 0) {
+            errno = -fd;
+            return -1;
+        }
+        retval = ftruncate(fd, size);
+        if (retval < 0) {
+            serrno = errno;
+        }
+        close(fd);
+        if (retval < 0) {
+            errno = serrno;
+        }
+        return retval;
+    } else {
+        return truncate(rpath(ctx, path), size);
+    }
 }
 
 static int local_rename(FsContext *ctx, const char *oldpath,
@@ -541,10 +626,30 @@ static int local_chown(FsContext *fs_ctx, const char *path, FsCred *credp)
     return -1;
 }
 
-static int local_utimensat(FsContext *s, const char *path,
-                           const struct timespec *buf)
+static int local_utimensat(FsContext *fs_ctx, const char *path,
+                const struct timespec *buf)
 {
-    return qemu_utimensat(AT_FDCWD, rpath(s, path), buf, AT_SYMLINK_NOFOLLOW);
+    if (fs_ctx->fs_sm == SM_PASSTHROUGH) {
+        int fd, retval, serrno = 0;
+        fd = passthrough_request(fs_ctx, NULL, path,
+                        O_RDONLY | O_NONBLOCK | O_NOFOLLOW, NULL, T_OPEN);
+        if (fd < 0) {
+            errno = -fd;
+            return -1;
+        }
+        retval = futimens(fd, buf);
+        if (retval < 0) {
+            serrno = errno;
+        }
+        close(fd);
+        if (retval < 0) {
+            errno = serrno;
+        }
+        return retval;
+    } else {
+        return utimensat(AT_FDCWD, rpath(fs_ctx, path), buf,
+                        AT_SYMLINK_NOFOLLOW);
+    }
 }
 
 static int local_remove(FsContext *ctx, const char *path)
-- 
1.7.4

[Qemu-devel] Re: [V10 PATCH 00/13] virtio-9p: Use chroot to safely access files in passthrough security model

[Stefan Hajnoczi]

On Thu, Mar 31, 2011 at 01:51:27PM +0530, M. Mohan Kumar wrote:
> In passthrough security model, following symbolic links in the server
> side could result in TOCTTOU vulnerabilities.
> 
> This patchset resolves this issue by creating a dedicated process which
> chroots into the share path and all file object access is done in the
> chroot environment.
> 
> This patchset implements chroot enviroment, provides necessary functions
> that can be used by the passthrough function calls.
> 
> Changes from version V9:
> * Error handling in special file object creation in virtio-9p-local.c
> * Fix memory leak issue
> 
> Changes from version V8:
> * Make chmod and chown also operate under chroot process
> * Check for invalid path requests, minor cleanups
> 
> Changes from version V7:
> * Add two chroot methods remove and rename
> * Minor cleanups like consolidating functions
> 
> Changes from version V6:
> * Send only fd/errno in socket operations instead of FdInfo structure
> * Minor cleanups
> 
> Changes from version V5:
> * Return errno on failure instead of setting errno
> * Minor cleanups like updated comments, enable CONFIG_THREAD if
>   CONFIG_VIRTFS is enabled
> 
> Changes from version V4:
> * Avoid using malloc/free inside chroot process
> * Seperate chroot server and client functions
> 
> Changes from version V3
> * Return EIO incase of socket read/write fail instead of exiting
> * Changed data types as suggested by Blue Swirl
> * Chroot process reports error through qemu process
> 
> Changes from version V2
> * Treat socket IO errors as fatal, ie qemu will exit
> * Split patchset based on chroot side (server) and qemu side(client)
>   functionalities
> M. Mohan Kumar (13):
>   Implement qemu_read_full
>   virtio-9p: Enable CONFIG_THREAD if CONFIG_VIRTFS is enabled
>   virtio-9p: Provide chroot worker side interfaces
>   virtio-9p: Add qemu side interfaces for chroot environment
>   virtio-9p: Add support to open a file in chroot environment
>   virtio-9p: Create support in chroot environment
>   virtio-9p: Support for creating special files
>   virtio-9p: Add support for removing file or directory
>   virtio-9p: Add support to rename
>   virtio-9p: Move file post creation changes to none security model
>   virtio-9p: Add support for chmod
>   virtio-9p: Add support for chown
>   virtio-9p: Chroot environment for other functions
> 
>  Makefile.objs                     |    1 +
>  configure                         |    1 +
>  hw/9pfs/virtio-9p-chroot-worker.c |  342 +++++++++++++++++++++++++++++++++++++
>  hw/9pfs/virtio-9p-chroot.c        |  105 +++++++++++
>  hw/9pfs/virtio-9p-chroot.h        |   48 +++++
>  hw/9pfs/virtio-9p-local.c         |  306 ++++++++++++++++++++++++++--------
>  hw/9pfs/virtio-9p.c               |   24 +++
>  hw/file-op-9p.h                   |    3 +
>  osdep.c                           |   32 ++++
>  qemu-common.h                     |    2 +
>  10 files changed, 796 insertions(+), 68 deletions(-)
>  create mode 100644 hw/9pfs/virtio-9p-chroot-worker.c
>  create mode 100644 hw/9pfs/virtio-9p-chroot.c
>  create mode 100644 hw/9pfs/virtio-9p-chroot.h
> 
> -- 

Reviewed-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>