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

[Qemu-devel] [V8 PATCH 00/11] 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 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 (11):
  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: Chroot environment for other functions

 Makefile.objs                     |    1 +
 configure                         |    1 +
 hw/9pfs/virtio-9p-chroot-worker.c |  306 ++++++++++++++++++++++++++++++++
 hw/9pfs/virtio-9p-chroot.c        |  104 +++++++++++
 hw/9pfs/virtio-9p-chroot.h        |   47 +++++
 hw/9pfs/virtio-9p-local.c         |  355 ++++++++++++++++++++++++++++--------
 hw/9pfs/virtio-9p.c               |   25 +++
 hw/file-op-9p.h                   |    4 +
 osdep.c                           |   32 ++++
 qemu-common.h                     |    2 +
 10 files changed, 798 insertions(+), 79 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.3.4

[Qemu-devel] [V8 PATCH 01/11] 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.3.4

[Qemu-devel] [V8 PATCH 02/11] 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 2560357..9eddd38 100755
--- a/configure
+++ b/configure
@@ -2767,6 +2767,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.3.4

[Qemu-devel] [V8 PATCH 03/11] 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        |   36 +++++++
 hw/9pfs/virtio-9p.c               |   25 +++++
 hw/file-op-9p.h                   |    4 +
 5 files changed, 249 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 6a9f765..44891c1 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -283,6 +283,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..e7bc6c2
--- /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 fd, 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:
+            fd = chroot_do_open(&request);
+            if (fd >= 0) {
+                valid_fd = 1;
+            }
+            break;
+        default:
+            fd = -1;
+            break;
+        }
+        chroot_sendfd(chroot_sock, fd, valid_fd);
+    }
+}
diff --git a/hw/9pfs/virtio-9p-chroot.h b/hw/9pfs/virtio-9p-chroot.h
new file mode 100644
index 0000000..c05e05b8
--- /dev/null
+++ b/hw/9pfs/virtio-9p-chroot.h
@@ -0,0 +1,36 @@
+#ifndef _QEMU_VIRTIO_9P_CHROOT_H
+#define _QEMU_VIRTIO_9P_CHROOT_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 08c0399..4dea542 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 void v9fs_reclaim_fd(V9fsState *s);
@@ -3925,5 +3927,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);
+        s->ctx.chroot_ioerror = 0;
+        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 126e60e..b52b432 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
 
@@ -55,6 +56,9 @@ typedef struct FsContext
     SecModel fs_sm;
     uid_t uid;
     struct xattr_operations **xops;
+    QemuMutex chroot_mutex;
+    int chroot_socket;
+    int chroot_ioerror;
 } FsContext;
 
 void cred_init(FsCred *);
-- 
1.7.3.4

[Qemu-devel] [V8 PATCH 04/11] 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 44891c1..6610de9 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -283,7 +283,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 c05e05b8..a218f95 100644
--- a/hw/9pfs/virtio-9p-chroot.h
+++ b/hw/9pfs/virtio-9p-chroot.h
@@ -32,5 +32,6 @@ typedef struct V9fsFileObjectRequest
 } V9fsFileObjectRequest;
 
 int v9fs_chroot(FsContext *fs_ctx);
+void chroot_dummy(void);
 
 #endif /* _QEMU_VIRTIO_9P_CHROOT_H */
-- 
1.7.3.4

[Qemu-devel] [V8 PATCH 05/11] 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 |   28 ++++++++++++-----
 hw/9pfs/virtio-9p-chroot.h |    2 +-
 hw/9pfs/virtio-9p-local.c  |   70 +++++++++++++++++++++++++++++++++++++++++--
 3 files changed, 87 insertions(+), 13 deletions(-)

diff --git a/hw/9pfs/virtio-9p-chroot.c b/hw/9pfs/virtio-9p-chroot.c
index 4aa3b43..0c91db6 100644
--- a/hw/9pfs/virtio-9p-chroot.c
+++ b/hw/9pfs/virtio-9p-chroot.c
@@ -80,13 +80,25 @@ 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_ioerror) {
+        fd = -EIO;
+        goto unlock;
+    }
+    if (v9fs_write_request(fs_ctx->chroot_socket, request) < 0) {
+        fs_ctx->chroot_ioerror = 1;
+        fd = -EIO;
+        goto unlock;
+    }
+    fd = v9fs_receivefd(fs_ctx->chroot_socket, &sock_error);
+    if (fd < 0 && sock_error) {
+        fs_ctx->chroot_ioerror = 1;
+    }
+unlock:
+    qemu_mutex_unlock(&fs_ctx->chroot_mutex);
+    return fd;
 }
