* [Qemu-devel] [V6 PATCH 1/9] Implement qemu_read_full
2011-02-28 11:22 [Qemu-devel] [V6 PATCH 0/9] virtio-9p: Use chroot to safely access files in passthrough security model M. Mohan Kumar
@ 2011-02-28 11:22 ` M. Mohan Kumar
2011-02-28 11:22 ` [Qemu-devel] [V6 PATCH 2/9] virtio-9p: Enable CONFIG_THREAD if CONFIG_VIRTFS is enabled M. Mohan Kumar
` (7 subsequent siblings)
8 siblings, 0 replies; 19+ messages in thread
From: M. Mohan Kumar @ 2011-02-28 11:22 UTC (permalink / raw)
To: qemu-devel, Stefan Hajnoczi
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
^ permalink raw reply related [flat|nested] 19+ messages in thread* [Qemu-devel] [V6 PATCH 2/9] virtio-9p: Enable CONFIG_THREAD if CONFIG_VIRTFS is enabled
2011-02-28 11:22 [Qemu-devel] [V6 PATCH 0/9] virtio-9p: Use chroot to safely access files in passthrough security model M. Mohan Kumar
2011-02-28 11:22 ` [Qemu-devel] [V6 PATCH 1/9] Implement qemu_read_full M. Mohan Kumar
@ 2011-02-28 11:22 ` M. Mohan Kumar
2011-02-28 11:22 ` [Qemu-devel] [V6 PATCH 3/9] virtio-9p: Provide chroot daemon side interfaces M. Mohan Kumar
` (6 subsequent siblings)
8 siblings, 0 replies; 19+ messages in thread
From: M. Mohan Kumar @ 2011-02-28 11:22 UTC (permalink / raw)
To: qemu-devel, Stefan Hajnoczi
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
^ permalink raw reply related [flat|nested] 19+ messages in thread* [Qemu-devel] [V6 PATCH 3/9] virtio-9p: Provide chroot daemon side interfaces
2011-02-28 11:22 [Qemu-devel] [V6 PATCH 0/9] virtio-9p: Use chroot to safely access files in passthrough security model M. Mohan Kumar
2011-02-28 11:22 ` [Qemu-devel] [V6 PATCH 1/9] Implement qemu_read_full M. Mohan Kumar
2011-02-28 11:22 ` [Qemu-devel] [V6 PATCH 2/9] virtio-9p: Enable CONFIG_THREAD if CONFIG_VIRTFS is enabled M. Mohan Kumar
@ 2011-02-28 11:22 ` M. Mohan Kumar
2011-03-03 11:16 ` [Qemu-devel] " Stefan Hajnoczi
2011-02-28 11:22 ` [Qemu-devel] [V6 PATCH 4/9] virtio-9p: Add qemu side interfaces for chroot environment M. Mohan Kumar
` (5 subsequent siblings)
8 siblings, 1 reply; 19+ messages in thread
From: M. Mohan Kumar @ 2011-02-28 11:22 UTC (permalink / raw)
To: qemu-devel, Stefan Hajnoczi
Implement chroot daemon 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-dm.c | 186 +++++++++++++++++++++++++++++++++++++++++
hw/9pfs/virtio-9p-chroot.h | 57 +++++++++++++
hw/9pfs/virtio-9p.c | 32 +++++++
hw/file-op-9p.h | 4 +
5 files changed, 280 insertions(+), 0 deletions(-)
create mode 100644 hw/9pfs/virtio-9p-chroot-dm.c
create mode 100644 hw/9pfs/virtio-9p-chroot.h
diff --git a/Makefile.objs b/Makefile.objs
index 6a9f765..f09cdee 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-dm.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-dm.c b/hw/9pfs/virtio-9p-chroot-dm.c
new file mode 100644
index 0000000..c1d8c6e
--- /dev/null
+++ b/hw/9pfs/virtio-9p-chroot-dm.c
@@ -0,0 +1,186 @@
+/*
+ * Virtio 9p chroot environment for contained access to the exported path
+ * Code path handles chroot daemon 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, const FdInfo *fd_info)
+{
+ struct msghdr msg = { };
+ struct iovec iov;
+ struct cmsghdr *cmsg;
+ int retval;
+ union MsgControl msg_control;
+
+ iov.iov_base = (void *)fd_info;
+ iov.iov_len = sizeof(*fd_info);
+
+ memset(&msg, 0, sizeof(msg));
+ msg.msg_iov = &iov;
+ msg.msg_iovlen = 1;
+ /* No ancillary data on error/fd invalid flag */
+ if (!(fd_info->fi_flags & FI_FD_INVALID)) {
+ msg.msg_control = &msg_control;
+ msg.msg_controllen = sizeof(msg_control);
+
+ cmsg = &msg_control.cmsg;
+ cmsg->cmsg_len = CMSG_LEN(sizeof(fd_info->fi_fd));
+ cmsg->cmsg_level = SOL_SOCKET;
+ cmsg->cmsg_type = SCM_RIGHTS;
+ memcpy(CMSG_DATA(cmsg), &fd_info->fi_fd, sizeof(fd_info->fi_fd));
+ }
+ retval = sendmsg(sockfd, &msg, 0);
+ if (retval < 0) {
+ _exit(1);
+ }
+ if (!(fd_info->fi_flags & FI_FD_INVALID)) {
+ close(fd_info->fi_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 and return response through
+ * FdInfo structure
+ */
+static void chroot_do_open(V9fsFileObjectRequest *request, FdInfo *fd_info)
+{
+ fd_info->fi_fd = open(request->path.path, request->data.flags);
+ if (fd_info->fi_fd < 0) {
+ fd_info->fi_fd = -errno;
+ fd_info->fi_flags = FI_FD_INVALID;
+ }
+}
+
+static int 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);
+ return 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;
+ uint64_t code;
+ FdInfo fd_info;
+
+ 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 = CHROOT_ERROR << 32 | errno;
+ error = qemu_write_full(chroot_sock, &code, sizeof(code));
+ _exit(1);
+ }
+
+ error = chroot_daemonize(chroot_sock);
+ if (error) {
+ code = SETSID_ERROR << 32 | error;
+ error = qemu_write_full(chroot_sock, &code, sizeof(code));
+ _exit(1);
+ }
+
+ /*
+ * 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) {
+ memset(&fd_info, 0, sizeof(fd_info));
+ if (chroot_read_request(chroot_sock, &request) < 0) {
+ fd_info.fi_fd = -1;
+ fd_info.fi_flags = FI_FD_SOCKERR;
+ chroot_sendfd(chroot_sock, &fd_info);
+ continue;
+ }
+ switch (request.data.type) {
+ case T_OPEN:
+ chroot_do_open(&request, &fd_info);
+ break;
+ default:
+ fd_info.fi_flags = FI_FD_SOCKERR;
+ break;
+ }
+ chroot_sendfd(chroot_sock, &fd_info);
+ }
+}
diff --git a/hw/9pfs/virtio-9p-chroot.h b/hw/9pfs/virtio-9p-chroot.h
new file mode 100644
index 0000000..a5b2a41
--- /dev/null
+++ b/hw/9pfs/virtio-9p-chroot.h
@@ -0,0 +1,57 @@
+#ifndef _QEMU_VIRTIO_9P_CHROOT_H
+#define _QEMU_VIRTIO_9P_CHROOT_H
+
+/* 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 CHROOT_ERROR 1ULL
+#define SETSID_ERROR 2ULL
+
+
+#define FI_FD_INVALID 0x1 /* fi_fd is not a valid file descriptor */
+#define FI_FD_SOCKERR 0x2 /* socket error happened during an IO */
+/*
+ * Structure used by chroot functions to transmit file descriptor and
+ * error info
+ */
+typedef struct {
+ int fi_fd;
+ int fi_flags;
+} FdInfo;
+
+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 path_len;
+ int oldpath_len;
+ 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..9547d57 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,35 @@ 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) {
+ uint64_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 (read(s->ctx.chroot_socket, &code, sizeof(code)) != sizeof(code)) {
+ error_report("chroot process creation failed");
+ exit(1);
+ }
+ if (code != 0) {
+ switch (code >> 32) {
+ case CHROOT_ERROR:
+ error_report("chroot system call failed: %s",
+ strerror(code & 0xFFFFFFFF));
+ break;
+ case SETSID_ERROR:
+ error_report("setsid failed: %s", strerror(code & 0xFFFFFFFF));
+ break;
+ }
+ 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
^ permalink raw reply related [flat|nested] 19+ messages in thread* [Qemu-devel] Re: [V6 PATCH 3/9] virtio-9p: Provide chroot daemon side interfaces
2011-02-28 11:22 ` [Qemu-devel] [V6 PATCH 3/9] virtio-9p: Provide chroot daemon side interfaces M. Mohan Kumar
@ 2011-03-03 11:16 ` Stefan Hajnoczi
0 siblings, 0 replies; 19+ messages in thread
From: Stefan Hajnoczi @ 2011-03-03 11:16 UTC (permalink / raw)
To: M. Mohan Kumar; +Cc: qemu-devel
On Mon, Feb 28, 2011 at 11:22 AM, M. Mohan Kumar <mohan@in.ibm.com> wrote:
> Implement chroot daemon 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.
daemon and dm aren't accurate descriptions because the chroot process
does not daemonize itself and isn't a daemon :). Words that come
closer: child, helper, worker, subprocess. Please choose one and use
it instead of "daemon".
> + /*
> + * Chroot process sends 0 to indicate chroot process creation is
> + * successful
> + */
> + if (read(s->ctx.chroot_socket, &code, sizeof(code)) != sizeof(code)) {
Might as well use qemu_read_full() here since we have it.
Stefan
^ permalink raw reply [flat|nested] 19+ messages in thread
* [Qemu-devel] [V6 PATCH 4/9] virtio-9p: Add qemu side interfaces for chroot environment
2011-02-28 11:22 [Qemu-devel] [V6 PATCH 0/9] virtio-9p: Use chroot to safely access files in passthrough security model M. Mohan Kumar
` (2 preceding siblings ...)
2011-02-28 11:22 ` [Qemu-devel] [V6 PATCH 3/9] virtio-9p: Provide chroot daemon side interfaces M. Mohan Kumar
@ 2011-02-28 11:22 ` M. Mohan Kumar
2011-03-03 11:38 ` [Qemu-devel] " Stefan Hajnoczi
2011-02-28 11:22 ` [Qemu-devel] [V6 PATCH 5/9] virtio-9p: Add support to open a file in " M. Mohan Kumar
` (4 subsequent siblings)
8 siblings, 1 reply; 19+ messages in thread
From: M. Mohan Kumar @ 2011-02-28 11:22 UTC (permalink / raw)
To: qemu-devel, Stefan Hajnoczi
QEMU side interfaces to communicate with chroot daemon process.
Signed-off-by: M. Mohan Kumar <mohan@in.ibm.com>
---
Makefile.objs | 2 +-
hw/9pfs/virtio-9p-chroot-qemu.c | 97 +++++++++++++++++++++++++++++++++++++++
hw/9pfs/virtio-9p-chroot.h | 1 +
3 files changed, 99 insertions(+), 1 deletions(-)
create mode 100644 hw/9pfs/virtio-9p-chroot-qemu.c
diff --git a/Makefile.objs b/Makefile.objs
index f09cdee..9ea7946 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-dm.o
+9pfs-nested-$(CONFIG_VIRTFS) += virtio-9p-chroot-dm.o virtio-9p-chroot-qemu.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-qemu.c b/hw/9pfs/virtio-9p-chroot-qemu.c
new file mode 100644
index 0000000..d2d8ab9
--- /dev/null
+++ b/hw/9pfs/virtio-9p-chroot-qemu.c
@@ -0,0 +1,97 @@
+/*
+ * Virtio 9p chroot environment for contained access to exported path
+ * Code handles qemu side interfaces to communicate with chroot daemon 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;
+ FdInfo fd_info;
+
+ iov.iov_base = &fd_info;
+ iov.iov_len = sizeof(fd_info);
+
+ *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);
+
+ retval = recvmsg(sockfd, &msg, 0);
+ if (retval < 0) {
+ *sock_error = 1;
+ return -EIO;
+ }
+ if (fd_info.fi_flags & FI_FD_SOCKERR) {
+ *sock_error = 1;
+ return -EIO;
+ }
+ /* If fd is invalid, ancillary data is not present */
+ if (fd_info.fi_fd < 0 || fd_info.fi_flags & FI_FD_INVALID) {
+ return fd_info.fi_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;
+ }
+ *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
+ */
+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 a5b2a41..bd186dd 100644
--- a/hw/9pfs/virtio-9p-chroot.h
+++ b/hw/9pfs/virtio-9p-chroot.h
@@ -53,5 +53,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
^ permalink raw reply related [flat|nested] 19+ messages in thread* [Qemu-devel] Re: [V6 PATCH 4/9] virtio-9p: Add qemu side interfaces for chroot environment
2011-02-28 11:22 ` [Qemu-devel] [V6 PATCH 4/9] virtio-9p: Add qemu side interfaces for chroot environment M. Mohan Kumar
@ 2011-03-03 11:38 ` Stefan Hajnoczi
2011-03-03 14:01 ` M. Mohan Kumar
0 siblings, 1 reply; 19+ messages in thread
From: Stefan Hajnoczi @ 2011-03-03 11:38 UTC (permalink / raw)
To: M. Mohan Kumar; +Cc: qemu-devel
On Mon, Feb 28, 2011 at 11:22 AM, M. Mohan Kumar <mohan@in.ibm.com> wrote:
> + retval = recvmsg(sockfd, &msg, 0);
> + if (retval < 0) {
> + *sock_error = 1;
> + return -EIO;
> + }
Are we guaranteed this will be called with signals blocked? Otherwise
we need to handle EINTR.
> + if (fd_info.fi_flags & FI_FD_SOCKERR) {
> + *sock_error = 1;
> + return -EIO;
> + }
> + /* If fd is invalid, ancillary data is not present */
> + if (fd_info.fi_fd < 0 || fd_info.fi_flags & FI_FD_INVALID) {
> + return fd_info.fi_fd;
> + }
Testing fd_info.fi_flags & FI_FD_INVALID looks dangerous to me. If
for some reason fi_fd >= 0 then we'd return success here. fd_fd < 0
should be a sufficient check, perhaps you wanted an assert() instead?
Stefan
^ permalink raw reply [flat|nested] 19+ messages in thread* [Qemu-devel] Re: [V6 PATCH 4/9] virtio-9p: Add qemu side interfaces for chroot environment
2011-03-03 11:38 ` [Qemu-devel] " Stefan Hajnoczi
@ 2011-03-03 14:01 ` M. Mohan Kumar
2011-03-03 14:25 ` Stefan Hajnoczi
0 siblings, 1 reply; 19+ messages in thread
From: M. Mohan Kumar @ 2011-03-03 14:01 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: qemu-devel
On Thursday 03 March 2011 5:08:10 pm Stefan Hajnoczi wrote:
> On Mon, Feb 28, 2011 at 11:22 AM, M. Mohan Kumar <mohan@in.ibm.com> wrote:
> > + retval = recvmsg(sockfd, &msg, 0);
> > + if (retval < 0) {
> > + *sock_error = 1;
> > + return -EIO;
> > + }
>
> Are we guaranteed this will be called with signals blocked? Otherwise
> we need to handle EINTR.
Ok
>
> > + if (fd_info.fi_flags & FI_FD_SOCKERR) {
> > + *sock_error = 1;
> > + return -EIO;
> > + }
> > + /* If fd is invalid, ancillary data is not present */
> > + if (fd_info.fi_fd < 0 || fd_info.fi_flags & FI_FD_INVALID) {
> > + return fd_info.fi_fd;
> > + }
>
> Testing fd_info.fi_flags & FI_FD_INVALID looks dangerous to me. If
> for some reason fi_fd >= 0 then we'd return success here. fd_fd < 0
> should be a sufficient check, perhaps you wanted an assert() instead?
This check is required because,
Creating special objects like directory, device nodes will not have a valid
fd, usually fd will be 0 on success and -ve on error. During success cases, we
can't do sendmsg for fd=0 value with SCM_RIGHTS, that would result in problem.
In this case, we set fd_info.fi_flags to FI_FD_INVALID indicating fd is not a
valid one, but its not an error case also.
----
M. Mohan Kumar
^ permalink raw reply [flat|nested] 19+ messages in thread* [Qemu-devel] Re: [V6 PATCH 4/9] virtio-9p: Add qemu side interfaces for chroot environment
2011-03-03 14:01 ` M. Mohan Kumar
@ 2011-03-03 14:25 ` Stefan Hajnoczi
0 siblings, 0 replies; 19+ messages in thread
From: Stefan Hajnoczi @ 2011-03-03 14:25 UTC (permalink / raw)
To: M. Mohan Kumar; +Cc: qemu-devel
On Thu, Mar 3, 2011 at 2:01 PM, M. Mohan Kumar <mohan@in.ibm.com> wrote:
> On Thursday 03 March 2011 5:08:10 pm Stefan Hajnoczi wrote:
>> On Mon, Feb 28, 2011 at 11:22 AM, M. Mohan Kumar <mohan@in.ibm.com> wrote:
>> > + retval = recvmsg(sockfd, &msg, 0);
>> > + if (retval < 0) {
>> > + *sock_error = 1;
>> > + return -EIO;
>> > + }
>>
>> Are we guaranteed this will be called with signals blocked? Otherwise
>> we need to handle EINTR.
>
> Ok
>
>>
>> > + if (fd_info.fi_flags & FI_FD_SOCKERR) {
>> > + *sock_error = 1;
>> > + return -EIO;
>> > + }
>> > + /* If fd is invalid, ancillary data is not present */
>> > + if (fd_info.fi_fd < 0 || fd_info.fi_flags & FI_FD_INVALID) {
>> > + return fd_info.fi_fd;
>> > + }
>>
>> Testing fd_info.fi_flags & FI_FD_INVALID looks dangerous to me. If
>> for some reason fi_fd >= 0 then we'd return success here. fd_fd < 0
>> should be a sufficient check, perhaps you wanted an assert() instead?
>
> This check is required because,
>
> Creating special objects like directory, device nodes will not have a valid
> fd, usually fd will be 0 on success and -ve on error. During success cases, we
> can't do sendmsg for fd=0 value with SCM_RIGHTS, that would result in problem.
> In this case, we set fd_info.fi_flags to FI_FD_INVALID indicating fd is not a
> valid one, but its not an error case also.
+ /* If fd is invalid, ancillary data is not present */
+ if (fd_info.fi_fd < 0 || fd_info.fi_flags & FI_FD_INVALID) {
if (fd_info.fi_fd < 0) {
+ return fd_info.fi_fd;
+ }
We can get here now when no fd has been sent, but that's okay because...
+
+ 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;
+ }
+ *sock_error = 1;
+ return -EIO;
...no SCM_RIGHTS was sent (it was optional) so instead of considering
this case an error, we have reached a success case without an fd:
return 0;
Stefan
^ permalink raw reply [flat|nested] 19+ messages in thread
* [Qemu-devel] [V6 PATCH 5/9] virtio-9p: Add support to open a file in chroot environment
2011-02-28 11:22 [Qemu-devel] [V6 PATCH 0/9] virtio-9p: Use chroot to safely access files in passthrough security model M. Mohan Kumar
` (3 preceding siblings ...)
2011-02-28 11:22 ` [Qemu-devel] [V6 PATCH 4/9] virtio-9p: Add qemu side interfaces for chroot environment M. Mohan Kumar
@ 2011-02-28 11:22 ` M. Mohan Kumar
2011-03-03 12:09 ` [Qemu-devel] " Stefan Hajnoczi
2011-02-28 11:22 ` [Qemu-devel] [V6 PATCH 6/9] virtio-9p: Create support " M. Mohan Kumar
` (3 subsequent siblings)
8 siblings, 1 reply; 19+ messages in thread
From: M. Mohan Kumar @ 2011-02-28 11:22 UTC (permalink / raw)
To: qemu-devel, Stefan Hajnoczi
This patch adds both chroot deamon 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-qemu.c | 24 +++++++++++-----
hw/9pfs/virtio-9p-chroot.h | 2 +-
hw/9pfs/virtio-9p-local.c | 58 ++++++++++++++++++++++++++++++++++++---
3 files changed, 71 insertions(+), 13 deletions(-)
diff --git a/hw/9pfs/virtio-9p-chroot-qemu.c b/hw/9pfs/virtio-9p-chroot-qemu.c
index d2d8ab9..41f9db2 100644
--- a/hw/9pfs/virtio-9p-chroot-qemu.c
+++ b/hw/9pfs/virtio-9p-chroot-qemu.c
@@ -85,13 +85,21 @@ 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;
+ }
+ v9fs_write_request(fs_ctx->chroot_socket, request);
+ 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 bd186dd..4592807 100644
--- a/hw/9pfs/virtio-9p-chroot.h
+++ b/hw/9pfs/virtio-9p-chroot.h
@@ -53,6 +53,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..0c55d35 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,40 @@
#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));
+ request->data.path_len = strlen(path);
+ 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_open(FsContext *fs_ctx, const char *path, int flags)
+{
+ V9fsFileObjectRequest request;
+ int fd;
+
+ fd = fill_fileobjectrequest(&request, path, NULL);
+ if (fd < 0) {
+ errno = -fd;
+ return -1;
+ }
+ request.data.flags = flags;
+ request.data.type = T_OPEN;
+ fd = v9fs_request(fs_ctx, &request);
+ return fd;
+}
static int local_lstat(FsContext *fs_ctx, const char *path, struct stat *stbuf)
{
@@ -138,14 +175,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_open(fs_ctx, path, flags);
+ } 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_open(fs_ctx, path, O_DIRECTORY);
+ 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
^ permalink raw reply related [flat|nested] 19+ messages in thread* [Qemu-devel] Re: [V6 PATCH 5/9] virtio-9p: Add support to open a file in chroot environment
2011-02-28 11:22 ` [Qemu-devel] [V6 PATCH 5/9] virtio-9p: Add support to open a file in " M. Mohan Kumar
@ 2011-03-03 12:09 ` Stefan Hajnoczi
2011-03-03 13:54 ` M. Mohan Kumar
0 siblings, 1 reply; 19+ messages in thread
From: Stefan Hajnoczi @ 2011-03-03 12:09 UTC (permalink / raw)
To: M. Mohan Kumar; +Cc: qemu-devel
On Mon, Feb 28, 2011 at 11:22 AM, M. Mohan Kumar <mohan@in.ibm.com> wrote:
> This patch adds both chroot deamon 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-qemu.c | 24 +++++++++++-----
> hw/9pfs/virtio-9p-chroot.h | 2 +-
> hw/9pfs/virtio-9p-local.c | 58 ++++++++++++++++++++++++++++++++++++---
> 3 files changed, 71 insertions(+), 13 deletions(-)
>
> diff --git a/hw/9pfs/virtio-9p-chroot-qemu.c b/hw/9pfs/virtio-9p-chroot-qemu.c
> index d2d8ab9..41f9db2 100644
> --- a/hw/9pfs/virtio-9p-chroot-qemu.c
> +++ b/hw/9pfs/virtio-9p-chroot-qemu.c
> @@ -85,13 +85,21 @@ 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;
> + }
> + v9fs_write_request(fs_ctx->chroot_socket, request);
> + fd = v9fs_receivefd(fs_ctx->chroot_socket, &sock_error);
> + if (fd < 0 && sock_error) {
> + fs_ctx->chroot_ioerror = 1;
> + }
chroot_ioerror, sock_error, and their FdInfo bits are redundant code.
The chroot child could just exit on error and the parent would get
errors when writing the request here, which is the same effect as
manually returning -EIO in this function. There is no need to
introduce variables and bits to communicate this failure mode.
Once that simplification has been made, FdInfo becomes just an -errno
value to pass back the result of open(2) and friends. That means we
can completely drop FdInfo and the fi_fd field which doesn't actually
hold a useful fd value for the QEMU process but just a -errno.
Instead send back only an int -errno return code from the chroot child.
You mentioned wanting to distinguish between socket errors and
blocking syscall errors but since there isn't any real error handling
that makes use of that information (and I'm not sure there is a good
error handling strategy that could be used), this is all just adding
complexity.
> +/* Helper routine to fill V9fsFileObjectRequest structure */
> +static int fill_fileobjectrequest(V9fsFileObjectRequest *request,
> + const char *path, FsCred *credp)
> +{
> + if (strlen(path) > PATH_MAX) {
> + return -ENAMETOOLONG;
> + }
Off-by-one error. It should strlen(path) >= PATH_MAX to account for
the NUL terminator.
> + memset(request, 0, sizeof(*request));
> + request->data.path_len = strlen(path);
> + 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_open(FsContext *fs_ctx, const char *path, int flags)
> +{
> + V9fsFileObjectRequest request;
> + int fd;
> +
> + fd = fill_fileobjectrequest(&request, path, NULL);
> + if (fd < 0) {
> + errno = -fd;
Please don't use errno to communicate errors back. In this function
it would be perfectly fine to return fd here since it is already a
-errno.
It's not safe to use errno since it's value can be lost by calling any
external function - its value may be modified even if no error occurs.
I quoted from the errno(3) man page in a previous review:
"a function that succeeds *is* allowed to change errno"
> + return -1;
> + }
> + request.data.flags = flags;
> + request.data.type = T_OPEN;
> + fd = v9fs_request(fs_ctx, &request);
> + return fd;
> +}
>
> static int local_lstat(FsContext *fs_ctx, const char *path, struct stat *stbuf)
> {
> @@ -138,14 +175,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_open(fs_ctx, path, flags);
> + } 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_open(fs_ctx, path, O_DIRECTORY);
> + if (fd < 0) {
> + return NULL;
> + }
> + return fdopendir(fd);
> + } else {
> + return opendir(rpath(fs_ctx, path));
> + }
Perhaps we should use a local_operations struct or similar function
pointer table instead of adding fs_ctx->fs_sm conditionals everywhere.
Also it would be neat to reuse the local implementations on the chroot
child side instead of instead of splitting code paths, but I'm not
sure whether that is possible.
Stefan
^ permalink raw reply [flat|nested] 19+ messages in thread* [Qemu-devel] Re: [V6 PATCH 5/9] virtio-9p: Add support to open a file in chroot environment
2011-03-03 12:09 ` [Qemu-devel] " Stefan Hajnoczi
@ 2011-03-03 13:54 ` M. Mohan Kumar
2011-03-03 14:16 ` Stefan Hajnoczi
0 siblings, 1 reply; 19+ messages in thread
From: M. Mohan Kumar @ 2011-03-03 13:54 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: qemu-devel
On Thursday 03 March 2011 5:39:55 pm Stefan Hajnoczi wrote:
> > + v9fs_write_request(fs_ctx->chroot_socket, request);
> > + fd = v9fs_receivefd(fs_ctx->chroot_socket, &sock_error);
> > + if (fd < 0 && sock_error) {
> > + fs_ctx->chroot_ioerror = 1;
> > + }
>
> chroot_ioerror, sock_error, and their FdInfo bits are redundant code.
> The chroot child could just exit on error and the parent would get
> errors when writing the request here, which is the same effect as
> manually returning -EIO in this function. There is no need to
> introduce variables and bits to communicate this failure mode.
>
> Once that simplification has been made, FdInfo becomes just an -errno
> value to pass back the result of open(2) and friends. That means we
> can completely drop FdInfo and the fi_fd field which doesn't actually
> hold a useful fd value for the QEMU process but just a -errno.
>
But we need to pass the fd to qemu process, so it could not be -errno. When
fd >= 0 its a valid fd otherwise its a errno.
> Instead send back only an int -errno return code from the chroot child.
>
> You mentioned wanting to distinguish between socket errors and
> blocking syscall errors but since there isn't any real error handling
> that makes use of that information (and I'm not sure there is a good
> error handling strategy that could be used), this is all just adding
> complexity.
>
> > +/* Helper routine to fill V9fsFileObjectRequest structure */
> > +static int fill_fileobjectrequest(V9fsFileObjectRequest *request,
> > + const char *path, FsCred *credp)
> > +{
> > + if (strlen(path) > PATH_MAX) {
> > + return -ENAMETOOLONG;
> > + }
>
> Off-by-one error. It should strlen(path) >= PATH_MAX to account for
> the NUL terminator.
>
Ok, I Will change!
> > + memset(request, 0, sizeof(*request));
> > + request->data.path_len = strlen(path);
> > + 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_open(FsContext *fs_ctx, const char *path, int
> > flags) +{
> > + V9fsFileObjectRequest request;
> > + int fd;
> > +
> > + fd = fill_fileobjectrequest(&request, path, NULL);
> > + if (fd < 0) {
> > + errno = -fd;
>
> Please don't use errno to communicate errors back. In this function
> it would be perfectly fine to return fd here since it is already a
> -errno.
>
> It's not safe to use errno since it's value can be lost by calling any
> external function - its value may be modified even if no error occurs.
> I quoted from the errno(3) man page in a previous review:
> "a function that succeeds *is* allowed to change errno"
>
> > + return -1;
> > + }
> > + request.data.flags = flags;
> > + request.data.type = T_OPEN;
> > + fd = v9fs_request(fs_ctx, &request);
> > + return fd;
> > +}
> >
> > static int local_lstat(FsContext *fs_ctx, const char *path, struct stat
> > *stbuf) {
> > @@ -138,14 +175,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_open(fs_ctx, path, flags);
> > + } 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_open(fs_ctx, path, O_DIRECTORY);
> > + if (fd < 0) {
> > + return NULL;
> > + }
> > + return fdopendir(fd);
> > + } else {
> > + return opendir(rpath(fs_ctx, path));
> > + }
>
> Perhaps we should use a local_operations struct or similar function
> pointer table instead of adding fs_ctx->fs_sm conditionals everywhere.
That would have more code duplication when compared to additional
conditionals. i.e, We need to create local_passthrough_opendir,
local_passthrough_open, .. etc.
>
> Also it would be neat to reuse the local implementations on the chroot
> child side instead of instead of splitting code paths, but I'm not
> sure whether that is possible.
I am not sure what do you mean by reusing the virtio-9p-local.c implementation
in chilld side. That may involve breaking down existing local functions?
----
M. Mohan Kumar
^ permalink raw reply [flat|nested] 19+ messages in thread* [Qemu-devel] Re: [V6 PATCH 5/9] virtio-9p: Add support to open a file in chroot environment
2011-03-03 13:54 ` M. Mohan Kumar
@ 2011-03-03 14:16 ` Stefan Hajnoczi
0 siblings, 0 replies; 19+ messages in thread
From: Stefan Hajnoczi @ 2011-03-03 14:16 UTC (permalink / raw)
To: M. Mohan Kumar; +Cc: qemu-devel
On Thu, Mar 3, 2011 at 1:54 PM, M. Mohan Kumar <mohan@in.ibm.com> wrote:
> On Thursday 03 March 2011 5:39:55 pm Stefan Hajnoczi wrote:
>> > + v9fs_write_request(fs_ctx->chroot_socket, request);
>> > + fd = v9fs_receivefd(fs_ctx->chroot_socket, &sock_error);
>> > + if (fd < 0 && sock_error) {
>> > + fs_ctx->chroot_ioerror = 1;
>> > + }
>>
>> chroot_ioerror, sock_error, and their FdInfo bits are redundant code.
>> The chroot child could just exit on error and the parent would get
>> errors when writing the request here, which is the same effect as
>> manually returning -EIO in this function. There is no need to
>> introduce variables and bits to communicate this failure mode.
>>
>> Once that simplification has been made, FdInfo becomes just an -errno
>> value to pass back the result of open(2) and friends. That means we
>> can completely drop FdInfo and the fi_fd field which doesn't actually
>> hold a useful fd value for the QEMU process but just a -errno.
>>
>
> But we need to pass the fd to qemu process, so it could not be -errno. When
> fd >= 0 its a valid fd otherwise its a errno.
The fd is optionally passed via SCM_RIGHTS, no as an FdInfo field.
The QEMU side can detect whether or not the fd has been passed, it
doesn't need anything in FdInfo other than int -errno.
> That would have more code duplication when compared to additional
> conditionals. i.e, We need to create local_passthrough_opendir,
> local_passthrough_open, .. etc.
The choice of conditionals vs function pointers isn't really about
code duplication, it's about putting all the passthrough code in one
place and all the non-passthrough code in another place rather than
interleaving everywhere. This is just a suggestion though and it's up
to you.
>> Also it would be neat to reuse the local implementations on the chroot
>> child side instead of instead of splitting code paths, but I'm not
>> sure whether that is possible.
> I am not sure what do you mean by reusing the virtio-9p-local.c implementation
> in chilld side. That may involve breaking down existing local functions?
This is more of a speculative suggestion but I'm imagining calling the
same open, mkdir, etc helper code in the chroot child as get executed
when the chroot helper is not in use. It looks like we're splitting
code paths even further than they already are today with this
patchset.
Stefan
^ permalink raw reply [flat|nested] 19+ messages in thread
* [Qemu-devel] [V6 PATCH 6/9] virtio-9p: Create support in chroot environment
2011-02-28 11:22 [Qemu-devel] [V6 PATCH 0/9] virtio-9p: Use chroot to safely access files in passthrough security model M. Mohan Kumar
` (4 preceding siblings ...)
2011-02-28 11:22 ` [Qemu-devel] [V6 PATCH 5/9] virtio-9p: Add support to open a file in " M. Mohan Kumar
@ 2011-02-28 11:22 ` M. Mohan Kumar
2011-03-01 22:55 ` Venkateswararao Jujjuri (JV)
2011-02-28 11:22 ` [Qemu-devel] [V6 PATCH 7/9] virtio-9p: Support for creating special files M. Mohan Kumar
` (2 subsequent siblings)
8 siblings, 1 reply; 19+ messages in thread
From: M. Mohan Kumar @ 2011-02-28 11:22 UTC (permalink / raw)
To: qemu-devel, Stefan Hajnoczi
Add both chroot deamon & 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-dm.c | 39 +++++++++++++++++++++++++++++++++++++++
hw/9pfs/virtio-9p-local.c | 21 +++++++++++++++++++--
2 files changed, 58 insertions(+), 2 deletions(-)
diff --git a/hw/9pfs/virtio-9p-chroot-dm.c b/hw/9pfs/virtio-9p-chroot-dm.c
index c1d8c6e..985d42b 100644
--- a/hw/9pfs/virtio-9p-chroot-dm.c
+++ b/hw/9pfs/virtio-9p-chroot-dm.c
@@ -83,6 +83,42 @@ static void chroot_do_open(V9fsFileObjectRequest *request, FdInfo *fd_info)
}
}
+/*
+ * Helper routine to create a file and return the file descriptor and
+ * error status in FdInfo structure.
+ */
+static void chroot_do_create(V9fsFileObjectRequest *request, FdInfo *fd_info)
+{
+ uid_t cur_uid;
+ gid_t cur_gid;
+
+ cur_uid = geteuid();
+ cur_gid = getegid();
+
+ fd_info->fi_fd = -1;
+
+ if (setfsuid(request->data.uid) < 0) {
+ fd_info->fi_fd = -errno;
+ fd_info->fi_flags = FI_FD_INVALID;
+ return;
+ }
+ if (setfsgid(request->data.gid) < 0) {
+ fd_info->fi_fd = -errno;
+ fd_info->fi_flags = FI_FD_INVALID;
+ goto unset_uid;
+ }
+
+ fd_info->fi_fd = open(request->path.path, request->data.flags,
+ request->data.mode);
+ if (fd_info->fi_fd < 0) {
+ fd_info->fi_fd = -errno;
+ fd_info->fi_flags = FI_FD_INVALID;
+ }
+ setfsgid(cur_gid);
+unset_uid:
+ setfsuid(cur_uid);
+}
+
static int chroot_daemonize(int chroot_sock)
{
sigset_t sigset;
@@ -177,6 +213,9 @@ int v9fs_chroot(FsContext *fs_ctx)
case T_OPEN:
chroot_do_open(&request, &fd_info);
break;
+ case T_CREATE:
+ chroot_do_create(&request, &fd_info);
+ break;
default:
fd_info.fi_flags = FI_FD_SOCKERR;
break;
diff --git a/hw/9pfs/virtio-9p-local.c b/hw/9pfs/virtio-9p-local.c
index 0c55d35..3fed16c 100644
--- a/hw/9pfs/virtio-9p-local.c
+++ b/hw/9pfs/virtio-9p-local.c
@@ -58,6 +58,22 @@ static int passthrough_open(FsContext *fs_ctx, const char *path, int flags)
return fd;
}
+static int passthrough_create(FsContext *fs_ctx, const char *path, int flags,
+ FsCred *credp)
+{
+ V9fsFileObjectRequest request;
+ int fd;
+
+ fd = fill_fileobjectrequest(&request, path, credp);
+ if (fd < 0) {
+ return fd;
+ }
+ request.data.flags = flags;
+ request.data.type = T_CREATE;
+ fd = v9fs_request(fs_ctx, &request);
+ return fd;
+}
+
static int local_lstat(FsContext *fs_ctx, const char *path, struct stat *stbuf)
{
int err;
@@ -382,8 +398,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;
@@ -393,6 +408,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_create(fs_ctx, path, flags, credp);
}
return fd;
--
1.7.3.4
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [Qemu-devel] [V6 PATCH 6/9] virtio-9p: Create support in chroot environment
2011-02-28 11:22 ` [Qemu-devel] [V6 PATCH 6/9] virtio-9p: Create support " M. Mohan Kumar
@ 2011-03-01 22:55 ` Venkateswararao Jujjuri (JV)
0 siblings, 0 replies; 19+ messages in thread
From: Venkateswararao Jujjuri (JV) @ 2011-03-01 22:55 UTC (permalink / raw)
To: M. Mohan Kumar; +Cc: Stefan Hajnoczi, qemu-devel
On 2/28/2011 3:22 AM, M. Mohan Kumar wrote:
> Add both chroot deamon & 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-dm.c | 39 +++++++++++++++++++++++++++++++++++++++
> hw/9pfs/virtio-9p-local.c | 21 +++++++++++++++++++--
> 2 files changed, 58 insertions(+), 2 deletions(-)
>
> diff --git a/hw/9pfs/virtio-9p-chroot-dm.c b/hw/9pfs/virtio-9p-chroot-dm.c
> index c1d8c6e..985d42b 100644
> --- a/hw/9pfs/virtio-9p-chroot-dm.c
> +++ b/hw/9pfs/virtio-9p-chroot-dm.c
> @@ -83,6 +83,42 @@ static void chroot_do_open(V9fsFileObjectRequest *request, FdInfo *fd_info)
> }
> }
>
> +/*
> + * Helper routine to create a file and return the file descriptor and
> + * error status in FdInfo structure.
> + */
> +static void chroot_do_create(V9fsFileObjectRequest *request, FdInfo *fd_info)
> +{
> + uid_t cur_uid;
> + gid_t cur_gid;
> +
> + cur_uid = geteuid();
> + cur_gid = getegid();
> +
> + fd_info->fi_fd = -1;
> +
> + if (setfsuid(request->data.uid) < 0) {
> + fd_info->fi_fd = -errno;
> + fd_info->fi_flags = FI_FD_INVALID;
> + return;
> + }
> + if (setfsgid(request->data.gid) < 0) {
> + fd_info->fi_fd = -errno;
> + fd_info->fi_flags = FI_FD_INVALID;
> + goto unset_uid;
> + }
> +
> + fd_info->fi_fd = open(request->path.path, request->data.flags,
> + request->data.mode);
> + if (fd_info->fi_fd < 0) {
> + fd_info->fi_fd = -errno;
> + fd_info->fi_flags = FI_FD_INVALID;
> + }
> + setfsgid(cur_gid);
> +unset_uid:
> + setfsuid(cur_uid);
> +}
> +
> static int chroot_daemonize(int chroot_sock)
> {
> sigset_t sigset;
> @@ -177,6 +213,9 @@ int v9fs_chroot(FsContext *fs_ctx)
> case T_OPEN:
> chroot_do_open(&request, &fd_info);
> break;
> + case T_CREATE:
> + chroot_do_create(&request, &fd_info);
> + break;
> default:
> fd_info.fi_flags = FI_FD_SOCKERR;
> break;
> diff --git a/hw/9pfs/virtio-9p-local.c b/hw/9pfs/virtio-9p-local.c
> index 0c55d35..3fed16c 100644
> --- a/hw/9pfs/virtio-9p-local.c
> +++ b/hw/9pfs/virtio-9p-local.c
> @@ -58,6 +58,22 @@ static int passthrough_open(FsContext *fs_ctx, const char *path, int flags)
> return fd;
> }
>
> +static int passthrough_create(FsContext *fs_ctx, const char *path, int flags,
> + FsCred *credp)
> +{
> + V9fsFileObjectRequest request;
> + int fd;
> +
> + fd = fill_fileobjectrequest(&request, path, credp);
> + if (fd < 0) {
> + return fd;
Here fd is -errno;
Need to assign it back to errno before returning.
You took care of it in passthrough_open() but not here.
Please check other places where this logic applies.
- JV
> + }
> + request.data.flags = flags;
> + request.data.type = T_CREATE;
> + fd = v9fs_request(fs_ctx, &request);
> + return fd;
> +}
> +
> static int local_lstat(FsContext *fs_ctx, const char *path, struct stat *stbuf)
> {
> int err;
> @@ -382,8 +398,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;
> @@ -393,6 +408,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_create(fs_ctx, path, flags, credp);
> }
> return fd;
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [Qemu-devel] [V6 PATCH 7/9] virtio-9p: Support for creating special files
2011-02-28 11:22 [Qemu-devel] [V6 PATCH 0/9] virtio-9p: Use chroot to safely access files in passthrough security model M. Mohan Kumar
` (5 preceding siblings ...)
2011-02-28 11:22 ` [Qemu-devel] [V6 PATCH 6/9] virtio-9p: Create support " M. Mohan Kumar
@ 2011-02-28 11:22 ` M. Mohan Kumar
2011-03-01 23:00 ` Venkateswararao Jujjuri (JV)
2011-02-28 11:22 ` [Qemu-devel] [V6 PATCH 8/9] virtio-9p: Move file post creation changes to none security model M. Mohan Kumar
2011-02-28 11:22 ` [Qemu-devel] [V6 PATCH 9/9] virtio-9p: Chroot environment for other functions M. Mohan Kumar
8 siblings, 1 reply; 19+ messages in thread
From: M. Mohan Kumar @ 2011-02-28 11:22 UTC (permalink / raw)
To: qemu-devel, Stefan Hajnoczi
Add both chroot deamon 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-dm.c | 57 ++++++++++++++++++++++++
hw/9pfs/virtio-9p-chroot-qemu.c | 19 ++++++++
hw/9pfs/virtio-9p-chroot.h | 1 +
hw/9pfs/virtio-9p-local.c | 93 ++++++++++++++++++++++++++-------------
4 files changed, 139 insertions(+), 31 deletions(-)
diff --git a/hw/9pfs/virtio-9p-chroot-dm.c b/hw/9pfs/virtio-9p-chroot-dm.c
index 985d42b..0ead017 100644
--- a/hw/9pfs/virtio-9p-chroot-dm.c
+++ b/hw/9pfs/virtio-9p-chroot-dm.c
@@ -119,6 +119,57 @@ unset_uid:
setfsuid(cur_uid);
}
+/*
+ * 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 in FdInfo
+ * structure
+ */
+static void chroot_do_create_special(V9fsFileObjectRequest *request,
+ FdInfo *fd_info)
+{
+ int cur_uid, cur_gid;
+
+ cur_uid = geteuid();
+ cur_gid = getegid();
+
+ fd_info->fi_fd = -1;
+ /* fd is not valid for create operations */
+ fd_info->fi_flags = FI_FD_INVALID;
+
+ if (setfsuid(request->data.uid) < 0) {
+ fd_info->fi_fd = -errno;
+ return;
+ }
+ if (setfsgid(request->data.gid) < 0) {
+ fd_info->fi_fd = -errno;
+ goto unset_uid;
+ }
+
+ switch (request->data.type) {
+ case T_MKDIR:
+ fd_info->fi_fd = mkdir(request->path.path, request->data.mode);
+ break;
+ case T_SYMLINK:
+ fd_info->fi_fd = symlink(request->path.old_path, request->path.path);
+ break;
+ case T_LINK:
+ fd_info->fi_fd = link(request->path.old_path, request->path.path);
+ break;
+ default:
+ fd_info->fi_fd = mknod(request->path.path, request->data.mode,
+ request->data.dev);
+ break;
+ }
+
+ if (fd_info->fi_fd < 0) {
+ fd_info->fi_fd = -errno;
+ }
+ setfsgid(cur_gid);
+unset_uid:
+ setfsuid(cur_uid);
+}
+
static int chroot_daemonize(int chroot_sock)
{
sigset_t sigset;
@@ -216,6 +267,12 @@ int v9fs_chroot(FsContext *fs_ctx)
case T_CREATE:
chroot_do_create(&request, &fd_info);
break;
+ case T_MKDIR:
+ case T_SYMLINK:
+ case T_LINK:
+ case T_MKNOD:
+ chroot_do_create_special(&request, &fd_info);
+ break;
default:
fd_info.fi_flags = FI_FD_SOCKERR;
break;
diff --git a/hw/9pfs/virtio-9p-chroot-qemu.c b/hw/9pfs/virtio-9p-chroot-qemu.c
index 41f9db2..1a42dc2 100644
--- a/hw/9pfs/virtio-9p-chroot-qemu.c
+++ b/hw/9pfs/virtio-9p-chroot-qemu.c
@@ -103,3 +103,22 @@ unlock:
qemu_mutex_unlock(&fs_ctx->chroot_mutex);
return fd;
}
+
+/* Return 0 on success or -errno on error */
+int v9fs_create_special(FsContext *fs_ctx, V9fsFileObjectRequest *request)
+{
+ int fd, sock_error;
+ qemu_mutex_lock(&fs_ctx->chroot_mutex);
+ if (fs_ctx->chroot_ioerror) {
+ fd = -EIO;
+ goto unlock;
+ }
+ v9fs_write_request(fs_ctx->chroot_socket, request);
+ 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 4592807..f113ff1 100644
--- a/hw/9pfs/virtio-9p-chroot.h
+++ b/hw/9pfs/virtio-9p-chroot.h
@@ -54,5 +54,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 3fed16c..c92c5dd 100644
--- a/hw/9pfs/virtio-9p-local.c
+++ b/hw/9pfs/virtio-9p-local.c
@@ -74,6 +74,28 @@ static int passthrough_create(FsContext *fs_ctx, const char *path, int flags,
return fd;
}
+static int passthrough_create_special(FsContext *fs_ctx, const char *oldpath,
+ const char *path, FsCred *credp, int type)
+{
+ V9fsFileObjectRequest request;
+ int retval;
+
+ retval = fill_fileobjectrequest(&request, path, credp);
+ if (retval < 0) {
+ return retval;
+ }
+ request.data.type = type;
+ if (oldpath) {
+ request.data.oldpath_len = strlen(oldpath);
+ if (strlen(oldpath) > PATH_MAX) {
+ return -ENAMETOOLONG;
+ }
+ strcpy(request.path.old_path, oldpath);
+ }
+ retval = v9fs_create_special(fs_ctx, &request);
+ return retval;
+}
+
static int local_lstat(FsContext *fs_ctx, const char *path, struct stat *stbuf)
{
int err;
@@ -291,8 +313,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;
@@ -302,6 +323,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_create_special(fs_ctx, NULL, path, credp, T_MKNOD);
+ if (err < 0) {
+ serrno = errno;
+ goto err_end;
+ }
}
return err;
@@ -328,8 +355,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;
@@ -339,6 +365,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_create_special(fs_ctx, NULL, path, credp, T_MKDIR);
+ if (err < 0) {
+ serrno = errno;
+ goto err_end;
+ }
}
return err;
@@ -456,23 +488,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_create_special(fs_ctx, oldpath, newpath, credp,
+ T_SYMLINK);
+ if (err < 0) {
+ serrno = errno;
+ goto err_end;
}
}
return err;
@@ -483,24 +511,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_create_special(fs_ctx, oldpath, newpath, 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
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [Qemu-devel] [V6 PATCH 7/9] virtio-9p: Support for creating special files
2011-02-28 11:22 ` [Qemu-devel] [V6 PATCH 7/9] virtio-9p: Support for creating special files M. Mohan Kumar
@ 2011-03-01 23:00 ` Venkateswararao Jujjuri (JV)
0 siblings, 0 replies; 19+ messages in thread
From: Venkateswararao Jujjuri (JV) @ 2011-03-01 23:00 UTC (permalink / raw)
To: M. Mohan Kumar; +Cc: Stefan Hajnoczi, qemu-devel
On 2/28/2011 3:22 AM, M. Mohan Kumar wrote:
> Add both chroot deamon 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-dm.c | 57 ++++++++++++++++++++++++
> hw/9pfs/virtio-9p-chroot-qemu.c | 19 ++++++++
> hw/9pfs/virtio-9p-chroot.h | 1 +
> hw/9pfs/virtio-9p-local.c | 93 ++++++++++++++++++++++++++-------------
> 4 files changed, 139 insertions(+), 31 deletions(-)
>
> diff --git a/hw/9pfs/virtio-9p-chroot-dm.c b/hw/9pfs/virtio-9p-chroot-dm.c
> index 985d42b..0ead017 100644
> --- a/hw/9pfs/virtio-9p-chroot-dm.c
> +++ b/hw/9pfs/virtio-9p-chroot-dm.c
> @@ -119,6 +119,57 @@ unset_uid:
> setfsuid(cur_uid);
> }
>
> +/*
> + * 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 in FdInfo
> + * structure
> + */
> +static void chroot_do_create_special(V9fsFileObjectRequest *request,
> + FdInfo *fd_info)
> +{
> + int cur_uid, cur_gid;
> +
> + cur_uid = geteuid();
> + cur_gid = getegid();
> +
> + fd_info->fi_fd = -1;
> + /* fd is not valid for create operations */
> + fd_info->fi_flags = FI_FD_INVALID;
> +
> + if (setfsuid(request->data.uid) < 0) {
> + fd_info->fi_fd = -errno;
> + return;
> + }
> + if (setfsgid(request->data.gid) < 0) {
> + fd_info->fi_fd = -errno;
> + goto unset_uid;
> + }
> +
> + switch (request->data.type) {
> + case T_MKDIR:
> + fd_info->fi_fd = mkdir(request->path.path, request->data.mode);
> + break;
> + case T_SYMLINK:
> + fd_info->fi_fd = symlink(request->path.old_path, request->path.path);
> + break;
> + case T_LINK:
> + fd_info->fi_fd = link(request->path.old_path, request->path.path);
> + break;
> + default:
> + fd_info->fi_fd = mknod(request->path.path, request->data.mode,
> + request->data.dev);
> + break;
> + }
> +
> + if (fd_info->fi_fd < 0) {
> + fd_info->fi_fd = -errno;
> + }
> + setfsgid(cur_gid);
> +unset_uid:
> + setfsuid(cur_uid);
> +}
> +
> static int chroot_daemonize(int chroot_sock)
> {
> sigset_t sigset;
> @@ -216,6 +267,12 @@ int v9fs_chroot(FsContext *fs_ctx)
> case T_CREATE:
> chroot_do_create(&request, &fd_info);
> break;
> + case T_MKDIR:
> + case T_SYMLINK:
> + case T_LINK:
> + case T_MKNOD:
> + chroot_do_create_special(&request, &fd_info);
> + break;
> default:
> fd_info.fi_flags = FI_FD_SOCKERR;
> break;
> diff --git a/hw/9pfs/virtio-9p-chroot-qemu.c b/hw/9pfs/virtio-9p-chroot-qemu.c
> index 41f9db2..1a42dc2 100644
> --- a/hw/9pfs/virtio-9p-chroot-qemu.c
> +++ b/hw/9pfs/virtio-9p-chroot-qemu.c
> @@ -103,3 +103,22 @@ unlock:
> qemu_mutex_unlock(&fs_ctx->chroot_mutex);
> return fd;
> }
> +
> +/* Return 0 on success or -errno on error */
> +int v9fs_create_special(FsContext *fs_ctx, V9fsFileObjectRequest *request)
> +{
> + int fd, sock_error;
Since this is not fd; may be you can use some other variable like err or something?
> + qemu_mutex_lock(&fs_ctx->chroot_mutex);
> + if (fs_ctx->chroot_ioerror) {
> + fd = -EIO;
> + goto unlock;
> + }
> + v9fs_write_request(fs_ctx->chroot_socket, request);
> + fd = v9fs_receivefd(fs_ctx->chroot_socket, &sock_error);
> + if (fd < 0 && sock_error) {
> + fs_ctx->chroot_ioerror = 1;
> + }
Format??
- JV
> +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 4592807..f113ff1 100644
> --- a/hw/9pfs/virtio-9p-chroot.h
> +++ b/hw/9pfs/virtio-9p-chroot.h
> @@ -54,5 +54,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 3fed16c..c92c5dd 100644
> --- a/hw/9pfs/virtio-9p-local.c
> +++ b/hw/9pfs/virtio-9p-local.c
> @@ -74,6 +74,28 @@ static int passthrough_create(FsContext *fs_ctx, const char *path, int flags,
> return fd;
> }
>
> +static int passthrough_create_special(FsContext *fs_ctx, const char *oldpath,
> + const char *path, FsCred *credp, int type)
> +{
> + V9fsFileObjectRequest request;
> + int retval;
> +
> + retval = fill_fileobjectrequest(&request, path, credp);
> + if (retval < 0) {
> + return retval;
> + }
> + request.data.type = type;
> + if (oldpath) {
> + request.data.oldpath_len = strlen(oldpath);
> + if (strlen(oldpath) > PATH_MAX) {
> + return -ENAMETOOLONG;
> + }
> + strcpy(request.path.old_path, oldpath);
> + }
> + retval = v9fs_create_special(fs_ctx, &request);
> + return retval;
> +}
> +
> static int local_lstat(FsContext *fs_ctx, const char *path, struct stat *stbuf)
> {
> int err;
> @@ -291,8 +313,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;
> @@ -302,6 +323,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_create_special(fs_ctx, NULL, path, credp, T_MKNOD);
> + if (err < 0) {
> + serrno = errno;
> + goto err_end;
> + }
> }
> return err;
>
> @@ -328,8 +355,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;
> @@ -339,6 +365,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_create_special(fs_ctx, NULL, path, credp, T_MKDIR);
> + if (err < 0) {
> + serrno = errno;
> + goto err_end;
> + }
> }
> return err;
>
> @@ -456,23 +488,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_create_special(fs_ctx, oldpath, newpath, credp,
> + T_SYMLINK);
> + if (err < 0) {
> + serrno = errno;
> + goto err_end;
> }
> }
> return err;
> @@ -483,24 +511,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_create_special(fs_ctx, oldpath, newpath, 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;
^ permalink raw reply [flat|nested] 19+ messages in thread
* [Qemu-devel] [V6 PATCH 8/9] virtio-9p: Move file post creation changes to none security model
2011-02-28 11:22 [Qemu-devel] [V6 PATCH 0/9] virtio-9p: Use chroot to safely access files in passthrough security model M. Mohan Kumar
` (6 preceding siblings ...)
2011-02-28 11:22 ` [Qemu-devel] [V6 PATCH 7/9] virtio-9p: Support for creating special files M. Mohan Kumar
@ 2011-02-28 11:22 ` M. Mohan Kumar
2011-02-28 11:22 ` [Qemu-devel] [V6 PATCH 9/9] virtio-9p: Chroot environment for other functions M. Mohan Kumar
8 siblings, 0 replies; 19+ messages in thread
From: M. Mohan Kumar @ 2011-02-28 11:22 UTC (permalink / raw)
To: qemu-devel, Stefan Hajnoczi
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 c92c5dd..f5dba35 100644
--- a/hw/9pfs/virtio-9p-local.c
+++ b/hw/9pfs/virtio-9p-local.c
@@ -163,21 +163,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;
}
@@ -318,7 +311,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;
@@ -360,7 +353,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;
@@ -435,7 +428,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
^ permalink raw reply related [flat|nested] 19+ messages in thread* [Qemu-devel] [V6 PATCH 9/9] virtio-9p: Chroot environment for other functions
2011-02-28 11:22 [Qemu-devel] [V6 PATCH 0/9] virtio-9p: Use chroot to safely access files in passthrough security model M. Mohan Kumar
` (7 preceding siblings ...)
2011-02-28 11:22 ` [Qemu-devel] [V6 PATCH 8/9] virtio-9p: Move file post creation changes to none security model M. Mohan Kumar
@ 2011-02-28 11:22 ` M. Mohan Kumar
8 siblings, 0 replies; 19+ messages in thread
From: M. Mohan Kumar @ 2011-02-28 11:22 UTC (permalink / raw)
To: qemu-devel, Stefan Hajnoczi
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 | 227 +++++++++++++++++++++++++++++++++++++++------
1 files changed, 197 insertions(+), 30 deletions(-)
diff --git a/hw/9pfs/virtio-9p-local.c b/hw/9pfs/virtio-9p-local.c
index f5dba35..b44ad55 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,
@@ -96,14 +97,35 @@ static int passthrough_create_special(FsContext *fs_ctx, const char *oldpath,
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) {
+ return fd;
+ }
+ request.data.type = T_OPEN;
+ fd = v9fs_request(fs_ctx, &request);
+ qemu_free(dpath);
+ 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;
@@ -125,6 +147,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;
}
@@ -189,9 +232,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 = 0;
+ }
+ close(pfd);
+ qemu_free(tmp_path);
+ errno = serrno;
}
return tsize;
}
@@ -283,8 +340,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;
@@ -532,53 +604,148 @@ 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;
+ fd = passthrough_open(ctx, path, O_RDWR);
+ if (fd < 0) {
+ errno = -fd;
+ return -1;
+ }
+ retval = ftruncate(fd, size);
+ close(fd);
+ return retval;
+ } else {
+ return truncate(rpath(ctx, path), 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 = 0;
- err = rename(tmp, rpath(ctx, newpath));
- if (err == -1) {
- int serrno = errno;
- qemu_free(tmp);
+ if (ctx->fs_sm == SM_PASSTHROUGH) {
+ int opfd, npfd;
+ char *old_tmppath, *new_tmppath;
+ opfd = get_dirfd(ctx, oldpath);
+ if (opfd < 0) {
+ return -1;
+ }
+ npfd = get_dirfd(ctx, newpath);
+ if (npfd < 0) {
+ close(opfd);
+ return -1;
+ }
+ old_tmppath = qemu_strdup(oldpath);
+ new_tmppath = qemu_strdup(newpath);
+ err = renameat(opfd, basename(old_tmppath),
+ npfd, basename(new_tmppath));
+ if (err == -1) {
+ serrno = errno;
+ }
+ close(npfd);
+ close(opfd);
+ qemu_free(old_tmppath);
+ qemu_free(new_tmppath);
errno = serrno;
} else {
- qemu_free(tmp);
+ char *tmp;
+ tmp = qemu_strdup(rpath(ctx, oldpath));
+
+ err = rename(tmp, rpath(ctx, newpath));
+ if (err == -1) {
+ int serrno = errno;
+ qemu_free(tmp);
+ errno = serrno;
+ } else {
+ qemu_free(tmp);
+ }
}
return err;
-
}
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_open(fs_ctx, path, O_RDONLY | O_NONBLOCK);
+ 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)
-{
- return remove(rpath(ctx, path));
+static int local_remove(FsContext *fs_ctx, const char *path)
+ {
+ if (fs_ctx->fs_sm == SM_PASSTHROUGH) {
+ int pfd, err, serrno, flags;
+ char *old_path;
+ struct stat stbuf;
+ pfd = get_dirfd(fs_ctx, path);
+ if (pfd < 0) {
+ return -1;
+ }
+ old_path = qemu_strdup(path);
+ err = fstatat(pfd, basename(old_path), &stbuf, AT_SYMLINK_NOFOLLOW);
+ if (err < 0) {
+ return -1;
+ }
+ serrno = flags = 0;
+ if ((stbuf.st_mode & S_IFMT) == S_IFDIR) {
+ flags = AT_REMOVEDIR;
+ } else {
+ flags = 0;
+ }
+ err = unlinkat(pfd, basename(old_path), flags);
+ if (err == -1) {
+ serrno = errno;
+ }
+ qemu_free(old_path);
+ close(pfd);
+ errno = serrno;
+ return err;
+ } else {
+ return remove(rpath(fs_ctx, path));
+ }
}
static int local_fsync(FsContext *ctx, int fd, int datasync)
--
1.7.3.4
^ permalink raw reply related [flat|nested] 19+ messages in thread