[Qemu-devel] [PATCH V12 00/15] virtio-9p: chroot environment for passthrough security model

[Qemu-devel] [PATCH V12 00/15] virtio-9p: chroot environment for passthrough security model

[M. Mohan Kumar]

In passthrough security model, following symbolic links in the server
side could result in TOCTTOU vulnerabilities.
(http://en.wikipedia.org/wiki/Time-of-check-to-time-of-use)

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

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

Qemu need to be invoked by root user for using virtfs with passthrough
security model (i.e to use chroot() syscall).

Question is: Is running qemu by root user expected and allowed? Some of the
virtfs features can be utilized only if qemu is started by root user (for
example passthrough security model and handle based file driver need root
privilege).

This issue can be resolved by root user starting qemu and spawning a process
with root privilege to do all privileged operations there and main qemu
process dropping its privileges to avoid any security issue in running qemu in
root mode. Privileged operations can be done similar to the chroot patchset.

But how to determine to which user-id(ie non root user id) qemu needs to drop
the privileges? Do we have a common user-id across all distributions/systems
to which qemu process can be dropped down? Also it becomes too complex i.e when
a new feature needing root privilege is added, a process with root privilege
needs to be created to handle this.

So is it allowed to run qemu by root user? If no, is it okay to add the
complexity of adding a root privilege process for each feature that needs root
privilege?

Changes from version V11:
* Rebased on top of latest qemu tree
* Moved chroot process creation into local_init function
* g_malloc/g_free instead qemu_malloc/g_free
* Rename qemu_recv function to chroot_recv

Changes from version V10:
* Added support to do lstat and readlink from chroot process
* Fixed an issue with dealing fds when qemu process reached maxfds limit

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

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 (15):
  Implement qemu_read_full
  virtio-9p: Enable CONFIG_THREAD if CONFIG_VIRTFS is enabled
  virtio-9p: Provide chroot worker side interfaces
  virtio-9p: qemu interfaces for chroot environment
  virtio-9p: Support for opening a file in chroot environment
  virtio-9p: Create support in chroot environment
  virtio-9p: Creating special files in chroot environment
  virtio-9p: Removing file or directory in chroot environment
  virtio-9p: Rename in chroot environment
  virtio-9p: Move file post creation changes to none security model
  virtio-9p: chmod in chroot environment
  virtio-9p: chown in chroot environment
  virtio-9p: stat in chroot environment
  virtio-9p: readlink in chroot environment
  virtio-9p: Chroot environment for other functions

 Makefile.objs                     |    1 +
 configure                         |    1 +
 fsdev/file-op-9p.h                |    3 +
 hw/9pfs/virtio-9p-chroot-worker.c |  413 +++++++++++++++++++++++++++++++++++++
 hw/9pfs/virtio-9p-chroot.c        |  173 ++++++++++++++++
 hw/9pfs/virtio-9p-chroot.h        |   54 +++++
 hw/9pfs/virtio-9p-device.c        |    1 +
 hw/9pfs/virtio-9p-local.c         |  277 ++++++++++++++++++++-----
 osdep.c                           |   32 +++
 qemu-common.h                     |    2 +
 10 files changed, 907 insertions(+), 50 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.5.4

[Qemu-devel] [PATCH V12 01/15] Implement qemu_read_full

[M. Mohan Kumar]

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 56e6963..5a4d670 100644
--- a/osdep.c
+++ b/osdep.c
@@ -126,6 +126,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 404c421..d6aabd2 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -189,6 +189,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.6

[Qemu-devel] [PATCH V12 02/15] hw/9pfs: 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 1340c33..ad59fcc 100755
--- a/configure
+++ b/configure
@@ -2980,6 +2980,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.6

[Qemu-devel] [PATCH V12 03/15] hw/9pfs: 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>
[malahal@us.ibm.com: Do not send fd as part of data, instead a special
value is sent as part of data when fd is sent as part of ancillary data]
---
 Makefile.objs                     |    1 +
 fsdev/file-op-9p.h                |    3 +
 hw/9pfs/virtio-9p-chroot-worker.c |  193 +++++++++++++++++++++++++++++++++++++
 hw/9pfs/virtio-9p-chroot.h        |   40 ++++++++
 hw/9pfs/virtio-9p-device.c        |   24 +++++
 5 files changed, 261 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 20c9e2b..01e9350 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -308,6 +308,7 @@ hw-obj-$(CONFIG_SOUND) += $(sound-obj-y)
 9pfs-nested-$(CONFIG_VIRTFS) += virtio-9p-coth.o cofs.o codir.o cofile.o
 9pfs-nested-$(CONFIG_VIRTFS) += coxattr.o virtio-9p-handle.o
 9pfs-nested-$(CONFIG_VIRTFS) += virtio-9p-synth.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)): QEMU_CFLAGS+=$(GLIB_CFLAGS)
diff --git a/fsdev/file-op-9p.h b/fsdev/file-op-9p.h
index 076af22..5547a2f 100644
--- a/fsdev/file-op-9p.h
+++ b/fsdev/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
@@ -72,6 +73,8 @@ typedef struct FsContext
     struct extended_ops exops;
     /* fs driver specific data */
     void *private;
+    QemuMutex chroot_mutex;
+    int chroot_socket;
 } FsContext;
 
 typedef struct V9fsPath {
diff --git a/hw/9pfs/virtio-9p-chroot-worker.c b/hw/9pfs/virtio-9p-chroot-worker.c
new file mode 100644
index 0000000..40a54b3
--- /dev/null
+++ b/hw/9pfs/virtio-9p-chroot-worker.c
@@ -0,0 +1,193 @@
+/*
+ * 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 "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, data;
+    union MsgControl msg_control;
+
+    iov.iov_base = &data;
+    iov.iov_len = sizeof(data);
+
+    memset(&msg, 0, sizeof(msg));
+    msg.msg_iov = &iov;
+    msg.msg_iovlen = 1;
+    /* No ancillary data on error */
+    if (!fd_valid) {
+        /*
+         * fd is really negative errno if the request failed. Or simply
+         * zero if the request is successful and it doesn't need a file
+         * descriptor.
+         */
+        data = fd;
+    } else {
+        data = V9FS_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;
+    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;
+        if (qemu_write_full(chroot_sock, &code, sizeof(code)) < 0) {
+                /* No need to process error, we are about to exit */
+        }
+        _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..a817bcf
--- /dev/null
+++ b/hw/9pfs/virtio-9p-chroot.h
@@ -0,0 +1,40 @@
+#ifndef _QEMU_VIRTIO_9P_CHROOT_H
+#define _QEMU_VIRTIO_9P_CHROOT_H
+
+#include "qemu_socket.h"
+/* types for V9fsFileObjectRequest */
+#define T_OPEN      1
+
+#define V9FS_FD_VALID INT_MAX
+
+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);
+void chroot_dummy(void);
+
+#endif /* _QEMU_VIRTIO_9P_CHROOT_H */
diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
index abd4795..4518123 100644
--- a/hw/9pfs/virtio-9p-device.c
+++ b/hw/9pfs/virtio-9p-device.c
@@ -19,6 +19,8 @@
 #include "fsdev/qemu-fsdev.h"
 #include "virtio-9p-xattr.h"
 #include "virtio-9p-coth.h"
+#include "qemu-error.h"
+#include "virtio-9p-chroot.h"
 
 static uint32_t virtio_9p_get_features(VirtIODevice *vdev, uint32_t features)
 {
@@ -152,6 +154,28 @@ VirtIODevice *virtio_9p_init(DeviceState *dev, V9fsConf *conf)
         fprintf(stderr, "worker thread initialization failed\n");
         exit(1);
     }
+    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;
 }
 