diff --git a/hw/9pfs/virtio-9p-chroot.h b/hw/9pfs/virtio-9p-chroot.h
index a218f95..ceb858d 100644
--- a/hw/9pfs/virtio-9p-chroot.h
+++ b/hw/9pfs/virtio-9p-chroot.h
@@ -32,6 +32,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 0a015de..9ba1644 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,52 @@
 #include <sys/un.h>
 #include <attr/xattr.h>
 
+/* Helper routine to fill V9fsFileObjectRequest structure */
+static int fill_fileobjectrequest(V9fsFileObjectRequest *request,
+                const char *path, FsCred *credp)
+{
+    if (strlen(path) >= PATH_MAX) {
+        return -ENAMETOOLONG;
+    }
+    memset(request, 0, sizeof(*request));
+    strcpy(request->path.path, path);
+    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;
+    }
+    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, path, credp);
+    if (retval < 0) {
+        errno = -retval;
+        return -1;
+    }
+    if (old_path) {
+        if (strlen(old_path) >= PATH_MAX) {
+            errno = -ENAMETOOLONG;
+            return -1;
+        }
+        strcpy(request.path.old_path, old_path);
+    }
+
+    request.data.flags = flags;
+    request.data.type = type;
+    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 +187,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.3.4

[Qemu-devel] [V8 PATCH 06/11] 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 e7bc6c2..cdcb303 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:
+            fd = chroot_do_create(&request);
+            if (fd >= 0) {
+                valid_fd = 1;
+            }
+            break;
         default:
             fd = -1;
             break;
