* [Qemu-devel] [V3 PATCH 0/8] virtio-9p: Use chroot to safely access files in passthrough model
@ 2011-01-18 6:23 M. Mohan Kumar
2011-01-18 6:25 ` [Qemu-devel] [V3 PATCH 1/8] virtio-9p: Implement qemu_read_full M. Mohan Kumar
` (7 more replies)
0 siblings, 8 replies; 20+ messages in thread
From: M. Mohan Kumar @ 2011-01-18 6:23 UTC (permalink / raw)
To: qemu-devel
In passthrough security model, following symbolic links in the server
side could result in TOCTTOU vulnerabilities.
This patchset resolves this issue by creating a dedicated process which
chroots into the share path and all file object access is done in the
chroot environment.
This patchset implements chroot enviroment, provides necessary functions
that can be used by the passthrough function calls.
Changes from version V2
* Treat socket IO errors as fatal, ie qemu will exit
* Split patchset based on chroot side (server) and qemu side(client)
functionalities
This patchset is tested with fsstress, connectathon, Tuxera POSIX test suite
and LTP FS testcases for all three security models.
M. Mohan Kumar (8):
Implement qemu_read_full
Provide chroot environment server side interfaces
Add client side interfaces for chroot environment
Add support to open a file in chroot environment
Create support in chroot environment
Support for creating special files
Move file post creation changes to none security model
Chroot environment for other functions
Makefile.objs | 1 +
hw/9pfs/virtio-9p-chroot.c | 414 ++++++++++++++++++++++++++++++++++++++++
hw/9pfs/virtio-9p-chroot.h | 42 ++++
hw/9pfs/virtio-9p-local.c | 456 ++++++++++++++++++++++++++++++++++++--------
hw/9pfs/virtio-9p.c | 23 +++
hw/file-op-9p.h | 2 +
osdep.c | 32 +++
qemu-common.h | 2 +
8 files changed, 891 insertions(+), 81 deletions(-)
create mode 100644 hw/9pfs/virtio-9p-chroot.c
create mode 100644 hw/9pfs/virtio-9p-chroot.h
--
1.7.3.4
^ permalink raw reply [flat|nested] 20+ messages in thread
* [Qemu-devel] [V3 PATCH 1/8] virtio-9p: Implement qemu_read_full
2011-01-18 6:23 [Qemu-devel] [V3 PATCH 0/8] virtio-9p: Use chroot to safely access files in passthrough model M. Mohan Kumar
@ 2011-01-18 6:25 ` M. Mohan Kumar
2011-01-18 6:25 ` [Qemu-devel] [V3 PATCH 2/8] virtio-9p: Provide chroot environment server side interfaces M. Mohan Kumar
` (6 subsequent siblings)
7 siblings, 0 replies; 20+ messages in thread
From: M. Mohan Kumar @ 2011-01-18 6:25 UTC (permalink / raw)
To: qemu-devel
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 b3957f1..0f60e08 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -190,6 +190,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] 20+ messages in thread
* [Qemu-devel] [V3 PATCH 2/8] virtio-9p: Provide chroot environment server side interfaces
2011-01-18 6:23 [Qemu-devel] [V3 PATCH 0/8] virtio-9p: Use chroot to safely access files in passthrough model M. Mohan Kumar
2011-01-18 6:25 ` [Qemu-devel] [V3 PATCH 1/8] virtio-9p: Implement qemu_read_full M. Mohan Kumar
@ 2011-01-18 6:25 ` M. Mohan Kumar
2011-01-18 17:03 ` Blue Swirl
2011-01-18 6:25 ` [Qemu-devel] [V3 PATCH 3/8] virtio-9p: Add client side interfaces for chroot environment M. Mohan Kumar
` (5 subsequent siblings)
7 siblings, 1 reply; 20+ messages in thread
From: M. Mohan Kumar @ 2011-01-18 6:25 UTC (permalink / raw)
To: qemu-devel
Implement chroot server 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.c | 183 ++++++++++++++++++++++++++++++++++++++++++++
hw/9pfs/virtio-9p-chroot.h | 39 +++++++++
hw/9pfs/virtio-9p.c | 23 ++++++
hw/file-op-9p.h | 2 +
5 files changed, 248 insertions(+), 0 deletions(-)
create mode 100644 hw/9pfs/virtio-9p-chroot.c
create mode 100644 hw/9pfs/virtio-9p-chroot.h
diff --git a/Makefile.objs b/Makefile.objs
index bc0344c..3007b6d 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -273,6 +273,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.o
hw-obj-$(CONFIG_REALLY_VIRTFS) += $(addprefix 9pfs/, $(9pfs-nested-y))
$(addprefix 9pfs/, $(9pfs-nested-y)): CFLAGS += -I$(SRC_PATH)/hw/
diff --git a/hw/9pfs/virtio-9p-chroot.c b/hw/9pfs/virtio-9p-chroot.c
new file mode 100644
index 0000000..dcde2cc
--- /dev/null
+++ b/hw/9pfs/virtio-9p-chroot.c
@@ -0,0 +1,183 @@
+/*
+ * Virtio 9p chroot environment for contained access to exported path
+ *
+ * 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"
+
+/*
+ * Structure used by chroot functions to transmit file descriptor and
+ * error info
+ */
+typedef struct {
+ int fi_fd;
+ int fi_error;
+} FdInfo;
+
+union MsgControl {
+ struct cmsghdr cmsg;
+ char control[CMSG_SPACE(sizeof(int))];
+};
+
+/* Send file descriptor and error status to qemu process */
+static int chroot_sendfd(int sockfd, FdInfo *fd_info)
+{
+ struct msghdr msg = { };
+ struct iovec iov;
+ struct cmsghdr *cmsg;
+ int retval;
+ union MsgControl msg_control;
+
+ iov.iov_base = 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 */
+ if (!fd_info->fi_error) {
+ 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);
+ close(fd_info->fi_fd);
+ return retval;
+}
+
+/* Read V9fsFileObjectRequest written by QEMU process */
+static void chroot_read_request(int sockfd, V9fsFileObjectRequest *request)
+{
+ int retval;
+ retval = qemu_read_full(sockfd, request, sizeof(request->data));
+ if (retval <= 0 || request->data.path_len <= 0) {
+ _exit(-1);
+ }
+ request->path.path = qemu_mallocz(request->data.path_len + 1);
+ retval = qemu_read_full(sockfd, (void *)request->path.path,
+ request->data.path_len);
+ if (retval <= 0) {
+ _exit(-1);
+ }
+ if (request->data.oldpath_len > 0) {
+ request->path.old_path =
+ qemu_mallocz(request->data.oldpath_len + 1);
+ retval = qemu_read_full(sockfd, (void *)request->path.old_path,
+ request->data.oldpath_len);
+ if (retval <= 0) {
+ _exit(-1);
+ }
+ }
+}
+
+static int chroot_daemonize(int chroot_sock)
+{
+ sigset_t sigset;
+ struct rlimit nr_fd;
+ int fd;
+
+ /* Block all signals for this process */
+ sigprocmask(SIG_SETMASK, &sigset, NULL);
+
+ /* Daemonize */
+ if (setsid() < 0) {
+ error_report("setsid %s", strerror(errno));
+ return -1;
+ }
+
+ /* Close other file descriptors */
+ getrlimit(RLIMIT_NOFILE, &nr_fd);
+ for (fd = 0; fd < nr_fd.rlim_cur; fd++) {
+ if (fd != chroot_sock) {
+ close(fd);
+ }
+ }
+
+ /* Create files with mode as requested by client */
+ umask(0);
+ return 0;
+}
+
+static void chroot_dummy(void)
+{
+ (void)chroot_sendfd;
+}
+
+/*
+ * Fork a process and chroot into the share path. Communication
+ * between qemu process and chroot process happens via socket
+ */
+int v9fs_chroot(FsContext *fs_ctx)
+{
+ int fd_pair[2], pid, chroot_sock, error;
+ V9fsFileObjectRequest request;
+
+ 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) {
+ error_report("chroot %s", strerror(errno));
+ return -1;
+ }
+
+ chroot_dummy();
+ if (chroot_daemonize(chroot_sock) < 0) {
+ _exit(-1);
+ }
+
+ /*
+ * Write 0 to chroot socket to indicate chroot process creation is
+ * successful
+ */
+ error = 0;
+ if (qemu_write_full(chroot_sock, &error, sizeof(error)) != sizeof(error)) {
+ _exit(-1);
+ }
+ /* get the request from the socket */
+ while (1) {
+ chroot_read_request(chroot_sock, &request);
+ qemu_free(request.path.path);
+ if (request.data.oldpath_len) {
+ qemu_free(request.path.old_path);
+ }
+ if (error) {
+ _exit(error);
+ }
+ }
+}
diff --git a/hw/9pfs/virtio-9p-chroot.h b/hw/9pfs/virtio-9p-chroot.h
new file mode 100644
index 0000000..3bee5e3
--- /dev/null
+++ b/hw/9pfs/virtio-9p-chroot.h
@@ -0,0 +1,39 @@
+#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
+
+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;
+ char *old_path;
+};
+
+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 3858e17..7f4c552 100644
--- a/hw/9pfs/virtio-9p.c
+++ b/hw/9pfs/virtio-9p.c
@@ -14,10 +14,13 @@
#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"
+#include <pthread.h>
int debug_9p_pdu;
@@ -3743,5 +3746,25 @@ 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) {
+ pthread_mutex_init(&s->ctx.chroot_mutex, 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, &i, sizeof(i)) != sizeof(i)) {
+ error_report("chroot process creation failed");
+ exit(1);
+ }
+ if (i != 0) {
+ error_report("chroot process creation failed");
+ exit(1);
+ }
+ }
+
return &s->vdev;
}
diff --git a/hw/file-op-9p.h b/hw/file-op-9p.h
index c7731c2..149a915 100644
--- a/hw/file-op-9p.h
+++ b/hw/file-op-9p.h
@@ -55,6 +55,8 @@ typedef struct FsContext
SecModel fs_sm;
uid_t uid;
struct xattr_operations **xops;
+ pthread_mutex_t chroot_mutex;
+ int chroot_socket;
} FsContext;
extern void cred_init(FsCred *);
--
1.7.3.4
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Qemu-devel] [V3 PATCH 3/8] virtio-9p: Add client side interfaces for chroot environment
2011-01-18 6:23 [Qemu-devel] [V3 PATCH 0/8] virtio-9p: Use chroot to safely access files in passthrough model M. Mohan Kumar
2011-01-18 6:25 ` [Qemu-devel] [V3 PATCH 1/8] virtio-9p: Implement qemu_read_full M. Mohan Kumar
2011-01-18 6:25 ` [Qemu-devel] [V3 PATCH 2/8] virtio-9p: Provide chroot environment server side interfaces M. Mohan Kumar
@ 2011-01-18 6:25 ` M. Mohan Kumar
2011-01-18 6:25 ` [Qemu-devel] [V3 PATCH 4/8] virtio-9p: Add support to open a file in " M. Mohan Kumar
` (4 subsequent siblings)
7 siblings, 0 replies; 20+ messages in thread
From: M. Mohan Kumar @ 2011-01-18 6:25 UTC (permalink / raw)
To: qemu-devel
Define QEMU side interfaces used for chroot environment.
Signed-off-by: M. Mohan Kumar <mohan@in.ibm.com>
---
hw/9pfs/virtio-9p-chroot.c | 78 ++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 78 insertions(+), 0 deletions(-)
diff --git a/hw/9pfs/virtio-9p-chroot.c b/hw/9pfs/virtio-9p-chroot.c
index dcde2cc..25a7fab 100644
--- a/hw/9pfs/virtio-9p-chroot.c
+++ b/hw/9pfs/virtio-9p-chroot.c
@@ -91,6 +91,82 @@ static void chroot_read_request(int sockfd, V9fsFileObjectRequest *request)
}
}
+/* Receive file descriptor and error status from chroot process */
+static int v9fs_receivefd(int sockfd, int *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);
+
+ 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) {
+ error_report("recvmsg: %s", strerror(errno));
+ exit(1);
+ }
+
+ /* If error is set, ancillary data is not present */
+ if (fd_info.fi_error) {
+ *error = fd_info.fi_error;
+ return -1;
+ }
+
+ 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;
+ }
+
+ *error = EAGAIN;
+ return -1;
+}
+
+/*
+ * V9fsFileObjectRequest is written into the socket by QEMU process.
+ * Then this request is read by chroot process using read_request function
+ */
+static void v9fs_write_request(int sockfd, V9fsFileObjectRequest *request)
+{
+ int retval;
+
+ retval = qemu_write_full(sockfd, &request->data,
+ sizeof(request->data));
+ if (retval < 0) {
+ error_report("socket write failed: %s", strerror(errno));
+ exit(-1);
+ }
+
+ retval = qemu_write_full(sockfd, request->path.path,
+ request->data.path_len);
+ if (retval < 0) {
+ error_report("socket write failed: %s", strerror(errno));
+ exit(-1);
+ }
+ if (request->data.oldpath_len > 0) {
+ retval = qemu_write_full(sockfd, request->path.old_path,
+ request->data.oldpath_len);
+ if (retval < 0) {
+ error_report("socket write failed: %s", strerror(errno));
+ exit(-1);
+ }
+ }
+}
+
static int chroot_daemonize(int chroot_sock)
{
sigset_t sigset;
@@ -122,6 +198,8 @@ static int chroot_daemonize(int chroot_sock)
static void chroot_dummy(void)
{
(void)chroot_sendfd;
+ (void)v9fs_receivefd;
+ (void)v9fs_write_request;
}
/*
--
1.7.3.4
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Qemu-devel] [V3 PATCH 4/8] virtio-9p: Add support to open a file in chroot environment
2011-01-18 6:23 [Qemu-devel] [V3 PATCH 0/8] virtio-9p: Use chroot to safely access files in passthrough model M. Mohan Kumar
` (2 preceding siblings ...)
2011-01-18 6:25 ` [Qemu-devel] [V3 PATCH 3/8] virtio-9p: Add client side interfaces for chroot environment M. Mohan Kumar
@ 2011-01-18 6:25 ` M. Mohan Kumar
2011-01-18 6:25 ` [Qemu-devel] [V3 PATCH 5/8] virtio-9p: Create support " M. Mohan Kumar
` (3 subsequent siblings)
7 siblings, 0 replies; 20+ messages in thread
From: M. Mohan Kumar @ 2011-01-18 6:25 UTC (permalink / raw)
To: qemu-devel
This patch adds both server and client 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 | 45 +++++++++++++++++++++++++++++++------
hw/9pfs/virtio-9p-chroot.h | 1 +
hw/9pfs/virtio-9p-local.c | 52 ++++++++++++++++++++++++++++++++++++++++---
3 files changed, 86 insertions(+), 12 deletions(-)
diff --git a/hw/9pfs/virtio-9p-chroot.c b/hw/9pfs/virtio-9p-chroot.c
index 25a7fab..b599e23 100644
--- a/hw/9pfs/virtio-9p-chroot.c
+++ b/hw/9pfs/virtio-9p-chroot.c
@@ -167,6 +167,32 @@ static void v9fs_write_request(int sockfd, V9fsFileObjectRequest *request)
}
}
+/* Return opened file descriptor on success or -1 on error */
+int v9fs_request(FsContext *fs_ctx, V9fsFileObjectRequest *request,
+ int *error)
+{
+ int fd;
+ pthread_mutex_lock(&fs_ctx->chroot_mutex);
+ v9fs_write_request(fs_ctx->chroot_socket, request);
+ fd = v9fs_receivefd(fs_ctx->chroot_socket, error);
+ pthread_mutex_unlock(&fs_ctx->chroot_mutex);
+ return fd;
+}
+
+/*
+ * Helper routine to open a file and return fd and error status in
+ * 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_error = errno;
+ } else {
+ fd_info->fi_error = 0;
+ }
+}
+
static int chroot_daemonize(int chroot_sock)
{
sigset_t sigset;
@@ -195,13 +221,6 @@ static int chroot_daemonize(int chroot_sock)
return 0;
}
-static void chroot_dummy(void)
-{
- (void)chroot_sendfd;
- (void)v9fs_receivefd;
- (void)v9fs_write_request;
-}
-
/*
* Fork a process and chroot into the share path. Communication
* between qemu process and chroot process happens via socket
@@ -210,6 +229,7 @@ int v9fs_chroot(FsContext *fs_ctx)
{
int fd_pair[2], pid, chroot_sock, error;
V9fsFileObjectRequest request;
+ FdInfo fd_info;
if (socketpair(PF_UNIX, SOCK_STREAM, 0, fd_pair) < 0) {
error_report("socketpair %s", strerror(errno));
@@ -234,7 +254,6 @@ int v9fs_chroot(FsContext *fs_ctx)
return -1;
}
- chroot_dummy();
if (chroot_daemonize(chroot_sock) < 0) {
_exit(-1);
}
@@ -250,6 +269,16 @@ int v9fs_chroot(FsContext *fs_ctx)
/* get the request from the socket */
while (1) {
chroot_read_request(chroot_sock, &request);
+ switch (request.data.type) {
+ case T_OPEN:
+ chroot_do_open(&request, &fd_info);
+ if (chroot_sendfd(chroot_sock, &fd_info) <= 0) {
+ error = -2;
+ }
+ break;
+ default:
+ break;
+ }
qemu_free(request.path.path);
if (request.data.oldpath_len) {
qemu_free(request.path.old_path);
diff --git a/hw/9pfs/virtio-9p-chroot.h b/hw/9pfs/virtio-9p-chroot.h
index 3bee5e3..f5a2ca0 100644
--- a/hw/9pfs/virtio-9p-chroot.h
+++ b/hw/9pfs/virtio-9p-chroot.h
@@ -35,5 +35,6 @@ typedef struct V9fsFileObjectRequest
int v9fs_chroot(FsContext *fs_ctx);
+int v9fs_request(FsContext *fs_ctx, V9fsFileObjectRequest *or, int *error);
#endif /* _QEMU_VIRTIO_9P_CHROOT_H */
diff --git a/hw/9pfs/virtio-9p-local.c b/hw/9pfs/virtio-9p-local.c
index a8e7525..2376ec2 100644
--- a/hw/9pfs/virtio-9p-local.c
+++ b/hw/9pfs/virtio-9p-local.c
@@ -13,6 +13,7 @@
#include "virtio.h"
#include "virtio-9p.h"
#include "virtio-9p-xattr.h"
+#include "virtio-9p-chroot.h"
#include <arpa/inet.h>
#include <pwd.h>
#include <grp.h>
@@ -20,6 +21,36 @@
#include <sys/un.h>
#include <attr/xattr.h>
+/* Helper routine to fill V9fsFileObjectRequest structure */
+static void fill_request(V9fsFileObjectRequest *request, const char *path,
+ FsCred *credp)
+{
+ memset(request, 0, sizeof(*request));
+ request->data.path_len = strlen(path);
+ request->path.path = qemu_strdup(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;
+ }
+}
+
+static int __open(FsContext *fs_ctx, const char *path, int flags)
+{
+ V9fsFileObjectRequest request;
+ int fd, error = 0;
+
+ fill_request(&request, path, NULL);
+ request.data.flags = flags;
+ request.data.type = T_OPEN;
+ fd = v9fs_request(fs_ctx, &request, &error);
+ if (fd < 0) {
+ errno = error;
+ }
+ qemu_free(request.path.path);
+ return fd;
+}
static int local_lstat(FsContext *fs_ctx, const char *path, struct stat *stbuf)
{
@@ -138,14 +169,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 __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 = __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] 20+ messages in thread
* [Qemu-devel] [V3 PATCH 5/8] virtio-9p: Create support in chroot environment
2011-01-18 6:23 [Qemu-devel] [V3 PATCH 0/8] virtio-9p: Use chroot to safely access files in passthrough model M. Mohan Kumar
` (3 preceding siblings ...)
2011-01-18 6:25 ` [Qemu-devel] [V3 PATCH 4/8] virtio-9p: Add support to open a file in " M. Mohan Kumar
@ 2011-01-18 6:25 ` M. Mohan Kumar
2011-01-18 17:08 ` Blue Swirl
2011-01-18 6:26 ` [Qemu-devel] [V3 PATCH 6/8] virtio-9p: Support for creating special files M. Mohan Kumar
` (2 subsequent siblings)
7 siblings, 1 reply; 20+ messages in thread
From: M. Mohan Kumar @ 2011-01-18 6:25 UTC (permalink / raw)
To: qemu-devel
Add both server & client side interfaces to create regular files in
chroot environment
Signed-off-by: M. Mohan Kumar <mohan@in.ibm.com>
---
hw/9pfs/virtio-9p-chroot.c | 42 ++++++++++++++++++++++++++++++++++++++++++
hw/9pfs/virtio-9p-local.c | 22 ++++++++++++++++++++--
2 files changed, 62 insertions(+), 2 deletions(-)
diff --git a/hw/9pfs/virtio-9p-chroot.c b/hw/9pfs/virtio-9p-chroot.c
index b599e23..e7f85e2 100644
--- a/hw/9pfs/virtio-9p-chroot.c
+++ b/hw/9pfs/virtio-9p-chroot.c
@@ -193,6 +193,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)
+{
+ int cur_uid, cur_gid;
+
+ cur_uid = geteuid();
+ cur_gid = getegid();
+
+ fd_info->fi_fd = -1;
+
+ if (setfsuid(request->data.uid) < 0) {
+ fd_info->fi_error = errno;
+ return;
+ }
+ if (setfsgid(request->data.gid) < 0) {
+ fd_info->fi_error = errno;
+ 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_error = errno;
+ } else {
+ fd_info->fi_error = 0;
+ }
+
+ setfsgid(cur_gid);
+unset_uid:
+ setfsuid(cur_uid);
+}
+
static int chroot_daemonize(int chroot_sock)
{
sigset_t sigset;
@@ -276,6 +312,12 @@ int v9fs_chroot(FsContext *fs_ctx)
error = -2;
}
break;
+ case T_CREATE:
+ chroot_do_create(&request, &fd_info);
+ if (chroot_sendfd(chroot_sock, &fd_info) <= 0) {
+ error = -2;
+ }
+ break;
default:
break;
}
diff --git a/hw/9pfs/virtio-9p-local.c b/hw/9pfs/virtio-9p-local.c
index 2376ec2..7f39b40 100644
--- a/hw/9pfs/virtio-9p-local.c
+++ b/hw/9pfs/virtio-9p-local.c
@@ -52,6 +52,23 @@ static int __open(FsContext *fs_ctx, const char *path, int flags)
return fd;
}
+static int __create(FsContext *fs_ctx, const char *path, int flags,
+ FsCred *credp)
+{
+ V9fsFileObjectRequest request;
+ int fd, error = 0;
+
+ fill_request(&request, path, credp);
+ request.data.flags = flags;
+ request.data.type = T_CREATE;
+ fd = v9fs_request(fs_ctx, &request, &error);
+ if (fd < 0) {
+ errno = error;
+ }
+ qemu_free(request.path.path);
+ return fd;
+}
+
static int local_lstat(FsContext *fs_ctx, const char *path, struct stat *stbuf)
{
int err;
@@ -376,8 +393,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;
@@ -387,6 +403,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 = __create(fs_ctx, path, flags, credp);
}
return fd;
--
1.7.3.4
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Qemu-devel] [V3 PATCH 6/8] virtio-9p: Support for creating special files
2011-01-18 6:23 [Qemu-devel] [V3 PATCH 0/8] virtio-9p: Use chroot to safely access files in passthrough model M. Mohan Kumar
` (4 preceding siblings ...)
2011-01-18 6:25 ` [Qemu-devel] [V3 PATCH 5/8] virtio-9p: Create support " M. Mohan Kumar
@ 2011-01-18 6:26 ` M. Mohan Kumar
2011-01-18 17:11 ` Blue Swirl
2011-01-18 6:26 ` [Qemu-devel] [V3 PATCH 8/8] virtio-9p: Chroot environment for other functions M. Mohan Kumar
2011-01-18 8:24 ` [Qemu-devel] [V3 PATCH 7/8] virtio-9p: Move file post creation changes to none security model M. Mohan Kumar
7 siblings, 1 reply; 20+ messages in thread
From: M. Mohan Kumar @ 2011-01-18 6:26 UTC (permalink / raw)
To: qemu-devel
Add both server and client 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.c | 84 ++++++++++++++++++++++++++-
hw/9pfs/virtio-9p-chroot.h | 2 +
hw/9pfs/virtio-9p-local.c | 141 ++++++++++++++++++++++++++++++++++----------
3 files changed, 195 insertions(+), 32 deletions(-)
diff --git a/hw/9pfs/virtio-9p-chroot.c b/hw/9pfs/virtio-9p-chroot.c
index e7f85e2..92a4917 100644
--- a/hw/9pfs/virtio-9p-chroot.c
+++ b/hw/9pfs/virtio-9p-chroot.c
@@ -193,6 +193,29 @@ static void chroot_do_open(V9fsFileObjectRequest *request, FdInfo *fd_info)
}
}
+int v9fs_create_special(FsContext *fs_ctx,
+ V9fsFileObjectRequest *request, int *error)
+{
+ int retval;
+
+ pthread_mutex_lock(&fs_ctx->chroot_mutex);
+
+ *error = 0;
+ v9fs_write_request(fs_ctx->chroot_socket, request);
+ retval = qemu_read_full(fs_ctx->chroot_socket, error, sizeof(*error));
+ if (retval != sizeof(*error)) {
+ error_report("reading from socket failed: %s", strerror(errno));
+ exit(1);
+ }
+
+ pthread_mutex_unlock(&fs_ctx->chroot_mutex);
+ if (*error) {
+ return -1;
+ } else {
+ return 0;
+ }
+}
+
/*
* Helper routine to create a file and return the file descriptor and
* error status in FdInfo structure.
@@ -229,6 +252,56 @@ 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 on success, returns errno on failure
+ */
+static int chroot_do_create_special(V9fsFileObjectRequest *request)
+{
+ int cur_uid, cur_gid;
+ int retval, error;
+
+ cur_uid = geteuid();
+ cur_gid = getegid();
+
+ if (setfsuid(request->data.uid) < 0) {
+ return errno;
+ }
+ if (setfsgid(request->data.gid) < 0) {
+ error = 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) {
+ error = errno;
+ } else {
+ error = 0;
+ }
+
+ setfsgid(cur_gid);
+unset_uid:
+ setfsuid(cur_uid);
+
+ return error;
+}
+
static int chroot_daemonize(int chroot_sock)
{
sigset_t sigset;
@@ -263,7 +336,7 @@ static int chroot_daemonize(int chroot_sock)
*/
int v9fs_chroot(FsContext *fs_ctx)
{
- int fd_pair[2], pid, chroot_sock, error;
+ int fd_pair[2], pid, chroot_sock, error, retval;
V9fsFileObjectRequest request;
FdInfo fd_info;
@@ -318,6 +391,15 @@ int v9fs_chroot(FsContext *fs_ctx)
error = -2;
}
break;
+ case T_MKDIR:
+ case T_SYMLINK:
+ case T_LINK:
+ case T_MKNOD:
+ retval = chroot_do_create_special(&request);
+ if (qemu_write_full(chroot_sock, &retval, sizeof(retval)) < 0) {
+ error = -2;
+ }
+ break;
default:
break;
}
diff --git a/hw/9pfs/virtio-9p-chroot.h b/hw/9pfs/virtio-9p-chroot.h
index f5a2ca0..9a0ba88 100644
--- a/hw/9pfs/virtio-9p-chroot.h
+++ b/hw/9pfs/virtio-9p-chroot.h
@@ -36,5 +36,7 @@ typedef struct V9fsFileObjectRequest
int v9fs_chroot(FsContext *fs_ctx);
int v9fs_request(FsContext *fs_ctx, V9fsFileObjectRequest *or, int *error);
+int v9fs_create_special(FsContext *fs_ctx,
+ V9fsFileObjectRequest *request, int *error);
#endif /* _QEMU_VIRTIO_9P_CHROOT_H */
diff --git a/hw/9pfs/virtio-9p-local.c b/hw/9pfs/virtio-9p-local.c
index 7f39b40..08fd67f 100644
--- a/hw/9pfs/virtio-9p-local.c
+++ b/hw/9pfs/virtio-9p-local.c
@@ -69,6 +69,78 @@ static int __create(FsContext *fs_ctx, const char *path, int flags,
return fd;
}
+static int __mknod(FsContext *fs_ctx, const char *path, FsCred *credp)
+{
+ V9fsFileObjectRequest request;
+ int retval, error = 0;
+
+ fill_request(&request, path, credp);
+ request.data.type = T_MKNOD;
+
+ retval = v9fs_create_special(fs_ctx, &request, &error);
+ if (retval < 0) {
+ errno = error;
+ }
+ return retval;
+}
+
+static int __mkdir(FsContext *fs_ctx, const char *path, FsCred *credp)
+{
+ V9fsFileObjectRequest request;
+ int retval, error = 0;
+
+ fill_request(&request, path, credp);
+ request.data.type = T_MKDIR;
+
+ retval = v9fs_create_special(fs_ctx, &request, &error);
+ if (retval > 0) {
+ errno = error;
+ }
+ qemu_free(request.path.path);
+ return retval;
+}
+
+static int __symlink(FsContext *fs_ctx, const char *oldpath,
+ const char *newpath, FsCred *credp)
+{
+ V9fsFileObjectRequest request;
+ int retval, error = 0;
+
+ fill_request(&request, newpath, credp);
+ request.data.type = T_SYMLINK;
+ request.data.oldpath_len = strlen(oldpath);
+ request.path.old_path = qemu_strdup(oldpath);
+
+ retval = v9fs_create_special(fs_ctx, &request, &error);
+ if (retval > 0) {
+ errno = error;
+ return 0;
+ }
+ qemu_free(request.path.old_path);
+ qemu_free(request.path.path);
+ return retval;
+}
+
+static int __link(FsContext *fs_ctx, const char *oldpath,
+ const char *newpath)
+{
+ V9fsFileObjectRequest request;
+ int retval, error = 0;
+
+ fill_request(&request, newpath, NULL);
+ request.data.type = T_LINK;
+ request.data.oldpath_len = strlen(oldpath);
+ request.path.old_path = qemu_strdup(oldpath);
+ retval = v9fs_create_special(fs_ctx, &request, &error);
+ if (retval > 0) {
+ errno = error;
+ return 0;
+ }
+ qemu_free(request.path.old_path);
+ qemu_free(request.path.path);
+ return retval;
+}
+
static int local_lstat(FsContext *fs_ctx, const char *path, struct stat *stbuf)
{
int err;
@@ -286,8 +358,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;
@@ -297,6 +368,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 = __mknod(fs_ctx, path, credp);
+ if (err < 0) {
+ serrno = errno;
+ goto err_end;
+ }
}
return err;
@@ -323,8 +400,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;
@@ -334,6 +410,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 = __mkdir(fs_ctx, path, credp);
+ if (err < 0) {
+ serrno = errno;
+ goto err_end;
+ }
}
return err;
@@ -451,23 +533,18 @@ 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 = __symlink(fs_ctx, oldpath, newpath, credp);
+ if (err < 0) {
+ serrno = errno;
+ goto err_end;
}
}
return err;
@@ -478,24 +555,26 @@ 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 = __link(fs_ctx, oldpath, newpath);
+ 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] 20+ messages in thread
* [Qemu-devel] [V3 PATCH 8/8] virtio-9p: Chroot environment for other functions
2011-01-18 6:23 [Qemu-devel] [V3 PATCH 0/8] virtio-9p: Use chroot to safely access files in passthrough model M. Mohan Kumar
` (5 preceding siblings ...)
2011-01-18 6:26 ` [Qemu-devel] [V3 PATCH 6/8] virtio-9p: Support for creating special files M. Mohan Kumar
@ 2011-01-18 6:26 ` M. Mohan Kumar
2011-01-18 8:24 ` [Qemu-devel] [V3 PATCH 7/8] virtio-9p: Move file post creation changes to none security model M. Mohan Kumar
7 siblings, 0 replies; 20+ messages in thread
From: M. Mohan Kumar @ 2011-01-18 6:26 UTC (permalink / raw)
To: qemu-devel
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 | 222 ++++++++++++++++++++++++++++++++++++++------
1 files changed, 191 insertions(+), 31 deletions(-)
diff --git a/hw/9pfs/virtio-9p-local.c b/hw/9pfs/virtio-9p-local.c
index d2e32e2..9586190 100644
--- a/hw/9pfs/virtio-9p-local.c
+++ b/hw/9pfs/virtio-9p-local.c
@@ -20,6 +20,7 @@
#include <sys/socket.h>
#include <sys/un.h>
#include <attr/xattr.h>
+#include <libgen.h>
/* Helper routine to fill V9fsFileObjectRequest structure */
static void fill_request(V9fsFileObjectRequest *request, const char *path,
@@ -141,14 +142,36 @@ static int __link(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, error = 0;
+ char *dpath = qemu_strdup(path);
+
+ fill_request(&request, dirname(dpath), NULL);
+ request.data.type = T_OPEN;
+ fd = v9fs_request(fs_ctx, &request, &error);
+ if (fd < 0) {
+ errno = error;
+ }
+ qemu_free(dpath);
+ qemu_free(request.path.path);
+ 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;
@@ -170,6 +193,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;
}
@@ -234,9 +278,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;
}
@@ -328,8 +386,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;
@@ -575,53 +648,140 @@ 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 = __open(ctx, path, O_RDWR);
+ if (fd < 0) {
+ return fd;
+ }
+ 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)) {
- return lchown(rpath(fs_ctx, path), credp->fc_uid, credp->fc_gid);
- } else if (fs_ctx->fs_sm == SM_MAPPED) {
+ 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_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 = __open(fs_ctx, path, O_RDONLY);
+ 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] 20+ messages in thread
* [Qemu-devel] [V3 PATCH 7/8] virtio-9p: Move file post creation changes to none security model
2011-01-18 6:23 [Qemu-devel] [V3 PATCH 0/8] virtio-9p: Use chroot to safely access files in passthrough model M. Mohan Kumar
` (6 preceding siblings ...)
2011-01-18 6:26 ` [Qemu-devel] [V3 PATCH 8/8] virtio-9p: Chroot environment for other functions M. Mohan Kumar
@ 2011-01-18 8:24 ` M. Mohan Kumar
2011-01-20 8:59 ` Stefan Hajnoczi
7 siblings, 1 reply; 20+ messages in thread
From: M. Mohan Kumar @ 2011-01-18 8:24 UTC (permalink / raw)
To: qemu-devel
After creating a file object, its permission and ownership details are updated
as per 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 08fd67f..d2e32e2 100644
--- a/hw/9pfs/virtio-9p-local.c
+++ b/hw/9pfs/virtio-9p-local.c
@@ -208,21 +208,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;
}
@@ -363,7 +356,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;
@@ -405,7 +398,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;
@@ -480,7 +473,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] 20+ messages in thread
* Re: [Qemu-devel] [V3 PATCH 2/8] virtio-9p: Provide chroot environment server side interfaces
2011-01-18 6:25 ` [Qemu-devel] [V3 PATCH 2/8] virtio-9p: Provide chroot environment server side interfaces M. Mohan Kumar
@ 2011-01-18 17:03 ` Blue Swirl
0 siblings, 0 replies; 20+ messages in thread
From: Blue Swirl @ 2011-01-18 17:03 UTC (permalink / raw)
To: M. Mohan Kumar; +Cc: qemu-devel
On Tue, Jan 18, 2011 at 6:25 AM, M. Mohan Kumar <mohan@in.ibm.com> wrote:
> Implement chroot server 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.c | 183 ++++++++++++++++++++++++++++++++++++++++++++
> hw/9pfs/virtio-9p-chroot.h | 39 +++++++++
> hw/9pfs/virtio-9p.c | 23 ++++++
> hw/file-op-9p.h | 2 +
> 5 files changed, 248 insertions(+), 0 deletions(-)
> create mode 100644 hw/9pfs/virtio-9p-chroot.c
> create mode 100644 hw/9pfs/virtio-9p-chroot.h
>
> diff --git a/Makefile.objs b/Makefile.objs
> index bc0344c..3007b6d 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -273,6 +273,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.o
>
> hw-obj-$(CONFIG_REALLY_VIRTFS) += $(addprefix 9pfs/, $(9pfs-nested-y))
> $(addprefix 9pfs/, $(9pfs-nested-y)): CFLAGS += -I$(SRC_PATH)/hw/
> diff --git a/hw/9pfs/virtio-9p-chroot.c b/hw/9pfs/virtio-9p-chroot.c
> new file mode 100644
> index 0000000..dcde2cc
> --- /dev/null
> +++ b/hw/9pfs/virtio-9p-chroot.c
> @@ -0,0 +1,183 @@
> +/*
> + * Virtio 9p chroot environment for contained access to exported path
> + *
> + * 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"
> +
> +/*
> + * Structure used by chroot functions to transmit file descriptor and
> + * error info
> + */
> +typedef struct {
> + int fi_fd;
> + int fi_error;
> +} FdInfo;
> +
> +union MsgControl {
> + struct cmsghdr cmsg;
> + char control[CMSG_SPACE(sizeof(int))];
> +};
> +
> +/* Send file descriptor and error status to qemu process */
> +static int chroot_sendfd(int sockfd, FdInfo *fd_info)
const FdInfo *?
> +{
> + struct msghdr msg = { };
> + struct iovec iov;
> + struct cmsghdr *cmsg;
> + int retval;
> + union MsgControl msg_control;
> +
> + iov.iov_base = 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 */
> + if (!fd_info->fi_error) {
> + 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);
> + close(fd_info->fi_fd);
> + return retval;
> +}
> +
> +/* Read V9fsFileObjectRequest written by QEMU process */
> +static void chroot_read_request(int sockfd, V9fsFileObjectRequest *request)
> +{
> + int retval;
> + retval = qemu_read_full(sockfd, request, sizeof(request->data));
> + if (retval <= 0 || request->data.path_len <= 0) {
> + _exit(-1);
> + }
> + request->path.path = qemu_mallocz(request->data.path_len + 1);
> + retval = qemu_read_full(sockfd, (void *)request->path.path,
> + request->data.path_len);
> + if (retval <= 0) {
> + _exit(-1);
> + }
> + if (request->data.oldpath_len > 0) {
> + request->path.old_path =
> + qemu_mallocz(request->data.oldpath_len + 1);
> + retval = qemu_read_full(sockfd, (void *)request->path.old_path,
> + request->data.oldpath_len);
> + if (retval <= 0) {
> + _exit(-1);
> + }
> + }
> +}
> +
> +static int chroot_daemonize(int chroot_sock)
> +{
> + sigset_t sigset;
> + struct rlimit nr_fd;
> + int fd;
> +
> + /* Block all signals for this process */
> + sigprocmask(SIG_SETMASK, &sigset, NULL);
> +
> + /* Daemonize */
> + if (setsid() < 0) {
> + error_report("setsid %s", strerror(errno));
> + return -1;
> + }
> +
> + /* Close other file descriptors */
> + getrlimit(RLIMIT_NOFILE, &nr_fd);
> + for (fd = 0; fd < nr_fd.rlim_cur; fd++) {
> + if (fd != chroot_sock) {
> + close(fd);
> + }
> + }
> +
> + /* Create files with mode as requested by client */
> + umask(0);
> + return 0;
> +}
> +
> +static void chroot_dummy(void)
> +{
> + (void)chroot_sendfd;
> +}
> +
> +/*
> + * Fork a process and chroot into the share path. Communication
> + * between qemu process and chroot process happens via socket
> + */
> +int v9fs_chroot(FsContext *fs_ctx)
> +{
> + int fd_pair[2], pid, chroot_sock, error;
The type of pid should be pid_t.
> + V9fsFileObjectRequest request;
> +
> + 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) {
> + error_report("chroot %s", strerror(errno));
> + return -1;
> + }
> +
> + chroot_dummy();
> + if (chroot_daemonize(chroot_sock) < 0) {
> + _exit(-1);
> + }
> +
> + /*
> + * Write 0 to chroot socket to indicate chroot process creation is
> + * successful
> + */
> + error = 0;
> + if (qemu_write_full(chroot_sock, &error, sizeof(error)) != sizeof(error)) {
> + _exit(-1);
> + }
> + /* get the request from the socket */
> + while (1) {
> + chroot_read_request(chroot_sock, &request);
> + qemu_free(request.path.path);
> + if (request.data.oldpath_len) {
> + qemu_free(request.path.old_path);
> + }
> + if (error) {
> + _exit(error);
> + }
> + }
> +}
> diff --git a/hw/9pfs/virtio-9p-chroot.h b/hw/9pfs/virtio-9p-chroot.h
> new file mode 100644
> index 0000000..3bee5e3
> --- /dev/null
> +++ b/hw/9pfs/virtio-9p-chroot.h
> @@ -0,0 +1,39 @@
> +#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
> +
> +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;
> + char *old_path;
Can these be const char *?
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [V3 PATCH 5/8] virtio-9p: Create support in chroot environment
2011-01-18 6:25 ` [Qemu-devel] [V3 PATCH 5/8] virtio-9p: Create support " M. Mohan Kumar
@ 2011-01-18 17:08 ` Blue Swirl
2011-01-19 11:08 ` M. Mohan Kumar
0 siblings, 1 reply; 20+ messages in thread
From: Blue Swirl @ 2011-01-18 17:08 UTC (permalink / raw)
To: M. Mohan Kumar; +Cc: qemu-devel
On Tue, Jan 18, 2011 at 6:25 AM, M. Mohan Kumar <mohan@in.ibm.com> wrote:
> Add both server & client side interfaces to create regular files in
> chroot environment
>
> Signed-off-by: M. Mohan Kumar <mohan@in.ibm.com>
> ---
> hw/9pfs/virtio-9p-chroot.c | 42 ++++++++++++++++++++++++++++++++++++++++++
> hw/9pfs/virtio-9p-local.c | 22 ++++++++++++++++++++--
> 2 files changed, 62 insertions(+), 2 deletions(-)
>
> diff --git a/hw/9pfs/virtio-9p-chroot.c b/hw/9pfs/virtio-9p-chroot.c
> index b599e23..e7f85e2 100644
> --- a/hw/9pfs/virtio-9p-chroot.c
> +++ b/hw/9pfs/virtio-9p-chroot.c
> @@ -193,6 +193,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)
> +{
> + int cur_uid, cur_gid;
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_error = errno;
> + return;
> + }
> + if (setfsgid(request->data.gid) < 0) {
> + fd_info->fi_error = errno;
> + 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_error = errno;
> + } else {
> + fd_info->fi_error = 0;
> + }
> +
> + setfsgid(cur_gid);
> +unset_uid:
> + setfsuid(cur_uid);
> +}
> +
> static int chroot_daemonize(int chroot_sock)
> {
> sigset_t sigset;
> @@ -276,6 +312,12 @@ int v9fs_chroot(FsContext *fs_ctx)
> error = -2;
> }
> break;
> + case T_CREATE:
> + chroot_do_create(&request, &fd_info);
> + if (chroot_sendfd(chroot_sock, &fd_info) <= 0) {
> + error = -2;
> + }
> + break;
> default:
> break;
> }
> diff --git a/hw/9pfs/virtio-9p-local.c b/hw/9pfs/virtio-9p-local.c
> index 2376ec2..7f39b40 100644
> --- a/hw/9pfs/virtio-9p-local.c
> +++ b/hw/9pfs/virtio-9p-local.c
> @@ -52,6 +52,23 @@ static int __open(FsContext *fs_ctx, const char *path, int flags)
> return fd;
> }
>
> +static int __create(FsContext *fs_ctx, const char *path, int flags,
Please don't use identifiers starting with underscores.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [V3 PATCH 6/8] virtio-9p: Support for creating special files
2011-01-18 6:26 ` [Qemu-devel] [V3 PATCH 6/8] virtio-9p: Support for creating special files M. Mohan Kumar
@ 2011-01-18 17:11 ` Blue Swirl
0 siblings, 0 replies; 20+ messages in thread
From: Blue Swirl @ 2011-01-18 17:11 UTC (permalink / raw)
To: M. Mohan Kumar; +Cc: qemu-devel
On Tue, Jan 18, 2011 at 6:26 AM, M. Mohan Kumar <mohan@in.ibm.com> wrote:
> Add both server and client 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.c | 84 ++++++++++++++++++++++++++-
> hw/9pfs/virtio-9p-chroot.h | 2 +
> hw/9pfs/virtio-9p-local.c | 141 ++++++++++++++++++++++++++++++++++----------
> 3 files changed, 195 insertions(+), 32 deletions(-)
>
> diff --git a/hw/9pfs/virtio-9p-chroot.c b/hw/9pfs/virtio-9p-chroot.c
> index e7f85e2..92a4917 100644
> --- a/hw/9pfs/virtio-9p-chroot.c
> +++ b/hw/9pfs/virtio-9p-chroot.c
> @@ -193,6 +193,29 @@ static void chroot_do_open(V9fsFileObjectRequest *request, FdInfo *fd_info)
> }
> }
>
> +int v9fs_create_special(FsContext *fs_ctx,
> + V9fsFileObjectRequest *request, int *error)
> +{
> + int retval;
> +
> + pthread_mutex_lock(&fs_ctx->chroot_mutex);
> +
> + *error = 0;
> + v9fs_write_request(fs_ctx->chroot_socket, request);
> + retval = qemu_read_full(fs_ctx->chroot_socket, error, sizeof(*error));
> + if (retval != sizeof(*error)) {
> + error_report("reading from socket failed: %s", strerror(errno));
> + exit(1);
> + }
> +
> + pthread_mutex_unlock(&fs_ctx->chroot_mutex);
> + if (*error) {
> + return -1;
> + } else {
> + return 0;
> + }
> +}
> +
> /*
> * Helper routine to create a file and return the file descriptor and
> * error status in FdInfo structure.
> @@ -229,6 +252,56 @@ 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 on success, returns errno on failure
> + */
> +static int chroot_do_create_special(V9fsFileObjectRequest *request)
> +{
> + int cur_uid, cur_gid;
uid_t cur_uid;
gid_t cur_gid;
> + int retval, error;
> +
> + cur_uid = geteuid();
> + cur_gid = getegid();
> +
> + if (setfsuid(request->data.uid) < 0) {
> + return errno;
> + }
> + if (setfsgid(request->data.gid) < 0) {
> + error = 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) {
> + error = errno;
> + } else {
> + error = 0;
> + }
> +
> + setfsgid(cur_gid);
> +unset_uid:
> + setfsuid(cur_uid);
> +
> + return error;
> +}
> +
> static int chroot_daemonize(int chroot_sock)
> {
> sigset_t sigset;
> @@ -263,7 +336,7 @@ static int chroot_daemonize(int chroot_sock)
> */
> int v9fs_chroot(FsContext *fs_ctx)
> {
> - int fd_pair[2], pid, chroot_sock, error;
> + int fd_pair[2], pid, chroot_sock, error, retval;
> V9fsFileObjectRequest request;
> FdInfo fd_info;
>
> @@ -318,6 +391,15 @@ int v9fs_chroot(FsContext *fs_ctx)
> error = -2;
> }
> break;
> + case T_MKDIR:
> + case T_SYMLINK:
> + case T_LINK:
> + case T_MKNOD:
> + retval = chroot_do_create_special(&request);
> + if (qemu_write_full(chroot_sock, &retval, sizeof(retval)) < 0) {
> + error = -2;
> + }
> + break;
> default:
> break;
> }
> diff --git a/hw/9pfs/virtio-9p-chroot.h b/hw/9pfs/virtio-9p-chroot.h
> index f5a2ca0..9a0ba88 100644
> --- a/hw/9pfs/virtio-9p-chroot.h
> +++ b/hw/9pfs/virtio-9p-chroot.h
> @@ -36,5 +36,7 @@ typedef struct V9fsFileObjectRequest
>
> int v9fs_chroot(FsContext *fs_ctx);
> int v9fs_request(FsContext *fs_ctx, V9fsFileObjectRequest *or, int *error);
> +int v9fs_create_special(FsContext *fs_ctx,
> + V9fsFileObjectRequest *request, int *error);
>
> #endif /* _QEMU_VIRTIO_9P_CHROOT_H */
> diff --git a/hw/9pfs/virtio-9p-local.c b/hw/9pfs/virtio-9p-local.c
> index 7f39b40..08fd67f 100644
> --- a/hw/9pfs/virtio-9p-local.c
> +++ b/hw/9pfs/virtio-9p-local.c
> @@ -69,6 +69,78 @@ static int __create(FsContext *fs_ctx, const char *path, int flags,
> return fd;
> }
>
> +static int __mknod(FsContext *fs_ctx, const char *path, FsCred *credp)
underscores
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [V3 PATCH 5/8] virtio-9p: Create support in chroot environment
2011-01-18 17:08 ` Blue Swirl
@ 2011-01-19 11:08 ` M. Mohan Kumar
0 siblings, 0 replies; 20+ messages in thread
From: M. Mohan Kumar @ 2011-01-19 11:08 UTC (permalink / raw)
To: Blue Swirl; +Cc: qemu-devel
Hi Blue Swirl,
Thanks for your review comments. I will address these in my next version of
patchset.
----
M. Mohan Kumar
On Tuesday 18 January 2011 10:38:21 pm Blue Swirl wrote:
> On Tue, Jan 18, 2011 at 6:25 AM, M. Mohan Kumar <mohan@in.ibm.com> wrote:
> > Add both server & client side interfaces to create regular files in
> > chroot environment
> >
> > Signed-off-by: M. Mohan Kumar <mohan@in.ibm.com>
> > ---
> > hw/9pfs/virtio-9p-chroot.c | 42
> > ++++++++++++++++++++++++++++++++++++++++++ hw/9pfs/virtio-9p-local.c |
> > 22 ++++++++++++++++++++--
> > 2 files changed, 62 insertions(+), 2 deletions(-)
> >
> > diff --git a/hw/9pfs/virtio-9p-chroot.c b/hw/9pfs/virtio-9p-chroot.c
> > index b599e23..e7f85e2 100644
> > --- a/hw/9pfs/virtio-9p-chroot.c
> > +++ b/hw/9pfs/virtio-9p-chroot.c
> > @@ -193,6 +193,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) +{
> > + int cur_uid, cur_gid;
>
> 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_error = errno;
> > + return;
> > + }
> > + if (setfsgid(request->data.gid) < 0) {
> > + fd_info->fi_error = errno;
> > + 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_error = errno;
> > + } else {
> > + fd_info->fi_error = 0;
> > + }
> > +
> > + setfsgid(cur_gid);
> > +unset_uid:
> > + setfsuid(cur_uid);
> > +}
> > +
> > static int chroot_daemonize(int chroot_sock)
> > {
> > sigset_t sigset;
> > @@ -276,6 +312,12 @@ int v9fs_chroot(FsContext *fs_ctx)
> > error = -2;
> > }
> > break;
> > + case T_CREATE:
> > + chroot_do_create(&request, &fd_info);
> > + if (chroot_sendfd(chroot_sock, &fd_info) <= 0) {
> > + error = -2;
> > + }
> > + break;
> > default:
> > break;
> > }
> > diff --git a/hw/9pfs/virtio-9p-local.c b/hw/9pfs/virtio-9p-local.c
> > index 2376ec2..7f39b40 100644
> > --- a/hw/9pfs/virtio-9p-local.c
> > +++ b/hw/9pfs/virtio-9p-local.c
> > @@ -52,6 +52,23 @@ static int __open(FsContext *fs_ctx, const char *path,
> > int flags) return fd;
> > }
> >
> > +static int __create(FsContext *fs_ctx, const char *path, int flags,
>
> Please don't use identifiers starting with underscores.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [V3 PATCH 7/8] virtio-9p: Move file post creation changes to none security model
2011-01-18 8:24 ` [Qemu-devel] [V3 PATCH 7/8] virtio-9p: Move file post creation changes to none security model M. Mohan Kumar
@ 2011-01-20 8:59 ` Stefan Hajnoczi
2011-01-20 14:41 ` M. Mohan Kumar
2011-01-20 21:15 ` Venkateswararao Jujjuri (JV)
0 siblings, 2 replies; 20+ messages in thread
From: Stefan Hajnoczi @ 2011-01-20 8:59 UTC (permalink / raw)
To: M. Mohan Kumar; +Cc: qemu-devel
On Tue, Jan 18, 2011 at 01:54:16PM +0530, M. Mohan Kumar wrote:
> After creating a file object, its permission and ownership details are updated
> as per 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 08fd67f..d2e32e2 100644
> --- a/hw/9pfs/virtio-9p-local.c
> +++ b/hw/9pfs/virtio-9p-local.c
> @@ -208,21 +208,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;
> }
retval is unused.
Can multiple virtio-9p requests execute at a time? chmod() and lchown()
after creation is a race condition if other requests can execute
concurrently.
Stefan
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [V3 PATCH 7/8] virtio-9p: Move file post creation changes to none security model
2011-01-20 8:59 ` Stefan Hajnoczi
@ 2011-01-20 14:41 ` M. Mohan Kumar
2011-01-20 14:48 ` Daniel P. Berrange
2011-01-20 21:15 ` Venkateswararao Jujjuri (JV)
1 sibling, 1 reply; 20+ messages in thread
From: M. Mohan Kumar @ 2011-01-20 14:41 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: qemu-devel
On Thursday 20 January 2011 2:29:54 pm Stefan Hajnoczi wrote:
> On Tue, Jan 18, 2011 at 01:54:16PM +0530, M. Mohan Kumar wrote:
> > - 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;
> >
> > }
>
> retval is unused.
>
That was used to disable the warning message "error: ignoring return value of
‘lchown’, declared with attribute warn_unused_result"
Otherwise I have to use
if (lchown(rpath(fs_ctx, path), credp->fc_uid, credp->fc_gid)) {
;
}
> Can multiple virtio-9p requests execute at a time? chmod() and lchown()
> after creation is a race condition if other requests can execute
> concurrently.
>
We can't implement file creation with requested user credentials and permission
bits in the none security model atomically. Its expected behaviour only
----
M. Mohan Kumar
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [V3 PATCH 7/8] virtio-9p: Move file post creation changes to none security model
2011-01-20 14:41 ` M. Mohan Kumar
@ 2011-01-20 14:48 ` Daniel P. Berrange
2011-01-20 21:15 ` Stefan Hajnoczi
0 siblings, 1 reply; 20+ messages in thread
From: Daniel P. Berrange @ 2011-01-20 14:48 UTC (permalink / raw)
To: M. Mohan Kumar; +Cc: Stefan Hajnoczi, qemu-devel
On Thu, Jan 20, 2011 at 08:11:27PM +0530, M. Mohan Kumar wrote:
> On Thursday 20 January 2011 2:29:54 pm Stefan Hajnoczi wrote:
> > On Tue, Jan 18, 2011 at 01:54:16PM +0530, M. Mohan Kumar wrote:
>
> > > - 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;
> > >
> > > }
> >
> > retval is unused.
> >
>
> That was used to disable the warning message "error: ignoring return value of
> ‘lchown’, declared with attribute warn_unused_result"
>
> Otherwise I have to use
> if (lchown(rpath(fs_ctx, path), credp->fc_uid, credp->fc_gid)) {
> ;
> }
>
> > Can multiple virtio-9p requests execute at a time? chmod() and lchown()
> > after creation is a race condition if other requests can execute
> > concurrently.
> >
>
> We can't implement file creation with requested user credentials and permission
> bits in the none security model atomically. Its expected behaviour only
Well you could do the nasty trick of forking a child process
and doing setuid/gid in that and then creating the file before
letting the parent continue.
if ((pid = fork()) == 0) {
setuid(fc_uid);
setgid(fc_gid);
fd =open("foo", O_CREAT);
close(fd);
} else {
waitpid(pid);
}
This kind of approach is in fact required if you want to
be able to create files with a special uid/gid on a root
squashing NFS server, because otherwise your QEMU running
as root will have its files squashed to 'nobody' when initially
created, and lchown will fail with EPERM. You might decide
that root squashing NFS is too painful to care about supporting
though :-)
Regards,
Daniel
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [V3 PATCH 7/8] virtio-9p: Move file post creation changes to none security model
2011-01-20 8:59 ` Stefan Hajnoczi
2011-01-20 14:41 ` M. Mohan Kumar
@ 2011-01-20 21:15 ` Venkateswararao Jujjuri (JV)
2011-01-20 21:45 ` Stefan Hajnoczi
1 sibling, 1 reply; 20+ messages in thread
From: Venkateswararao Jujjuri (JV) @ 2011-01-20 21:15 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: M. Mohan Kumar, qemu-devel
On 1/20/2011 12:59 AM, Stefan Hajnoczi wrote:
> On Tue, Jan 18, 2011 at 01:54:16PM +0530, M. Mohan Kumar wrote:
>> After creating a file object, its permission and ownership details are updated
>> as per 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 08fd67f..d2e32e2 100644
>> --- a/hw/9pfs/virtio-9p-local.c
>> +++ b/hw/9pfs/virtio-9p-local.c
>> @@ -208,21 +208,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;
>> }
>
> retval is unused.
>
> Can multiple virtio-9p requests execute at a time? chmod() and lchown()
> after creation is a race condition if other requests can execute
> concurrently.
If some level of serialization is needed it will be done at the client/guest
inode level.
Are you worried about filesystem semantics? or do you see some corruption if they
get executed in parallel?
JV
>
> Stefan
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [V3 PATCH 7/8] virtio-9p: Move file post creation changes to none security model
2011-01-20 14:48 ` Daniel P. Berrange
@ 2011-01-20 21:15 ` Stefan Hajnoczi
0 siblings, 0 replies; 20+ messages in thread
From: Stefan Hajnoczi @ 2011-01-20 21:15 UTC (permalink / raw)
To: M. Mohan Kumar; +Cc: qemu-devel
On Thu, Jan 20, 2011 at 2:48 PM, Daniel P. Berrange <berrange@redhat.com> wrote:
> On Thu, Jan 20, 2011 at 08:11:27PM +0530, M. Mohan Kumar wrote:
>> On Thursday 20 January 2011 2:29:54 pm Stefan Hajnoczi wrote:
>> > On Tue, Jan 18, 2011 at 01:54:16PM +0530, M. Mohan Kumar wrote:
>>
>> > > - 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;
>> > >
>> > > }
>> >
>> > retval is unused.
>> >
>>
>> That was used to disable the warning message "error: ignoring return value of
>> ‘lchown’, declared with attribute warn_unused_result"
>>
>> Otherwise I have to use
>> if (lchown(rpath(fs_ctx, path), credp->fc_uid, credp->fc_gid)) {
>> ;
>> }
>>
>> > Can multiple virtio-9p requests execute at a time? chmod() and lchown()
>> > after creation is a race condition if other requests can execute
>> > concurrently.
>> >
>>
>> We can't implement file creation with requested user credentials and permission
>> bits in the none security model atomically. Its expected behaviour only
>
> Well you could do the nasty trick of forking a child process
> and doing setuid/gid in that and then creating the file before
> letting the parent continue.
>
> if ((pid = fork()) == 0) {
> setuid(fc_uid);
> setgid(fc_gid);
> fd =open("foo", O_CREAT);
> close(fd);
> } else {
> waitpid(pid);
> }
>
> This kind of approach is in fact required if you want to
> be able to create files with a special uid/gid on a root
> squashing NFS server, because otherwise your QEMU running
> as root will have its files squashed to 'nobody' when initially
> created, and lchown will fail with EPERM. You might decide
> that root squashing NFS is too painful to care about supporting
> though :-)
I was thinking about this approach and it's similar to the chroot
helper process, but this time you have a helper process that does
umask/setgid/setuid as necessary. Performance will be bad but there's
really no way around this.
Either implement something that works 90% of the time only but runs a
bit faster or implement something that works all the time but runs
slow. It's not a nice trade-off.
Stefan
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [V3 PATCH 7/8] virtio-9p: Move file post creation changes to none security model
2011-01-20 21:15 ` Venkateswararao Jujjuri (JV)
@ 2011-01-20 21:45 ` Stefan Hajnoczi
2011-01-21 6:55 ` Venkateswararao Jujjuri (JV)
0 siblings, 1 reply; 20+ messages in thread
From: Stefan Hajnoczi @ 2011-01-20 21:45 UTC (permalink / raw)
To: Venkateswararao Jujjuri (JV); +Cc: M. Mohan Kumar, qemu-devel
On Thu, Jan 20, 2011 at 9:15 PM, Venkateswararao Jujjuri (JV)
<jvrao@linux.vnet.ibm.com> wrote:
> On 1/20/2011 12:59 AM, Stefan Hajnoczi wrote:
>> On Tue, Jan 18, 2011 at 01:54:16PM +0530, M. Mohan Kumar wrote:
>>> After creating a file object, its permission and ownership details are updated
>>> as per 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 08fd67f..d2e32e2 100644
>>> --- a/hw/9pfs/virtio-9p-local.c
>>> +++ b/hw/9pfs/virtio-9p-local.c
>>> @@ -208,21 +208,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;
>>> }
>>
>> retval is unused.
>>
>> Can multiple virtio-9p requests execute at a time? chmod() and lchown()
>> after creation is a race condition if other requests can execute
>> concurrently.
>
> If some level of serialization is needed it will be done at the client/guest
> inode level.
> Are you worried about filesystem semantics? or do you see some corruption if they
> get executed in parallel?
My main concern is unreliable results due to the race conditions
between creation and the fixups that are performed afterwards.
Is virtio-9p only useful for single guest exclusive access? I thought
both guest and host could access files at the same time? What about
multiple VMs sharing a directory? These scenarios can only work if
operations are made atomic.
Stefan
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [V3 PATCH 7/8] virtio-9p: Move file post creation changes to none security model
2011-01-20 21:45 ` Stefan Hajnoczi
@ 2011-01-21 6:55 ` Venkateswararao Jujjuri (JV)
0 siblings, 0 replies; 20+ messages in thread
From: Venkateswararao Jujjuri (JV) @ 2011-01-21 6:55 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: M. Mohan Kumar, qemu-devel
On 1/20/2011 1:45 PM, Stefan Hajnoczi wrote:
> On Thu, Jan 20, 2011 at 9:15 PM, Venkateswararao Jujjuri (JV)
> <jvrao@linux.vnet.ibm.com> wrote:
>> On 1/20/2011 12:59 AM, Stefan Hajnoczi wrote:
>>> On Tue, Jan 18, 2011 at 01:54:16PM +0530, M. Mohan Kumar wrote:
>>>> After creating a file object, its permission and ownership details are updated
>>>> as per 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 08fd67f..d2e32e2 100644
>>>> --- a/hw/9pfs/virtio-9p-local.c
>>>> +++ b/hw/9pfs/virtio-9p-local.c
>>>> @@ -208,21 +208,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;
>>>> }
>>>
>>> retval is unused.
>>>
>>> Can multiple virtio-9p requests execute at a time? chmod() and lchown()
>>> after creation is a race condition if other requests can execute
>>> concurrently.
>>
>> If some level of serialization is needed it will be done at the client/guest
>> inode level.
>> Are you worried about filesystem semantics? or do you see some corruption if they
>> get executed in parallel?
>
> My main concern is unreliable results due to the race conditions
> between creation and the fixups that are performed afterwards.
>
> Is virtio-9p only useful for single guest exclusive access? I thought
> both guest and host could access files at the same time? What about
> multiple VMs sharing a directory? These scenarios can only work if
> operations are made atomic.
For now, there is only one exploiter for the filesystem. The Guest/client.
In the future it could be different and we 'may' support multiple exploiters/users.
Note that we have two security models
1. Passthrough 2. Mapped. (3. None - can be ignored as it is intended for
developer)
Mapped model is advised when you have only one exploiter;
Passthrough model is for more practical application/uses and it can be
used for multiple exploiters (say guests).
In passthrough model we don't do chmod() lchmod() after creating files.
Thanks,
JV
>
> Stefan
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2011-01-21 6:55 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-01-18 6:23 [Qemu-devel] [V3 PATCH 0/8] virtio-9p: Use chroot to safely access files in passthrough model M. Mohan Kumar
2011-01-18 6:25 ` [Qemu-devel] [V3 PATCH 1/8] virtio-9p: Implement qemu_read_full M. Mohan Kumar
2011-01-18 6:25 ` [Qemu-devel] [V3 PATCH 2/8] virtio-9p: Provide chroot environment server side interfaces M. Mohan Kumar
2011-01-18 17:03 ` Blue Swirl
2011-01-18 6:25 ` [Qemu-devel] [V3 PATCH 3/8] virtio-9p: Add client side interfaces for chroot environment M. Mohan Kumar
2011-01-18 6:25 ` [Qemu-devel] [V3 PATCH 4/8] virtio-9p: Add support to open a file in " M. Mohan Kumar
2011-01-18 6:25 ` [Qemu-devel] [V3 PATCH 5/8] virtio-9p: Create support " M. Mohan Kumar
2011-01-18 17:08 ` Blue Swirl
2011-01-19 11:08 ` M. Mohan Kumar
2011-01-18 6:26 ` [Qemu-devel] [V3 PATCH 6/8] virtio-9p: Support for creating special files M. Mohan Kumar
2011-01-18 17:11 ` Blue Swirl
2011-01-18 6:26 ` [Qemu-devel] [V3 PATCH 8/8] virtio-9p: Chroot environment for other functions M. Mohan Kumar
2011-01-18 8:24 ` [Qemu-devel] [V3 PATCH 7/8] virtio-9p: Move file post creation changes to none security model M. Mohan Kumar
2011-01-20 8:59 ` Stefan Hajnoczi
2011-01-20 14:41 ` M. Mohan Kumar
2011-01-20 14:48 ` Daniel P. Berrange
2011-01-20 21:15 ` Stefan Hajnoczi
2011-01-20 21:15 ` Venkateswararao Jujjuri (JV)
2011-01-20 21:45 ` Stefan Hajnoczi
2011-01-21 6:55 ` Venkateswararao Jujjuri (JV)
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.