-- 
1.7.6

[Qemu-devel] [PATCH V12 04/15] hw/9pfs: qemu 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>
[malahal@us.ibm.com: Handle when qemu process can not receive fd because
it already reached max fds]
---
 Makefile.objs              |    2 +-
 hw/9pfs/virtio-9p-chroot.c |  103 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 104 insertions(+), 1 deletions(-)
 create mode 100644 hw/9pfs/virtio-9p-chroot.c

diff --git a/Makefile.objs b/Makefile.objs
index 01e9350..fa8a755 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -308,7 +308,7 @@ hw-obj-$(CONFIG_SOUND) += $(sound-obj-y)
 9pfs-nested-$(CONFIG_VIRTFS) += virtio-9p-coth.o cofs.o codir.o cofile.o
 9pfs-nested-$(CONFIG_VIRTFS) += coxattr.o virtio-9p-handle.o
 9pfs-nested-$(CONFIG_VIRTFS) += virtio-9p-synth.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)): QEMU_CFLAGS+=$(GLIB_CFLAGS)
diff --git a/hw/9pfs/virtio-9p-chroot.c b/hw/9pfs/virtio-9p-chroot.c
new file mode 100644
index 0000000..63de410
--- /dev/null
+++ b/hw/9pfs/virtio-9p-chroot.c
@@ -0,0 +1,103 @@
+/*
+ * 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 "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, data, fd;
+
+    iov.iov_base = &data;
+    iov.iov_len = sizeof(data);
+
+    *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;
+    }
+
+    /*
+     * data is set to V9FS_FD_VALID, if ancillary data is sent.  If this
+     * request doesn't need ancillary data (fd) or an error occurred,
+     * data is set to negative errno value.
+     */
+    if (data != V9FS_FD_VALID) {
+        return data;
+    }
+
+    /*
+     * File descriptor (fd) is sent in the ancillary data. Check if we
+     * indeed received it. One of the reasons to fail to receive it is if
+     * we exceeded the maximum number of file descriptors!
+     */
+    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 -ENFILE; /* Ancillary data sent but not received */
+}
+
+/*
+ * 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 caller. To avoid compiler warning message,
+ * refer these two functions
+ */
+void chroot_dummy(void)
+{
+    (void)v9fs_receivefd;
+    (void)v9fs_write_request;
+}
-- 
1.7.6

[Qemu-devel] [PATCH V12 05/15] hw/9pfs: Support for opening 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  |   79 ++++++++++++++++++++++++++++++++++++++++++--
 3 files changed, 98 insertions(+), 12 deletions(-)

diff --git a/hw/9pfs/virtio-9p-chroot.c b/hw/9pfs/virtio-9p-chroot.c
index 63de410..f5b3abc 100644
--- a/hw/9pfs/virtio-9p-chroot.c
+++ b/hw/9pfs/virtio-9p-chroot.c
@@ -91,13 +91,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 caller. 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 a817bcf..326238d 100644
--- a/hw/9pfs/virtio-9p-chroot.h
+++ b/hw/9pfs/virtio-9p-chroot.h
@@ -35,6 +35,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 7a46e93..a91adb8 100644
--- a/hw/9pfs/virtio-9p-local.c
+++ b/hw/9pfs/virtio-9p-local.c
@@ -14,6 +14,9 @@
 #include "hw/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>