diff --git a/hw/9pfs/virtio-9p-chroot.h b/hw/9pfs/virtio-9p-chroot.h
index ceb858d..298d5b5 100644
--- a/hw/9pfs/virtio-9p-chroot.h
+++ b/hw/9pfs/virtio-9p-chroot.h
@@ -3,6 +3,7 @@
 
 /* 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 9ba1644..625d29c 100644
--- a/hw/9pfs/virtio-9p-local.c
+++ b/hw/9pfs/virtio-9p-local.c
@@ -394,8 +394,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;
@@ -405,6 +404,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.3.4

[Qemu-devel] [V8 PATCH 07/11] 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         |   71 +++++++++++++++++++++----------------
 3 files changed, 97 insertions(+), 31 deletions(-)

diff --git a/hw/9pfs/virtio-9p-chroot-worker.c b/hw/9pfs/virtio-9p-chroot-worker.c
index cdcb303..fcc0be1 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:
+            fd = chroot_do_create_special(&request);
+            break;
         default:
             fd = -1;
             break;
diff --git a/hw/9pfs/virtio-9p-chroot.h b/hw/9pfs/virtio-9p-chroot.h
index 298d5b5..b2db276 100644
--- a/hw/9pfs/virtio-9p-chroot.h
+++ b/hw/9pfs/virtio-9p-chroot.h
@@ -4,6 +4,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;
@@ -34,5 +38,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 625d29c..e7d39c0 100644
--- a/hw/9pfs/virtio-9p-local.c
+++ b/hw/9pfs/virtio-9p-local.c
@@ -287,8 +287,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;
@@ -298,6 +297,12 @@ 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);
+        if (err < 0) {
+            serrno = errno;
+            goto err_end;
+        }
     }
     return err;
 
@@ -324,8 +329,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;
@@ -335,6 +339,12 @@ 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);
+        if (err < 0) {
+            serrno = errno;
+            goto err_end;
+        }
     }
     return err;
 
@@ -452,23 +462,19 @@ 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);
+        if (err < 0) {
+            serrno = errno;
+            goto err_end;
         }
     }
     return err;
@@ -479,24 +485,27 @@ 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;
 
-    if (tmp == NULL) {
-        return -ENOMEM;
-    }
-
-    err = link(tmp, rpath(ctx, newpath));
-    if (err == -1) {
-        serrno = errno;
-    }
-
-    qemu_free(tmp);
-
-    if (err == -1) {
-        errno = serrno;
+    if (fs_ctx->fs_sm == SM_PASSTHROUGH) {
+        err = passthrough_request(fs_ctx, oldpath, newpath, 0, NULL,
+                        T_LINK);
+        if (err < 0) {
+            serrno = errno;
+        }
+    } else {
+        char *tmp = qemu_strdup(rpath(fs_ctx, oldpath));
+        if (tmp == NULL) {
+            return -ENOMEM;
+        }
+        err = link(tmp, rpath(fs_ctx, newpath));
+        if (err == -1) {
+            serrno = errno;
+        }
+        qemu_free(tmp);
     }
 
     return err;
-- 
1.7.3.4

[Qemu-devel] [V8 PATCH 08/11] 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 |   34 ++++++++++++++++++++++++++--------
 hw/9pfs/virtio-9p-chroot.h        |    1 +
 hw/9pfs/virtio-9p-local.c         |    6 +++++-
 3 files changed, 32 insertions(+), 9 deletions(-)

diff --git a/hw/9pfs/virtio-9p-chroot-worker.c b/hw/9pfs/virtio-9p-chroot-worker.c
index fcc0be1..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;
@@ -199,7 +214,7 @@ int v9fs_chroot(FsContext *fs_ctx)
     V9fsFileObjectRequest request;
     pid_t pid;
     uint32_t code;
-    int fd, valid_fd;
+    int retval, valid_fd;
 
     if (socketpair(PF_UNIX, SOCK_STREAM, 0, fd_pair) < 0) {
         error_report("socketpair %s", strerror(errno));
@@ -245,14 +260,14 @@ int v9fs_chroot(FsContext *fs_ctx)
         }
         switch (request.data.type) {
         case T_OPEN:
-            fd = chroot_do_open(&request);
-            if (fd >= 0) {
+            retval = chroot_do_open(&request);
+            if (retval >= 0) {
                 valid_fd = 1;
             }
             break;
         case T_CREATE:
-            fd = chroot_do_create(&request);
-            if (fd >= 0) {
+            retval = chroot_do_create(&request);
+            if (retval >= 0) {
                 valid_fd = 1;
             }
             break;
@@ -260,12 +275,15 @@ int v9fs_chroot(FsContext *fs_ctx)
         case T_SYMLINK:
         case T_LINK:
         case T_MKNOD:
-            fd = chroot_do_create_special(&request);
+            retval = chroot_do_create_special(&request);
+            break;
+        case T_REMOVE:
+            retval = chroot_do_remove(&request);
             break;
         default:
-            fd = -1;
+            retval = -1;
             break;
         }
-        chroot_sendfd(chroot_sock, fd, valid_fd);
+        chroot_sendfd(chroot_sock, retval, valid_fd);
     }
 }
diff --git a/hw/9pfs/virtio-9p-chroot.h b/hw/9pfs/virtio-9p-chroot.h
index b2db276..2d7984a 100644
--- a/hw/9pfs/virtio-9p-chroot.h
+++ b/hw/9pfs/virtio-9p-chroot.h
@@ -8,6 +8,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 e7d39c0..cdbaf6a 100644
--- a/hw/9pfs/virtio-9p-local.c
+++ b/hw/9pfs/virtio-9p-local.c
@@ -559,7 +559,11 @@ static int local_utimensat(FsContext *s, const char *path,
 
 static int local_remove(FsContext *ctx, const char *path)
 {
-    return remove(rpath(ctx, path));
+    if (ctx->fs_sm == SM_PASSTHROUGH) {
+        return passthrough_request(ctx, NULL, path, 0, NULL, T_REMOVE);
+    } else {
+        return remove(rpath(ctx, path));
+    }
 }
 
 static int local_fsync(FsContext *ctx, int fd, int datasync)
-- 
1.7.3.4

[Qemu-devel] [V8 PATCH 09/11] 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         |   26 +++++++++++++++-----------
 3 files changed, 33 insertions(+), 11 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 2d7984a..e8251b4 100644
--- a/hw/9pfs/virtio-9p-chroot.h
+++ b/hw/9pfs/virtio-9p-chroot.h
@@ -9,6 +9,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 cdbaf6a..cf4792b 100644
--- a/hw/9pfs/virtio-9p-local.c
+++ b/hw/9pfs/virtio-9p-local.c
@@ -519,22 +519,26 @@ static int local_truncate(FsContext *ctx, const char *path, off_t size)
 static int local_rename(FsContext *ctx, const char *oldpath,
                         const char *newpath)
 {
-    char *tmp;
-    int err;
-
-    tmp = qemu_strdup(rpath(ctx, oldpath));
+    int err, serrno;
 
-    err = rename(tmp, rpath(ctx, newpath));
-    if (err == -1) {
-        int serrno = errno;
-        qemu_free(tmp);
-        errno = serrno;
+    if (ctx->fs_sm == SM_PASSTHROUGH) {
+        err = passthrough_request(ctx, oldpath, newpath, 0, NULL, T_RENAME);
+        if (err == -1) {
+            serrno = errno;
+        }
     } else {
+        char *tmp;
+        tmp = qemu_strdup(rpath(ctx, oldpath));
+        err = rename(tmp, rpath(ctx, newpath));
+        if (err == -1) {
+            serrno = errno;
+        }
         qemu_free(tmp);
     }
-
+    if (err == -1) {
+        errno = serrno;
+    }
     return err;
-
 }
 
 static int local_chown(FsContext *fs_ctx, const char *path, FsCred *credp)
-- 
1.7.3.4

[Qemu-devel] [V8 PATCH 10/11] 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 cf4792b..1670e33 100644
--- a/hw/9pfs/virtio-9p-local.c
+++ b/hw/9pfs/virtio-9p-local.c
@@ -137,21 +137,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;
 }
 
@@ -292,7 +285,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;
@@ -334,7 +327,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;
@@ -409,7 +402,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.3.4

[Qemu-devel] [V8 PATCH 11/11] 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 |  156 ++++++++++++++++++++++++++++++++++++++++-----
 1 files changed, 139 insertions(+), 17 deletions(-)

diff --git a/hw/9pfs/virtio-9p-local.c b/hw/9pfs/virtio-9p-local.c
index 1670e33..0157eb8 100644
--- a/hw/9pfs/virtio-9p-local.c
+++ b/hw/9pfs/virtio-9p-local.c
@@ -22,6 +22,7 @@
 #include <sys/socket.h>
 #include <sys/un.h>
 #include <attr/xattr.h>
+#include <libgen.h>
 
 /* Helper routine to fill V9fsFileObjectRequest structure */
 static int fill_fileobjectrequest(V9fsFileObjectRequest *request,
@@ -70,14 +71,41 @@ 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)
+{
+    V9fsFileObjectRequest request;
+    int fd;
+    char *dpath = qemu_strdup(path);
+
+    fd = fill_fileobjectrequest(&request, dirname(dpath), NULL);
+    if (fd < 0) {
+        qemu_free(dpath);
+        errno = -fd;
+        return -1;
+    }
+    request.data.type = T_OPEN;
+    fd = v9fs_request(fs_ctx, &request);
+    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;
@@ -99,6 +127,27 @@ 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);
+        errno = serrno;
+    } else {
+        err =  lstat(rpath(fs_ctx, path), stbuf);
+        if (err) {
+            return err;
+        }
     }
     return err;
 }