@@ -24,6 +27,63 @@
 #include <linux/magic.h>
 #include <sys/ioctl.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, V9fsPath *fs_path, struct stat *stbuf)
 {
     int err;
@@ -157,7 +217,11 @@ static int local_open(FsContext *ctx, V9fsPath *fs_path,
     char buffer[PATH_MAX];
     char *path = fs_path->data;
 
-    fs->fd = open(rpath(ctx, path, buffer), flags);
+    if (ctx->fs_sm == SM_PASSTHROUGH) {
+        fs->fd = passthrough_request(ctx, NULL, path, flags, NULL, T_OPEN);
+    } else {
+        fs->fd = open(rpath(ctx, path, buffer), flags);
+    }
     return fs->fd;
 }
 
@@ -167,7 +231,17 @@ static int local_opendir(FsContext *ctx,
     char buffer[PATH_MAX];
     char *path = fs_path->data;
 
-    fs->dir = opendir(rpath(ctx, path, buffer));
+    if (ctx->fs_sm == SM_PASSTHROUGH) {
+        int fd;
+        fd = passthrough_request(ctx, NULL, path, O_DIRECTORY, NULL, T_OPEN);
+        if (fd < 0) {
+            return -1;
+        }
+        fs->dir = fdopendir(fd);
+    } else {
+        fs->dir = opendir(rpath(ctx, path, buffer));
+    }
+
     if (!fs->dir) {
         return -1;
     }
@@ -426,7 +500,6 @@ out:
     return err;
 }
 
-
 static int local_symlink(FsContext *fs_ctx, const char *oldpath,
                          V9fsPath *dir_path, const char *name, FsCred *credp)
 {
-- 
1.7.6

[Qemu-devel] [PATCH V12 06/15] hw/9pfs: 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 40a54b3..581bfa9 100644
--- a/hw/9pfs/virtio-9p-chroot-worker.c
+++ b/hw/9pfs/virtio-9p-chroot-worker.c
@@ -93,6 +93,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;
@@ -184,6 +214,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 326238d..d5c3f37 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
 
 #define V9FS_FD_VALID INT_MAX
 
diff --git a/hw/9pfs/virtio-9p-local.c b/hw/9pfs/virtio-9p-local.c
index a91adb8..4e40fa8 100644
--- a/hw/9pfs/virtio-9p-local.c
+++ b/hw/9pfs/virtio-9p-local.c
@@ -474,8 +474,7 @@ static int local_open2(FsContext *fs_ctx, V9fsPath *dir_path, const char *name,
             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, buffer), flags, credp->fc_mode);
         if (fd == -1) {
             err = fd;
@@ -486,6 +485,8 @@ static int local_open2(FsContext *fs_ctx, V9fsPath *dir_path, const char *name,
             serrno = errno;
             goto err_end;
         }
+    } else if (fs_ctx->fs_sm == SM_PASSTHROUGH) {
+        fd = passthrough_request(fs_ctx, NULL, path, flags, credp, T_CREATE);
     }
     err = fd;
     fs->fd = fd;
-- 
1.7.6

[Qemu-devel] [PATCH V12 07/15] hw/9pfs: Creating special files in chroot environment

[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         |   37 +++++++++++++-------------
 3 files changed, 75 insertions(+), 19 deletions(-)

diff --git a/hw/9pfs/virtio-9p-chroot-worker.c b/hw/9pfs/virtio-9p-chroot-worker.c
index 581bfa9..10f290d 100644
--- a/hw/9pfs/virtio-9p-chroot-worker.c
+++ b/hw/9pfs/virtio-9p-chroot-worker.c
@@ -123,6 +123,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;
@@ -220,6 +266,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 d5c3f37..9075361 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
 
 #define V9FS_FD_VALID INT_MAX
 
@@ -37,5 +41,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 4e40fa8..86eb81a 100644
--- a/hw/9pfs/virtio-9p-local.c
+++ b/hw/9pfs/virtio-9p-local.c
@@ -341,8 +341,7 @@ static int local_mknod(FsContext *fs_ctx, V9fsPath *dir_path,
             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, buffer), credp->fc_mode,
                 credp->fc_rdev);
         if (err == -1) {
@@ -353,6 +352,8 @@ static int local_mknod(FsContext *fs_ctx, V9fsPath *dir_path,
             serrno = errno;
             goto err_end;
         }
+    } else {
+        err = passthrough_request(fs_ctx, NULL, path, 0, credp, T_MKNOD);
     }
     goto out;
 
@@ -389,8 +390,7 @@ static int local_mkdir(FsContext *fs_ctx, V9fsPath *dir_path,
             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, buffer), credp->fc_mode);
         if (err == -1) {
             goto out;
@@ -400,6 +400,8 @@ static int local_mkdir(FsContext *fs_ctx, V9fsPath *dir_path,
             serrno = errno;
             goto err_end;
         }
+    } else {
+        err = passthrough_request(fs_ctx, NULL, path, 0, credp, T_MKDIR);
     }
     goto out;
 