@@ -163,9 +212,23 @@ 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);
+        errno = serrno;
     }
     return tsize;
 }
@@ -257,8 +320,23 @@ 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) {
+        int pfd, err, serrno = 0;
+        char *tmp_path;
+        pfd = get_dirfd(fs_ctx, path);
+        if (pfd < 0) {
+            return -1;
+        }
+        tmp_path = qemu_strdup(path);
+        err = fchmodat(pfd, basename(tmp_path), credp->fc_mode, 0);
+        if (err == -1) {
+            serrno = errno;
+        }
+        qemu_free(tmp_path);
+        close(pfd);
+        errno = serrno;
+        return err;
+    } else if (fs_ctx->fs_sm == SM_NONE) {
         return chmod(rpath(fs_ctx, path), credp->fc_mode);
     }
     return -1;
@@ -506,7 +584,21 @@ 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);
+        serrno = errno;
+        close(fd);
+        errno = serrno;
+        return retval;
+    } else {
+        return truncate(rpath(ctx, path), size);
+    }
 }
 
 static int local_rename(FsContext *ctx, const char *oldpath,
@@ -536,22 +628,52 @@ 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_NONE) {
         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)) {
-        return lchown(rpath(fs_ctx, path), credp->fc_uid, credp->fc_gid);
+    } else if (fs_ctx->fs_sm == SM_PASSTHROUGH) {
+        int pfd, err, serrno = 0;
+        char *old_path;
+        pfd = get_dirfd(fs_ctx, path);
+        if (pfd < 0) {
+            return -1;
+        }
+        old_path = qemu_strdup(path);
+        err = fchownat(pfd, basename(old_path), credp->fc_uid, credp->fc_gid,
+                        AT_SYMLINK_NOFOLLOW);
+        if (err == -1) {
+            serrno = errno;
+        }
+        qemu_free(old_path);
+        close(pfd);
+        errno = serrno;
+        return err;
     }
     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;
+        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);
+        close(fd);
+        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.3.4

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

[Stefan Hajnoczi]

On Wed, Mar 9, 2011 at 5:15 PM, M. Mohan Kumar <mohan@in.ibm.com> wrote:
> +/* 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_ioerror) {
> +        fd = -EIO;
> +        goto unlock;
> +    }
> +    if (v9fs_write_request(fs_ctx->chroot_socket, request) < 0) {
> +        fs_ctx->chroot_ioerror = 1;
> +        fd = -EIO;
> +        goto unlock;
> +    }
> +    fd = v9fs_receivefd(fs_ctx->chroot_socket, &sock_error);
> +    if (fd < 0 && sock_error) {
> +        fs_ctx->chroot_ioerror = 1;
> +    }
> +unlock:
> +    qemu_mutex_unlock(&fs_ctx->chroot_mutex);
> +    return fd;
>  }

If the socket is broken why not just close it?  Right now we're
keeping the file descriptor and have an additional chroot_ioerror
variable to keep track of the fact that we don't want to touch the
socket.

> +/* Helper routine to fill V9fsFileObjectRequest structure */
> +static int fill_fileobjectrequest(V9fsFileObjectRequest *request,
> +                const char *path, FsCred *credp)
> +{
> +    if (strlen(path) >= PATH_MAX) {
> +        return -ENAMETOOLONG;
> +    }
> +    memset(request, 0, sizeof(*request));

Perhaps remove this since request is a big struct (two PATH_MAX
buffers) or is it necessary?

> +    strcpy(request->path.path, path);
> +    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;
> +    }
> +    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, path, credp);

This function could also handle old_path, flags, and type.  It seems
to only fill half the request struct at the moment.

> +    if (retval < 0) {
> +        errno = -retval;
> +        return -1;
> +    }
> +    if (old_path) {
> +        if (strlen(old_path) >= PATH_MAX) {
> +            errno = -ENAMETOOLONG;

errno = ENAMETOOLONG;

Stefan

[Qemu-devel] Re: [V8 PATCH 11/11] virtio-9p: Chroot environment for other functions

[Stefan Hajnoczi]

On Wed, Mar 9, 2011 at 5:16 PM, M. Mohan Kumar <mohan@in.ibm.com> wrote:
> Add chroot functionality for systemcalls that can operate on a file
> using relative directory file descriptor.

I suspect the relative directory approach is broken and escapes the
chroot.  Here's why:

The request is local_chmod(fs_ctx, "/..", credp).  dirname("/..") is
"/" and basename("..") is "..".

I'm not 100% sure of the semantics but I suspect that chmodat(dir_fd,
"..", ...) does not honor the chroot since your current task is not
inside the chroot.  If so, then you can manipulate the parent
directory of the chroot using some of the operations added in this
patch.

The safe solution is to perform all operations inside the chroot.
This will require extending the chroot socket protocol.

Stefan

Re: [Qemu-devel] Re: [V8 PATCH 11/11] virtio-9p: Chroot environment for other functions

[Venkateswararao Jujjuri (JV)]

On 3/10/2011 4:29 AM, Stefan Hajnoczi wrote:
> On Wed, Mar 9, 2011 at 5:16 PM, M. Mohan Kumar <mohan@in.ibm.com> wrote:
>> Add chroot functionality for systemcalls that can operate on a file
>> using relative directory file descriptor.
> 
> I suspect the relative directory approach is broken and escapes the
> chroot.  Here's why:
> 
> The request is local_chmod(fs_ctx, "/..", credp).  dirname("/..") is
> "/" and basename("..") is "..".

We should never receive protocol operations with relative path.
Client should always resolve to full path and send the request.
If the client is malicious this scenario can be be possible.. but in that case
it is fine to fail the operation.

Thanks,
JV

> I'm not 100% sure of the semantics but I suspect that chmodat(dir_fd,
> "..", ...) does not honor the chroot since your current task is not
> inside the chroot.  If so, then you can manipulate the parent
> directory of the chroot using some of the operations added in this
> patch.
> 
> The safe solution is to perform all operations inside the chroot.
> This will require extending the chroot socket protocol.
> 
> Stefan
> 