@@ -544,25 +546,17 @@ 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, buffer));
         if (err) {
             goto out;
         }
         err = lchown(rpath(fs_ctx, newpath, buffer), 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 {
+        err = passthrough_request(fs_ctx, oldpath, newpath, 0, credp,
+                        T_SYMLINK);
     }
     goto out;
 
@@ -584,8 +578,13 @@ static int local_link(FsContext *ctx, V9fsPath *oldpath,
     v9fs_string_init(&newpath);
     v9fs_string_sprintf(&newpath, "%s/%s", dirpath->data, name);
 
-    ret = link(rpath(ctx, oldpath->data, buffer),
-               rpath(ctx, newpath.data, buffer1));
+    if (ctx->fs_sm == SM_PASSTHROUGH) {
+        ret = passthrough_request(ctx, oldpath->data, newpath.data, 0, NULL,
+                        T_LINK);
+    } else {
+        ret = link(rpath(ctx, oldpath->data, buffer),
+                rpath(ctx, newpath.data, buffer1));
+    }
     v9fs_string_free(&newpath);
     return ret;
 }
-- 
1.7.6

[Qemu-devel] [PATCH V12 08/15] hw/9pfs: Removing file or directory in chroot environment

[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         |    4 ++++
 3 files changed, 23 insertions(+), 0 deletions(-)

diff --git a/hw/9pfs/virtio-9p-chroot-worker.c b/hw/9pfs/virtio-9p-chroot-worker.c
index 10f290d..f269c7b 100644
--- a/hw/9pfs/virtio-9p-chroot-worker.c
+++ b/hw/9pfs/virtio-9p-chroot-worker.c
@@ -169,6 +169,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;
@@ -272,6 +287,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 9075361..9d69739 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
 
 #define V9FS_FD_VALID INT_MAX
 
diff --git a/hw/9pfs/virtio-9p-local.c b/hw/9pfs/virtio-9p-local.c
index 86eb81a..9d5354a 100644
--- a/hw/9pfs/virtio-9p-local.c
+++ b/hw/9pfs/virtio-9p-local.c
@@ -637,6 +637,10 @@ static int local_utimensat(FsContext *s, V9fsPath *fs_path,
 static int local_remove(FsContext *ctx, const char *path)
 {
     char buffer[PATH_MAX];
+
+    if (ctx->fs_sm == SM_PASSTHROUGH) {
+        return passthrough_request(ctx, NULL, path, 0, NULL, T_REMOVE);
+    }
     return remove(rpath(ctx, path, buffer));
 }
 
-- 
1.7.6

[Qemu-devel] [PATCH V12 09/15] hw/9pfs: Rename in chroot environment

[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         |    3 +++
 3 files changed, 21 insertions(+), 0 deletions(-)

diff --git a/hw/9pfs/virtio-9p-chroot-worker.c b/hw/9pfs/virtio-9p-chroot-worker.c
index f269c7b..9bb94f2 100644
--- a/hw/9pfs/virtio-9p-chroot-worker.c
+++ b/hw/9pfs/virtio-9p-chroot-worker.c
@@ -184,6 +184,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;
@@ -290,6 +304,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 9d69739..fda60af 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
 
 #define V9FS_FD_VALID INT_MAX
 
diff --git a/hw/9pfs/virtio-9p-local.c b/hw/9pfs/virtio-9p-local.c
index 9d5354a..3b97f51 100644
--- a/hw/9pfs/virtio-9p-local.c
+++ b/hw/9pfs/virtio-9p-local.c
@@ -602,6 +602,9 @@ static int local_rename(FsContext *ctx, const char *oldpath,
 {
     char buffer[PATH_MAX], buffer1[PATH_MAX];
 
+    if (ctx->fs_sm == SM_PASSTHROUGH) {
+        return passthrough_request(ctx, oldpath, newpath, 0, NULL, T_RENAME);
+    }
     return rename(rpath(ctx, oldpath, buffer), rpath(ctx, newpath, buffer1));
 }
 
-- 
1.7.6

[Qemu-devel] [PATCH V12 10/15] hw/9pfs: 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 |   25 +++++++++----------------
 1 files changed, 9 insertions(+), 16 deletions(-)

diff --git a/hw/9pfs/virtio-9p-local.c b/hw/9pfs/virtio-9p-local.c
index 3b97f51..68551e2 100644
--- a/hw/9pfs/virtio-9p-local.c
+++ b/hw/9pfs/virtio-9p-local.c
@@ -155,23 +155,16 @@ static int local_set_xattr(const char *path, FsCred *credp)
     return 0;
 }
 
-static int local_post_create_passthrough(FsContext *fs_ctx, const char *path,
-                                         FsCred *credp)
+static int local_post_create_none(const char *path, FsCred *credp)
 {
-    char buffer[PATH_MAX];
+    int retval;
 
-    if (chmod(rpath(fs_ctx, path, buffer), credp->fc_mode & 07777) < 0) {
+    if (chmod(path, credp->fc_mode & 07777) < 0) {
         return -1;
     }
-    if (lchown(rpath(fs_ctx, path, buffer), 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(path, credp->fc_uid, credp->fc_gid);
+    if (retval < 0) {
+        /* Ignore return value for none security model */
     }
     return 0;
 }
@@ -347,7 +340,7 @@ static int local_mknod(FsContext *fs_ctx, V9fsPath *dir_path,
         if (err == -1) {
             goto out;
         }
-        err = local_post_create_passthrough(fs_ctx, path, credp);
+        err = local_post_create_none(buffer, credp);
         if (err == -1) {
             serrno = errno;
             goto err_end;
@@ -395,7 +388,7 @@ static int local_mkdir(FsContext *fs_ctx, V9fsPath *dir_path,
         if (err == -1) {
             goto out;
         }
-        err = local_post_create_passthrough(fs_ctx, path, credp);
+        err = local_post_create_none(buffer, credp);
         if (err == -1) {
             serrno = errno;
             goto err_end;
@@ -482,7 +475,7 @@ static int local_open2(FsContext *fs_ctx, V9fsPath *dir_path, const char *name,
             err = fd;
             goto out;
         }
-        err = local_post_create_passthrough(fs_ctx, path, credp);
+        err = local_post_create_none(buffer, credp);
         if (err == -1) {
             serrno = errno;
             goto err_end;
-- 
1.7.6

[Qemu-devel] [PATCH V12 11/15] hw/9pfs: chmod in chroot environment

[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 9bb94f2..d297b50 100644
--- a/hw/9pfs/virtio-9p-chroot-worker.c
+++ b/hw/9pfs/virtio-9p-chroot-worker.c
@@ -198,6 +198,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;
@@ -307,6 +322,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 fda60af..fc7a134 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
 
 #define V9FS_FD_VALID INT_MAX
 
diff --git a/hw/9pfs/virtio-9p-local.c b/hw/9pfs/virtio-9p-local.c
index 68551e2..50e55ed 100644
--- a/hw/9pfs/virtio-9p-local.c
+++ b/hw/9pfs/virtio-9p-local.c
@@ -302,9 +302,10 @@ static int local_chmod(FsContext *fs_ctx, V9fsPath *fs_path, FsCred *credp)
 
     if (fs_ctx->fs_sm == SM_MAPPED) {
         return local_set_xattr(rpath(fs_ctx, path, buffer), credp);
-    } else if ((fs_ctx->fs_sm == SM_PASSTHROUGH) ||
-               (fs_ctx->fs_sm == SM_NONE)) {
+    } else if (fs_ctx->fs_sm == SM_NONE) {
         return chmod(rpath(fs_ctx, path, buffer), credp->fc_mode);
+    } else {
+        return passthrough_request(fs_ctx, NULL, path, 0, credp, T_CHMOD);
     }
     return -1;
 }
-- 
1.7.6

[Qemu-devel] [PATCH V12 12/15] hw/9pfs: chown in chroot environment

[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 d297b50..8ca4805 100644
--- a/hw/9pfs/virtio-9p-chroot-worker.c
+++ b/hw/9pfs/virtio-9p-chroot-worker.c
@@ -213,6 +213,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;
@@ -325,6 +340,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 fc7a134..07c6627 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
 
 #define V9FS_FD_VALID INT_MAX
 
diff --git a/hw/9pfs/virtio-9p-local.c b/hw/9pfs/virtio-9p-local.c
index 50e55ed..673cd44 100644
--- a/hw/9pfs/virtio-9p-local.c
+++ b/hw/9pfs/virtio-9p-local.c
@@ -607,16 +607,17 @@ static int local_chown(FsContext *fs_ctx, V9fsPath *fs_path, FsCred *credp)
     char buffer[PATH_MAX];
     char *path = fs_path->data;
 
-    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, buffer), credp->fc_uid,
                 credp->fc_gid);
     } else if (fs_ctx->fs_sm == SM_MAPPED) {
         return local_set_xattr(rpath(fs_ctx, path, buffer), credp);
-    } else if ((fs_ctx->fs_sm == SM_PASSTHROUGH) ||
-               (fs_ctx->fs_sm == SM_NONE)) {
+    } else if (fs_ctx->fs_sm == SM_NONE) {
         return lchown(rpath(fs_ctx, path, buffer), credp->fc_uid,
                 credp->fc_gid);
+    } else if (fs_ctx->fs_sm == SM_PASSTHROUGH) {
+        return passthrough_request(fs_ctx, NULL, path, 0, credp, T_CHOWN);
     }
     return -1;
 }
-- 
1.7.6

[Qemu-devel] [PATCH V12 13/15] hw/9pfs: stat in chroot environment

[M. Mohan Kumar]

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

diff --git a/hw/9pfs/virtio-9p-chroot-worker.c b/hw/9pfs/virtio-9p-chroot-worker.c
index 8ca4805..a2f6dcd 100644
--- a/hw/9pfs/virtio-9p-chroot-worker.c
+++ b/hw/9pfs/virtio-9p-chroot-worker.c
@@ -66,6 +66,29 @@ static void chroot_sendfd(int sockfd, int fd, int fd_valid)
     }
 }
 
+/* Send special information and error status to qemu process */
+static void chroot_sendspecial(int sockfd, void *buff, int size, int error)
+{
+    int retval;
+
+    /* If there is an error, send NULL buff also set status to error */
+    if (error) {
+        memset(buff, 0, size);
+    }
+    do {
+        retval = send(sockfd, &error, sizeof(error), 0);
+    } while (retval < 0 && errno == EINTR);
+    if (retval < 0) {
+        _exit(1);
+    }
+    do {
+        retval = send(sockfd, buff, size, 0);
+    } while (retval < 0 && errno == EINTR);
+    if (retval < 0) {
+        _exit(1);
+    }
+}
+
 /* Read V9fsFileObjectRequest sent by QEMU process */
 static int chroot_read_request(int sockfd, V9fsFileObjectRequest *request)
 {
@@ -267,6 +290,7 @@ int v9fs_chroot(FsContext *fs_ctx)
     pid_t pid;
     uint32_t code;
     int retval, valid_fd;
+    char *buff;
 
     if (socketpair(PF_UNIX, SOCK_STREAM, 0, fd_pair) < 0) {
         error_report("socketpair %s", strerror(errno));
@@ -346,7 +370,33 @@ int v9fs_chroot(FsContext *fs_ctx)
         default:
             retval = -1;
             break;
+        case T_LSTAT:
+            buff = g_malloc(request.data.size);
+            retval = lstat(request.path.path, (struct stat *)buff);
+            if (retval  < 0) {
+                retval = -errno;
+            }
+            break;
+        }
+
+        /* Send the results */
+        switch (request.data.type) {
+        case T_OPEN:
+        case T_CREATE:
+        case T_MKDIR:
+        case T_SYMLINK:
+        case T_LINK:
+        case T_MKNOD:
+        case T_REMOVE:
+        case T_RENAME:
+        case T_CHMOD:
+        case T_CHOWN:
+            chroot_sendfd(chroot_sock, retval, valid_fd);
+            break;
+        case T_LSTAT:
+            chroot_sendspecial(chroot_sock, buff, request.data.size, retval);
+            g_free(buff);
+            break;
         }
-        chroot_sendfd(chroot_sock, retval, valid_fd);
     }
 }
diff --git a/hw/9pfs/virtio-9p-chroot.c b/hw/9pfs/virtio-9p-chroot.c
index f5b3abc..3094186 100644
--- a/hw/9pfs/virtio-9p-chroot.c
+++ b/hw/9pfs/virtio-9p-chroot.c
@@ -77,6 +77,37 @@ static int v9fs_receivefd(int sockfd, int *sock_error)
     return -ENFILE; /* Ancillary data sent but not received */
 }
 
+static ssize_t chroot_recv(int sockfd, void *data, size_t size, int flags)
+{
+    ssize_t retval;
+    do {
+        retval = recv(sockfd, data, size, 0);
+    } while (retval < 0 && errno == EINTR);
+    return retval;
+}
+
+/*
+ * Return received struct stat on success and -errno on failure.
+ * sock_error is set to 1 whenever there is error in socket IO
+ */
+static int v9fs_special_receive(int sockfd, int *sock_error, char *buff,
+                int size)
+{
+    int retval, status;
+    if (chroot_recv(sockfd, &status, sizeof(status), 0) <= 0) {
+        *sock_error = 1;
+        return -EIO;
+    }
+
+    retval = chroot_recv(sockfd, buff, size, 0);
+    if (retval > 0) {
+        return status;
+    } else {
+        *sock_error = 1;
+        return -EIO;
+    }
+}
+
 /*
  * V9fsFileObjectRequest is written into the socket by QEMU process.
  * Then this request is read by chroot process using v9fs_read_request function
@@ -94,7 +125,7 @@ static int v9fs_write_request(int sockfd, V9fsFileObjectRequest *request)
 /* Return opened file descriptor on success or -errno on error */
 int v9fs_request(FsContext *fs_ctx, V9fsFileObjectRequest *request)
 {
-    int fd, sock_error;
+    int fd, sock_error = 0;
     qemu_mutex_lock(&fs_ctx->chroot_mutex);
     if (fs_ctx->chroot_socket == -1) {
         goto error;
@@ -114,3 +145,29 @@ error:
     qemu_mutex_unlock(&fs_ctx->chroot_mutex);
     return -EIO;
 }
+
+/* Return special information on success or -errno on error */
+int v9fs_special(FsContext *fs_ctx, V9fsFileObjectRequest *request,
+                char *buff, int size)
+{
+    int retval, sock_error = 0;
+    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;
+    }
+    retval = v9fs_special_receive(fs_ctx->chroot_socket, &sock_error, buff,
+                    size);
+    if (retval < 0 && sock_error) {
+        goto error;
+    }
+    qemu_mutex_unlock(&fs_ctx->chroot_mutex);
+    return retval;
+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 07c6627..9ed3f4d 100644
--- a/hw/9pfs/virtio-9p-chroot.h
+++ b/hw/9pfs/virtio-9p-chroot.h
@@ -13,6 +13,7 @@
 #define T_RENAME    8
 #define T_CHMOD     9
 #define T_CHOWN     10
+#define T_LSTAT     11
 
 #define V9FS_FD_VALID INT_MAX
 
@@ -29,6 +30,7 @@ struct V9fsFileObjectData
     gid_t gid;
     dev_t dev;
     int type;
+    int size; /* for special requests */
 };
 
 struct V9fsFileObjectPath
@@ -45,6 +47,7 @@ typedef struct V9fsFileObjectRequest
 
 int v9fs_chroot(FsContext *fs_ctx);
 int v9fs_request(FsContext *fs_ctx, V9fsFileObjectRequest *or);
+int v9fs_special(FsContext *fs_ctx, V9fsFileObjectRequest *or, char *, int);
 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 673cd44..13c93a5 100644
--- a/hw/9pfs/virtio-9p-local.c
+++ b/hw/9pfs/virtio-9p-local.c
@@ -84,12 +84,42 @@ static int passthrough_request(FsContext *fs_ctx, const char *old_path,
     return retval;
 }
 
+static int passthrough_special_request(FsContext *fs_ctx, const char *path,
+                char *buff, int size, int type)
+{
+    V9fsFileObjectRequest request;
+    int retval;
+
+    retval = fill_fileobjectrequest(&request, NULL, path, 0, NULL, type);
+    if (retval < 0) {
+        errno = -retval;
+        return -1;
+    }
+
+    request.data.size = size;
+    if (type == T_LSTAT) {
+        retval = v9fs_special(fs_ctx, &request, buff, size);
+    } else {
+            return -1;
+    }
+    if (retval < 0) {
+        errno = -retval;
+        retval = -1;
+    }
+    return retval;
+}
+
 static int local_lstat(FsContext *fs_ctx, V9fsPath *fs_path, struct stat *stbuf)
 {
     int err;
     char buffer[PATH_MAX];
     char *path = fs_path->data;
 
+    if (fs_ctx->fs_sm == SM_PASSTHROUGH) {
+        return passthrough_special_request(fs_ctx, path, (char *)stbuf,
+                        sizeof(*stbuf), T_LSTAT);
+    }
+
     err =  lstat(rpath(fs_ctx, path, buffer), stbuf);
     if (err) {
         return err;
-- 
1.7.6

[Qemu-devel] [PATCH V12 14/15] hw/9pfs: readlink in chroot environment

[M. Mohan Kumar]

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         |   14 ++++++++++++--
 3 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/hw/9pfs/virtio-9p-chroot-worker.c b/hw/9pfs/virtio-9p-chroot-worker.c
index a2f6dcd..3040d98 100644
--- a/hw/9pfs/virtio-9p-chroot-worker.c
+++ b/hw/9pfs/virtio-9p-chroot-worker.c
@@ -367,9 +367,6 @@ int v9fs_chroot(FsContext *fs_ctx)
         case T_CHOWN:
             retval = chroot_do_chown(&request);
             break;
-        default:
-            retval = -1;
-            break;
         case T_LSTAT:
             buff = g_malloc(request.data.size);
             retval = lstat(request.path.path, (struct stat *)buff);
@@ -377,6 +374,19 @@ int v9fs_chroot(FsContext *fs_ctx)
                 retval = -errno;
             }
             break;
+        case T_READLINK:
+            buff = g_malloc(request.data.size);
+            retval = readlink(request.path.path, buff, request.data.size);
+            if (retval < 0) {
+                retval = -errno;
+            } else {
+                buff[retval] = '\0';
+                retval = 0;
+            }
+            break;
+        default:
+            retval = -1;
+            break;
         }
 
         /* Send the results */
@@ -394,6 +404,7 @@ int v9fs_chroot(FsContext *fs_ctx)
             chroot_sendfd(chroot_sock, retval, valid_fd);
             break;
         case T_LSTAT:
+        case T_READLINK:
             chroot_sendspecial(chroot_sock, buff, request.data.size, retval);
             g_free(buff);
             break;
diff --git a/hw/9pfs/virtio-9p-chroot.h b/hw/9pfs/virtio-9p-chroot.h
index 9ed3f4d..ebf04b5 100644
--- a/hw/9pfs/virtio-9p-chroot.h
+++ b/hw/9pfs/virtio-9p-chroot.h
@@ -14,6 +14,7 @@
 #define T_CHMOD     9
 #define T_CHOWN     10
 #define T_LSTAT     11
+#define T_READLINK  12
 
 #define V9FS_FD_VALID INT_MAX
 
diff --git a/hw/9pfs/virtio-9p-local.c b/hw/9pfs/virtio-9p-local.c
index 13c93a5..ffef8a2 100644
--- a/hw/9pfs/virtio-9p-local.c
+++ b/hw/9pfs/virtio-9p-local.c
@@ -99,6 +99,12 @@ static int passthrough_special_request(FsContext *fs_ctx, const char *path,
     request.data.size = size;
     if (type == T_LSTAT) {
         retval = v9fs_special(fs_ctx, &request, buff, size);
+    } else if (type == T_READLINK) {
+        retval = v9fs_special(fs_ctx, &request, buff, size);
+        /* readlink needs to return the number of bytes placed in buff */
+        if (retval >= 0) {
+            retval = strlen(buff);
+        }
     } else {
             return -1;
     }
@@ -206,6 +212,11 @@ static ssize_t local_readlink(FsContext *fs_ctx, V9fsPath *fs_path,
     char buffer[PATH_MAX];
     char *path = fs_path->data;
 
+    if (fs_ctx->fs_sm == SM_PASSTHROUGH) {
+        return passthrough_special_request(fs_ctx, path, buf, bufsz,
+                        T_READLINK);
+    }
+
     if (fs_ctx->fs_sm == SM_MAPPED) {
         int fd;
         fd = open(rpath(fs_ctx, path, buffer), O_RDONLY);
@@ -217,8 +228,7 @@ static ssize_t local_readlink(FsContext *fs_ctx, V9fsPath *fs_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, buffer), buf, bufsz);
     }
     return tsize;
-- 
1.7.6

[Qemu-devel] [PATCH V12 15/15] hw/9pfs: Chroot environment for other functions

[M. Mohan Kumar]

Add chroot functionality for system calls 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 |   41 +++++++++++++++++++++++++++++++++++++++--
 1 files changed, 39 insertions(+), 2 deletions(-)

diff --git a/hw/9pfs/virtio-9p-local.c b/hw/9pfs/virtio-9p-local.c
index ffef8a2..c7cceb5 100644
--- a/hw/9pfs/virtio-9p-local.c
+++ b/hw/9pfs/virtio-9p-local.c
@@ -628,7 +628,25 @@ static int local_truncate(FsContext *ctx, V9fsPath *fs_path, off_t size)
     char buffer[PATH_MAX];
     char *path = fs_path->data;
 
-    return truncate(rpath(ctx, path, buffer), 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, buffer), size);
+    }
 }
 
 static int local_rename(FsContext *ctx, const char *oldpath,
@@ -668,8 +686,27 @@ static int local_utimensat(FsContext *s, V9fsPath *fs_path,
     char buffer[PATH_MAX];
     char *path = fs_path->data;
 
-    return qemu_utimensat(AT_FDCWD, rpath(s, path, buffer), buf,
+    if (s->fs_sm == SM_PASSTHROUGH) {
+        int fd, retval, serrno = 0;
+        fd = passthrough_request(s, 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 qemu_utimensat(AT_FDCWD, rpath(s, path, buffer), buf,
                           AT_SYMLINK_NOFOLLOW);
+    }
 }
 
 static int local_remove(FsContext *ctx, const char *path)
-- 
1.7.6

Re: [Qemu-devel] [PATCH V12 01/15] Implement qemu_read_full

[malc]

On Mon, 5 Sep 2011, M. Mohan Kumar wrote:

> 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 56e6963..5a4d670 100644
> --- a/osdep.c
> +++ b/osdep.c
> @@ -126,6 +126,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) {

ret == 0 is not an error so here the stale value of errno is checked,
iow this is wrong and recipe for an endless loop.

> +                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 404c421..d6aabd2 100644
> --- a/qemu-common.h
> +++ b/qemu-common.h
> @@ -189,6 +189,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
> 

-- 
mailto:av1474@comtv.ru

Re: [Qemu-devel] [PATCH V12 00/15] virtio-9p: chroot environment for passthrough security model

[Stefan Hajnoczi]

On Mon, Sep 05, 2011 at 09:48:21PM +0530, M. Mohan Kumar wrote:
> Qemu need to be invoked by root user for using virtfs with passthrough
> security model (i.e to use chroot() syscall).
> 
> Question is: Is running qemu by root user expected and allowed? Some of the
> virtfs features can be utilized only if qemu is started by root user (for
> example passthrough security model and handle based file driver need root
> privilege).
> 
> This issue can be resolved by root user starting qemu and spawning a process
> with root privilege to do all privileged operations there and main qemu
> process dropping its privileges to avoid any security issue in running qemu in
> root mode. Privileged operations can be done similar to the chroot patchset.
> 
> But how to determine to which user-id(ie non root user id) qemu needs to drop
> the privileges? Do we have a common user-id across all distributions/systems
> to which qemu process can be dropped down? Also it becomes too complex i.e when
> a new feature needing root privilege is added, a process with root privilege
> needs to be created to handle this.
> 
> So is it allowed to run qemu by root user? If no, is it okay to add the
> complexity of adding a root privilege process for each feature that needs root
> privilege?

I believe libvirt performs the privilege dropping itself and then
invokes QEMU.  So in the context of KVM + libvirt we do not have
privileges in QEMU.  Of course the administrator can edit
/etc/libvirt/qemu.conf and configure the user to run QEMU as (i.e.
root).  But the intention here is to run QEMU unprivileged.

QEMU has its own -runas switch which may be used when QEMU is run
directly by a user or by custom scripts.  This switch looks up the user
and switches to their uid/gid/groups.

We need to think carefully before adding privileged features to QEMU
since they usually require extra configuration to safely limit the group
of users who may use the feature.  These features will be unavailable to
unprivileged users on a system.

The main part of QEMU (vcpu execution and device emulation) should never
run privileged.  This way attacks on QEMU's code are limited to giving
unprivileged access on the host.

A virtfs feature that needs root therefore needs to be in a separate
process.  Either QEMU needs to fork or virtfs could use a separate
daemon binary.

You have already implemented the fork approach in the chroot patches.
Handle-based open could work in the same way.

To summarize this architecture: all path-related operations are
performed by a separate privileged process.  File descriptors are passed
to QEMU over a UNIX domain socket.  This way QEMU can do the actual
read(2)/write(2) calls directly to/from guest memory.

I think it would be nice to build a completely separate binary that QEMU
connects to.  The separate binary would have a much smaller footprint
(doesn't include QEMU code).  More importantly the
privileged/unprivileged boundary would be simple and could be
automatically set up by libvirt:

$ sudo namespace_helper --sock /var/run/virtfs/1234.sock --export my_dir/
$ qemu -fsdev local,id=my_fs,namespace_helper=/var/run/virtfs/1234.sock \
       -device virtio-9p-pci,fsdev=my_fs

Stefan

Re: [Qemu-devel] [PATCH V12 00/15] virtio-9p: chroot environment for passthrough security model

[Stefan Hajnoczi]

Sorry, I forgot to include Daniel Berrange who might have thoughts
about a nice way of running the privileged virtfs helper and how to
integrate with libvirt.

On Tue, Sep 6, 2011 at 3:48 PM, Stefan Hajnoczi <stefanha@gmail.com> wrote:
> On Mon, Sep 05, 2011 at 09:48:21PM +0530, M. Mohan Kumar wrote:
>> Qemu need to be invoked by root user for using virtfs with passthrough
>> security model (i.e to use chroot() syscall).
>>
>> Question is: Is running qemu by root user expected and allowed? Some of the
>> virtfs features can be utilized only if qemu is started by root user (for
>> example passthrough security model and handle based file driver need root
>> privilege).
>>
>> This issue can be resolved by root user starting qemu and spawning a process
>> with root privilege to do all privileged operations there and main qemu
>> process dropping its privileges to avoid any security issue in running qemu in
>> root mode. Privileged operations can be done similar to the chroot patchset.
>>
>> But how to determine to which user-id(ie non root user id) qemu needs to drop
>> the privileges? Do we have a common user-id across all distributions/systems
>> to which qemu process can be dropped down? Also it becomes too complex i.e when
>> a new feature needing root privilege is added, a process with root privilege
>> needs to be created to handle this.
>>
>> So is it allowed to run qemu by root user? If no, is it okay to add the
>> complexity of adding a root privilege process for each feature that needs root
>> privilege?
>
> I believe libvirt performs the privilege dropping itself and then
> invokes QEMU.  So in the context of KVM + libvirt we do not have
> privileges in QEMU.  Of course the administrator can edit
> /etc/libvirt/qemu.conf and configure the user to run QEMU as (i.e.
> root).  But the intention here is to run QEMU unprivileged.
>
> QEMU has its own -runas switch which may be used when QEMU is run
> directly by a user or by custom scripts.  This switch looks up the user
> and switches to their uid/gid/groups.
>
> We need to think carefully before adding privileged features to QEMU
> since they usually require extra configuration to safely limit the group
> of users who may use the feature.  These features will be unavailable to
> unprivileged users on a system.
>
> The main part of QEMU (vcpu execution and device emulation) should never
> run privileged.  This way attacks on QEMU's code are limited to giving
> unprivileged access on the host.
>
> A virtfs feature that needs root therefore needs to be in a separate
> process.  Either QEMU needs to fork or virtfs could use a separate
> daemon binary.
>
> You have already implemented the fork approach in the chroot patches.
> Handle-based open could work in the same way.
>
> To summarize this architecture: all path-related operations are
> performed by a separate privileged process.  File descriptors are passed
> to QEMU over a UNIX domain socket.  This way QEMU can do the actual
> read(2)/write(2) calls directly to/from guest memory.
>
> I think it would be nice to build a completely separate binary that QEMU
> connects to.  The separate binary would have a much smaller footprint
> (doesn't include QEMU code).  More importantly the
> privileged/unprivileged boundary would be simple and could be
> automatically set up by libvirt:
>
> $ sudo namespace_helper --sock /var/run/virtfs/1234.sock --export my_dir/
> $ qemu -fsdev local,id=my_fs,namespace_helper=/var/run/virtfs/1234.sock \
>       -device virtio-9p-pci,fsdev=my_fs
>
> Stefan
>

Re: [Qemu-devel] [PATCH V12 00/15] virtio-9p: chroot environment for passthrough security model

[M. Mohan Kumar]

	
On Tuesday, September 06, 2011 08:18:22 PM Stefan Hajnoczi wrote:
 
> A virtfs feature that needs root therefore needs to be in a separate
> process.  Either QEMU needs to fork or virtfs could use a separate
> daemon binary.
> 
> You have already implemented the fork approach in the chroot patches.
> Handle-based open could work in the same way.
> 
> To summarize this architecture: all path-related operations are
> performed by a separate privileged process.  File descriptors are passed
> to QEMU over a UNIX domain socket.  This way QEMU can do the actual
> read(2)/write(2) calls directly to/from guest memory.
> 
> I think it would be nice to build a completely separate binary that QEMU
> connects to.  The separate binary would have a much smaller footprint
> (doesn't include QEMU code).  More importantly the
> privileged/unprivileged boundary would be simple and could be
> automatically set up by libvirt:
>
Hi Stefan,

Thanks for your inputs. Sorry for the delayed reply.

> $ sudo namespace_helper --sock /var/run/virtfs/1234.sock --export my_dir/
> $ qemu -fsdev local,id=my_fs,namespace_helper=/var/run/virtfs/1234.sock \
>        -device virtio-9p-pci,fsdev=my_fs

We already isolated all path related operations in a privileged process. 
Should we add this complexity? If we take this approach, for handle based 
filesystem driver, we need to ship another binary. Won't it be an usability 
issue for people who use qemu directly?

Re: [Qemu-devel] [PATCH V12 00/15] virtio-9p: chroot environment for passthrough security model

[Daniel P. Berrange]

On Tue, Sep 06, 2011 at 03:48:22PM +0100, Stefan Hajnoczi wrote:
> On Mon, Sep 05, 2011 at 09:48:21PM +0530, M. Mohan Kumar wrote:
> > Qemu need to be invoked by root user for using virtfs with passthrough
> > security model (i.e to use chroot() syscall).
> > 
> > Question is: Is running qemu by root user expected and allowed? Some of the
> > virtfs features can be utilized only if qemu is started by root user (for
> > example passthrough security model and handle based file driver need root
> > privilege).
> > 
> > This issue can be resolved by root user starting qemu and spawning a process
> > with root privilege to do all privileged operations there and main qemu
> > process dropping its privileges to avoid any security issue in running qemu in
> > root mode. Privileged operations can be done similar to the chroot patchset.
> > 
> > But how to determine to which user-id(ie non root user id) qemu needs to drop
> > the privileges? Do we have a common user-id across all distributions/systems
> > to which qemu process can be dropped down? Also it becomes too complex i.e when
> > a new feature needing root privilege is added, a process with root privilege
> > needs to be created to handle this.
> > 
> > So is it allowed to run qemu by root user? If no, is it okay to add the
> > complexity of adding a root privilege process for each feature that needs root
> > privilege?
> 
> I believe libvirt performs the privilege dropping itself and then
> invokes QEMU.  So in the context of KVM + libvirt we do not have
> privileges in QEMU.  Of course the administrator can edit
> /etc/libvirt/qemu.conf and configure the user to run QEMU as (i.e.
> root).  But the intention here is to run QEMU unprivileged.
> 
> QEMU has its own -runas switch which may be used when QEMU is run
> directly by a user or by custom scripts.  This switch looks up the user
> and switches to their uid/gid/groups.
> 
> We need to think carefully before adding privileged features to QEMU
> since they usually require extra configuration to safely limit the group
> of users who may use the feature.  These features will be unavailable to
> unprivileged users on a system.

I agree, regardless of libvirt's needs, p9fs needs to be secure for any
non-root user using QEMU. As non-root I should be able todo

  $ qemu -virtfs $HOME/shared

and have strong confidence that symlink attacks can't be used by the
guest to access other locations nuder $HOME.

> A virtfs feature that needs root therefore needs to be in a separate
> process.  Either QEMU needs to fork or virtfs could use a separate
> daemon binary.

One other idea I just had is 'fakechroot'. This is basically an LD_PRELOAD
hack which wraps the C library's native chroot(), open() etc call to do
chroot in userspace, thus avoiding a need for root privileges.

Either you could just invoke QEMU via fakechroot, enabling your code from
these patches to be used as non-root. Or we could take the code from the
fakechroot library and use that directly in the p9fs code to apply the
path security checks

Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH V12 00/15] virtio-9p: chroot environment for passthrough security model

[M. Mohan Kumar]

> I agree, regardless of libvirt's needs, p9fs needs to be secure for any
> non-root user using QEMU. As non-root I should be able todo
> 
>   $ qemu -virtfs $HOME/shared
> 
> and have strong confidence that symlink attacks can't be used by the
> guest to access other locations nuder $HOME.
> 
> > A virtfs feature that needs root therefore needs to be in a separate
> > process.  Either QEMU needs to fork or virtfs could use a separate
> > daemon binary.
> 
> One other idea I just had is 'fakechroot'. This is basically an LD_PRELOAD
> hack which wraps the C library's native chroot(), open() etc call to do
> chroot in userspace, thus avoiding a need for root privileges.
> 
> Either you could just invoke QEMU via fakechroot, enabling your code from
> these patches to be used as non-root. Or we could take the code from the
> fakechroot library and use that directly in the p9fs code to apply the
> path security checks
> 
With fakechroot is that I can still do following:
chroot("/etc/cups");
fd = open("../passwd", O_RDONLY);

It does not check access beyond the chroot path. Also in virtio-9p case, a 
modified guest kernel can send a symbolic link and that could resolve outside 
chroot path.

passthrough security model in virtio-9p needs root privilege not only for 
chroot() syscall but also to do chown and chmod on files created by the guest.

So IMHO fakechroot can't be used in this case.