Re: [Qemu-devel] Re: [V8 PATCH 11/11] virtio-9p: Chroot environment for other functions

[Stefan Hajnoczi]

On Fri, Mar 11, 2011 at 5:54 AM, Venkateswararao Jujjuri (JV)
<jvrao@linux.vnet.ibm.com> wrote:
> On 3/10/2011 4:29 AM, Stefan Hajnoczi wrote:
>> On Wed, Mar 9, 2011 at 5:16 PM, M. Mohan Kumar <mohan@in.ibm.com> wrote:
>>> Add chroot functionality for systemcalls that can operate on a file
>>> using relative directory file descriptor.
>>
>> I suspect the relative directory approach is broken and escapes the
>> chroot.  Here's why:
>>
>> The request is local_chmod(fs_ctx, "/..", credp).  dirname("/..") is
>> "/" and basename("..") is "..".
>
> We should never receive protocol operations with relative path.
> Client should always resolve to full path and send the request.
> If the client is malicious this scenario can be be possible.. but in that case
> it is fine to fail the operation.

What I haven't audited yet is whether symlinks can be abused in any of
these *at(2) operations.

The *at(2) approach seems like a shortcut to avoid implementing
individual chroot protocol requests/responses for stat(2) and friends.
 But it carries the risk that if we don't use NOFOLLOW then we can be
tricked into escaping the "chroot" because we're performing the
operation outside the chroot.

I'll take a look later today to make sure all operations safe traverse
paths outside the chroot.

Stefan

Re: [Qemu-devel] Re: [V8 PATCH 11/11] virtio-9p: Chroot environment for other functions

[Venkateswararao Jujjuri (JV)]

On 3/10/2011 10:30 PM, Stefan Hajnoczi wrote:
> On Fri, Mar 11, 2011 at 5:54 AM, Venkateswararao Jujjuri (JV)
> <jvrao@linux.vnet.ibm.com> wrote:
>> On 3/10/2011 4:29 AM, Stefan Hajnoczi wrote:
>>> On Wed, Mar 9, 2011 at 5:16 PM, M. Mohan Kumar <mohan@in.ibm.com> wrote:
>>>> Add chroot functionality for systemcalls that can operate on a file
>>>> using relative directory file descriptor.
>>>
>>> I suspect the relative directory approach is broken and escapes the
>>> chroot.  Here's why:
>>>
>>> The request is local_chmod(fs_ctx, "/..", credp).  dirname("/..") is
>>> "/" and basename("..") is "..".
>>
>> We should never receive protocol operations with relative path.
>> Client should always resolve to full path and send the request.
>> If the client is malicious this scenario can be be possible.. but in that case
>> it is fine to fail the operation.
> 
> What I haven't audited yet is whether symlinks can be abused in any of
> these *at(2) operations.

Reading symlink sends only contents to the client. So a symlink can contain
anything.
But when the fully populated path comes we avoid the potential symlink issue by
opening
the entire dir in chrooted process.
> 
> The *at(2) approach seems like a shortcut to avoid implementing
> individual chroot protocol requests/responses for stat(2) and friends.
>  But it carries the risk that if we don't use NOFOLLOW then we can be
> tricked into escaping the "chroot" because we're performing the
> operation outside the chroot.

I would agree with your observation. With *at(2) we need the following:

1. The path should never have ".." May be we can check it early enough and fail.
2. Get the pfd from the chroot_thread
3. use NO_FOLLOW.

If we do all three then we are fool prof.
> 
> I'll take a look later today to make sure all operations safe traverse
> paths outside the chroot.

If we move all the operations to chroot, we are essentially serializing all
operations.
But the code looks lot cleaner though.

- JV


> 
> Stefan
>