qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH V4 00/13] Proxy FS driver for VirtFS
@ 2011-12-05 16:18 M. Mohan Kumar
  2011-12-05 16:18 ` [Qemu-devel] [PATCH V4 01/13] hw/9pfs: Move pdu_marshal/unmarshal code to a seperate file M. Mohan Kumar
                   ` (13 more replies)
  0 siblings, 14 replies; 22+ messages in thread
From: M. Mohan Kumar @ 2011-12-05 16:18 UTC (permalink / raw)
  To: qemu-devel, aneesh.kumar, stefanha; +Cc: M. Mohan Kumar

From: "M. Mohan Kumar" <mohan@in.ibm.com>

Pass-through security model in QEMU 9p server needs root privilege to do
few file operations (like chown, chmod to any mode/uid:gid).  There are two
issues in pass-through security model

1) TOCTTOU vulnerability: Following symbolic links in the server could
provide access to files beyond 9p export path.

2) Running QEMU with root privilege could be a security issue.

To overcome above issues, following approach is used: A new filesytem
type 'proxy' is introduced. Proxy FS uses chroot + socket combination
for securing the vulnerability known with following symbolic links.
Intention of adding a new filesystem type is to allow qemu to run
in non-root mode, but doing privileged operations using socket IO.

Proxy helper(a stand alone binary part of qemu) is invoked with
root privileges. Proxy helper chroots into 9p export path and creates
a socket pair or a named socket based on the command line parameter.
Qemu and proxy helper communicate using this socket. QEMU proxy fs
driver sends filesystem request to proxy helper and receives the
response from it.

Proxy helper is designed so that it can drop the root privilege but
retaining capbilities that are needed for doing filesystem operations
(like CAP_DAC_OVERRIDE, CAP_FOWNER etc)

Changes from previous version V3:
* Improve error handling in marshaling/unmarshaling code
* Return ENOBUFS when request/response size exceeds maximum buffer size

Changes from previous version V2:
* Added check to handle failures in marshaling/unmarshaling code. Also make
  sure that request/response will not exceed maximum buffer size (BUFF_SZ)
* Addressed Stefan's review comments

Changes from previous version:
*) Communication between qemu and helper process is similar to 9p way of packing
elements (pdu marshaling).

M. Mohan Kumar (13):
  hw/9pfs: Move pdu_marshal/unmarshal code to a seperate file
  hw/9pfs: Add validation to {un}marshal code
  hw/9pfs: Add new proxy filesystem driver
  hw/9pfs: File system helper process for qemu 9p proxy FS
  hw/9pfs: Open and create files
  hw/9pfs: Create other filesystem objects
  hw/9pfs: Add stat/readlink/statfs for proxy FS
  hw/9pfs: File ownership and others
  hw/9pfs: xattr interfaces in proxy filesystem driver
  hw/9pfs: Proxy getversion
  hw/9pfs: Documentation changes related to proxy fs
  hw/9pfs: man page for proxy helper
  hw/9pfs: Add support to use named socket for proxy FS

 Makefile                       |   15 +-
 Makefile.objs                  |    3 +-
 configure                      |   19 +
 fsdev/file-op-9p.h             |    2 +
 fsdev/qemu-fsdev.c             |    1 +
 fsdev/qemu-fsdev.h             |    1 +
 fsdev/virtfs-proxy-helper.c    | 1134 ++++++++++++++++++++++++++++++++++++
 fsdev/virtfs-proxy-helper.texi |   63 ++
 fsdev/virtio-9p-marshal.c      |  360 ++++++++++++
 fsdev/virtio-9p-marshal.h      |   90 +++
 hw/9pfs/virtio-9p-proxy.c      | 1230 ++++++++++++++++++++++++++++++++++++++++
 hw/9pfs/virtio-9p-proxy.h      |   95 +++
 hw/9pfs/virtio-9p.c            |  756 +++++++++++--------------
 hw/9pfs/virtio-9p.h            |   83 +---
 qemu-config.c                  |   13 +
 qemu-options.hx                |   32 +-
 vl.c                           |   10 +-
 17 files changed, 3406 insertions(+), 501 deletions(-)
 create mode 100644 fsdev/virtfs-proxy-helper.c
 create mode 100644 fsdev/virtfs-proxy-helper.texi
 create mode 100644 fsdev/virtio-9p-marshal.c
 create mode 100644 fsdev/virtio-9p-marshal.h
 create mode 100644 hw/9pfs/virtio-9p-proxy.c
 create mode 100644 hw/9pfs/virtio-9p-proxy.h

-- 
1.7.6

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [Qemu-devel] [PATCH V4 01/13] hw/9pfs: Move pdu_marshal/unmarshal code to a seperate file
  2011-12-05 16:18 [Qemu-devel] [PATCH V4 00/13] Proxy FS driver for VirtFS M. Mohan Kumar
@ 2011-12-05 16:18 ` M. Mohan Kumar
  2011-12-05 16:18 ` [Qemu-devel] [PATCH V4 02/13] hw/9pfs: Add validation to {un}marshal code M. Mohan Kumar
                   ` (12 subsequent siblings)
  13 siblings, 0 replies; 22+ messages in thread
From: M. Mohan Kumar @ 2011-12-05 16:18 UTC (permalink / raw)
  To: qemu-devel, aneesh.kumar, stefanha; +Cc: M. Mohan Kumar

From: "M. Mohan Kumar" <mohan@in.ibm.com>

Move p9 marshaling/unmarshaling code to a separate file so that
proxy filesytem driver can use these calls. Also made marshaling
code generic to accept "struct iovec" instead of V9fsPDU.

Signed-off-by: M. Mohan Kumar <mohan@in.ibm.com>
---
 Makefile.objs             |    2 +-
 fsdev/virtio-9p-marshal.c |  338 +++++++++++++++++++++++++++++++++++++++++++++
 fsdev/virtio-9p-marshal.h |   87 ++++++++++++
 hw/9pfs/virtio-9p.c       |  297 +---------------------------------------
 hw/9pfs/virtio-9p.h       |   83 +----------
 5 files changed, 438 insertions(+), 369 deletions(-)
 create mode 100644 fsdev/virtio-9p-marshal.c
 create mode 100644 fsdev/virtio-9p-marshal.h

diff --git a/Makefile.objs b/Makefile.objs
index 3a699ee..1ca8369 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -61,7 +61,7 @@ ifeq ($(CONFIG_VIRTIO)$(CONFIG_VIRTFS)$(CONFIG_PCI),yyy)
 # Lots of the fsdev/9pcode is pulled in by vl.c via qemu_fsdev_add.
 # only pull in the actual virtio-9p device if we also enabled virtio.
 CONFIG_REALLY_VIRTFS=y
-fsdev-nested-y = qemu-fsdev.o
+fsdev-nested-y = qemu-fsdev.o virtio-9p-marshal.o
 else
 fsdev-nested-y = qemu-fsdev-dummy.o
 endif
diff --git a/fsdev/virtio-9p-marshal.c b/fsdev/virtio-9p-marshal.c
new file mode 100644
index 0000000..029c29e
--- /dev/null
+++ b/fsdev/virtio-9p-marshal.c
@@ -0,0 +1,338 @@
+/*
+ * Virtio 9p backend
+ *
+ * Copyright IBM, Corp. 2010
+ *
+ * Authors:
+ *  Anthony Liguori   <aliguori@us.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 <glib.h>
+#include <glib/gprintf.h>
+#include <sys/types.h>
+#include <dirent.h>
+#include <sys/time.h>
+#include <utime.h>
+#include <sys/uio.h>
+#include <string.h>
+#include <stdint.h>
+
+#include "compiler.h"
+#include "virtio-9p-marshal.h"
+#include "bswap.h"
+
+void v9fs_string_init(V9fsString *str)
+{
+    str->data = NULL;
+    str->size = 0;
+}
+
+void v9fs_string_free(V9fsString *str)
+{
+    g_free(str->data);
+    str->data = NULL;
+    str->size = 0;
+}
+
+void v9fs_string_null(V9fsString *str)
+{
+    v9fs_string_free(str);
+}
+
+void GCC_FMT_ATTR(2, 3)
+v9fs_string_sprintf(V9fsString *str, const char *fmt, ...)
+{
+    va_list ap;
+
+    v9fs_string_free(str);
+
+    va_start(ap, fmt);
+    str->size = g_vasprintf(&str->data, fmt, ap);
+    va_end(ap);
+}
+
+void v9fs_string_copy(V9fsString *lhs, V9fsString *rhs)
+{
+    v9fs_string_free(lhs);
+    v9fs_string_sprintf(lhs, "%s", rhs->data);
+}
+
+
+static size_t v9fs_packunpack(void *addr, struct iovec *sg, int sg_count,
+                              size_t offset, size_t size, int pack)
+{
+    int i = 0;
+    size_t copied = 0;
+
+    for (i = 0; size && i < sg_count; i++) {
+        size_t len;
+        if (offset >= sg[i].iov_len) {
+            /* skip this sg */
+            offset -= sg[i].iov_len;
+            continue;
+        } else {
+            len = MIN(sg[i].iov_len - offset, size);
+            if (pack) {
+                memcpy(sg[i].iov_base + offset, addr, len);
+            } else {
+                memcpy(addr, sg[i].iov_base + offset, len);
+            }
+            size -= len;
+            copied += len;
+            addr += len;
+            if (size) {
+                offset = 0;
+                continue;
+            }
+        }
+    }
+
+    return copied;
+}
+
+static size_t v9fs_unpack(void *dst, struct iovec *out_sg, int out_num,
+                          size_t offset, size_t size)
+{
+    return v9fs_packunpack(dst, out_sg, out_num, offset, size, 0);
+}
+
+size_t v9fs_pack(struct iovec *in_sg, int in_num, size_t offset,
+                const void *src, size_t size)
+{
+    return v9fs_packunpack((void *)src, in_sg, in_num, offset, size, 1);
+}
+
+static int v9fs_copy_sg(struct iovec *src_sg, unsigned int num,
+                        size_t offset, struct iovec *sg)
+{
+    int i, j;
+    size_t pos = 0;
+
+    j = 0;
+    for (i = 0; i < num; i++) {
+        if (offset <= pos) {
+            sg[j].iov_base = src_sg[i].iov_base;
+            sg[j].iov_len = src_sg[i].iov_len;
+            j++;
+        } else if (offset < (src_sg[i].iov_len + pos)) {
+            sg[j].iov_base = src_sg[i].iov_base;
+            sg[j].iov_len = src_sg[i].iov_len;
+            sg[j].iov_base += (offset - pos);
+            sg[j].iov_len -= (offset - pos);
+            j++;
+        }
+        pos += src_sg[i].iov_len;
+    }
+
+    return j;
+}
+
+size_t v9fs_unmarshal(struct iovec *out_sg, int out_num, size_t offset,
+                      int bswap, const char *fmt, ...)
+{
+    int i;
+    va_list ap;
+    size_t old_offset = offset;
+
+    va_start(ap, fmt);
+    for (i = 0; fmt[i]; i++) {
+        switch (fmt[i]) {
+        case 'b': {
+            uint8_t *valp = va_arg(ap, uint8_t *);
+            offset += v9fs_unpack(valp, out_sg, out_num, offset, sizeof(*valp));
+            break;
+        }
+        case 'w': {
+            uint16_t val, *valp;
+            valp = va_arg(ap, uint16_t *);
+            offset += v9fs_unpack(&val, out_sg, out_num, offset, sizeof(val));
+            if (bswap) {
+                *valp = le16_to_cpu(val);
+            } else {
+                *valp = val;
+            }
+            break;
+        }
+        case 'd': {
+            uint32_t val, *valp;
+            valp = va_arg(ap, uint32_t *);
+            offset += v9fs_unpack(&val, out_sg, out_num, offset, sizeof(val));
+            if (bswap) {
+                *valp = le32_to_cpu(val);
+            } else {
+                *valp = val;
+            }
+            break;
+        }
+        case 'q': {
+            uint64_t val, *valp;
+            valp = va_arg(ap, uint64_t *);
+            offset += v9fs_unpack(&val, out_sg, out_num, offset, sizeof(val));
+            if (bswap) {
+                *valp = le64_to_cpu(val);
+            } else {
+                *valp = val;
+            }
+            break;
+        }
+        case 'v': {
+            struct iovec *iov = va_arg(ap, struct iovec *);
+            int *iovcnt = va_arg(ap, int *);
+            *iovcnt = v9fs_copy_sg(out_sg, out_num, offset, iov);
+            break;
+        }
+        case 's': {
+            V9fsString *str = va_arg(ap, V9fsString *);
+            offset += v9fs_unmarshal(out_sg, out_num, offset, bswap,
+                            "w", &str->size);
+            /* FIXME: sanity check str->size */
+            str->data = g_malloc(str->size + 1);
+            offset += v9fs_unpack(str->data, out_sg, out_num, offset,
+                            str->size);
+            str->data[str->size] = 0;
+            break;
+        }
+        case 'Q': {
+            V9fsQID *qidp = va_arg(ap, V9fsQID *);
+            offset += v9fs_unmarshal(out_sg, out_num, offset, bswap, "bdq",
+                                     &qidp->type, &qidp->version, &qidp->path);
+            break;
+        }
+        case 'S': {
+            V9fsStat *statp = va_arg(ap, V9fsStat *);
+            offset += v9fs_unmarshal(out_sg, out_num, offset, bswap,
+                                    "wwdQdddqsssssddd",
+                                     &statp->size, &statp->type, &statp->dev,
+                                     &statp->qid, &statp->mode, &statp->atime,
+                                     &statp->mtime, &statp->length,
+                                     &statp->name, &statp->uid, &statp->gid,
+                                     &statp->muid, &statp->extension,
+                                     &statp->n_uid, &statp->n_gid,
+                                     &statp->n_muid);
+            break;
+        }
+        case 'I': {
+            V9fsIattr *iattr = va_arg(ap, V9fsIattr *);
+            offset += v9fs_unmarshal(out_sg, out_num, offset, bswap,
+                                     "ddddqqqqq",
+                                     &iattr->valid, &iattr->mode,
+                                     &iattr->uid, &iattr->gid, &iattr->size,
+                                     &iattr->atime_sec, &iattr->atime_nsec,
+                                     &iattr->mtime_sec, &iattr->mtime_nsec);
+            break;
+        }
+        default:
+            break;
+        }
+    }
+
+    va_end(ap);
+
+    return offset - old_offset;
+}
+
+size_t v9fs_marshal(struct iovec *in_sg, int in_num, size_t offset,
+                    int bswap, const char *fmt, ...)
+{
+    int i;
+    va_list ap;
+    size_t old_offset = offset;
+
+    va_start(ap, fmt);
+    for (i = 0; fmt[i]; i++) {
+        switch (fmt[i]) {
+        case 'b': {
+            uint8_t val = va_arg(ap, int);
+            offset += v9fs_pack(in_sg, in_num, offset, &val, sizeof(val));
+            break;
+        }
+        case 'w': {
+            uint16_t val;
+            if (bswap) {
+                cpu_to_le16w(&val, va_arg(ap, int));
+            } else {
+                val =  va_arg(ap, int);
+            }
+            offset += v9fs_pack(in_sg, in_num, offset, &val, sizeof(val));
+            break;
+        }
+        case 'd': {
+            uint32_t val;
+            if (bswap) {
+                cpu_to_le32w(&val, va_arg(ap, uint32_t));
+            } else {
+                val =  va_arg(ap, uint32_t);
+            }
+            offset += v9fs_pack(in_sg, in_num, offset, &val, sizeof(val));
+            break;
+        }
+        case 'q': {
+            uint64_t val;
+            if (bswap) {
+                cpu_to_le64w(&val, va_arg(ap, uint64_t));
+            } else {
+                val =  va_arg(ap, uint64_t);
+            }
+            offset += v9fs_pack(in_sg, in_num, offset, &val, sizeof(val));
+            break;
+        }
+        case 'v': {
+            struct iovec *iov = va_arg(ap, struct iovec *);
+            int *iovcnt = va_arg(ap, int *);
+            *iovcnt = v9fs_copy_sg(in_sg, in_num, offset, iov);
+            break;
+        }
+        case 's': {
+            V9fsString *str = va_arg(ap, V9fsString *);
+            offset += v9fs_marshal(in_sg, in_num, offset, bswap,
+                            "w", str->size);
+            offset += v9fs_pack(in_sg, in_num, offset, str->data, str->size);
+            break;
+        }
+        case 'Q': {
+            V9fsQID *qidp = va_arg(ap, V9fsQID *);
+            offset += v9fs_marshal(in_sg, in_num, offset, bswap, "bdq",
+                                   qidp->type, qidp->version, qidp->path);
+            break;
+        }
+        case 'S': {
+            V9fsStat *statp = va_arg(ap, V9fsStat *);
+            offset += v9fs_marshal(in_sg, in_num, offset, bswap,
+                                   "wwdQdddqsssssddd",
+                                   statp->size, statp->type, statp->dev,
+                                   &statp->qid, statp->mode, statp->atime,
+                                   statp->mtime, statp->length, &statp->name,
+                                   &statp->uid, &statp->gid, &statp->muid,
+                                   &statp->extension, statp->n_uid,
+                                   statp->n_gid, statp->n_muid);
+            break;
+        }
+        case 'A': {
+            V9fsStatDotl *statp = va_arg(ap, V9fsStatDotl *);
+            offset += v9fs_marshal(in_sg, in_num, offset, bswap,
+                                   "qQdddqqqqqqqqqqqqqqq",
+                                   statp->st_result_mask,
+                                   &statp->qid, statp->st_mode,
+                                   statp->st_uid, statp->st_gid,
+                                   statp->st_nlink, statp->st_rdev,
+                                   statp->st_size, statp->st_blksize,
+                                   statp->st_blocks, statp->st_atime_sec,
+                                   statp->st_atime_nsec, statp->st_mtime_sec,
+                                   statp->st_mtime_nsec, statp->st_ctime_sec,
+                                   statp->st_ctime_nsec, statp->st_btime_sec,
+                                   statp->st_btime_nsec, statp->st_gen,
+                                   statp->st_data_version);
+            break;
+        }
+        default:
+            break;
+        }
+    }
+    va_end(ap);
+
+    return offset - old_offset;
+}
diff --git a/fsdev/virtio-9p-marshal.h b/fsdev/virtio-9p-marshal.h
new file mode 100644
index 0000000..0960abf
--- /dev/null
+++ b/fsdev/virtio-9p-marshal.h
@@ -0,0 +1,87 @@
+#ifndef _QEMU_VIRTIO_9P_MARSHAL_H
+#define _QEMU_VIRTIO_9P_MARSHAL_H
+
+typedef struct V9fsString
+{
+    int16_t size;
+    char *data;
+} V9fsString;
+
+typedef struct V9fsQID
+{
+    int8_t type;
+    int32_t version;
+    int64_t path;
+} V9fsQID;
+
+typedef struct V9fsStat
+{
+    int16_t size;
+    int16_t type;
+    int32_t dev;
+    V9fsQID qid;
+    int32_t mode;
+    int32_t atime;
+    int32_t mtime;
+    int64_t length;
+    V9fsString name;
+    V9fsString uid;
+    V9fsString gid;
+    V9fsString muid;
+    /* 9p2000.u */
+    V9fsString extension;
+   int32_t n_uid;
+    int32_t n_gid;
+    int32_t n_muid;
+} V9fsStat;
+
+typedef struct V9fsIattr
+{
+    int32_t valid;
+    int32_t mode;
+    int32_t uid;
+    int32_t gid;
+    int64_t size;
+    int64_t atime_sec;
+    int64_t atime_nsec;
+    int64_t mtime_sec;
+    int64_t mtime_nsec;
+} V9fsIattr;
+
+typedef struct V9fsStatDotl {
+    uint64_t st_result_mask;
+    V9fsQID qid;
+    uint32_t st_mode;
+    uint32_t st_uid;
+    uint32_t st_gid;
+    uint64_t st_nlink;
+    uint64_t st_rdev;
+    uint64_t st_size;
+    uint64_t st_blksize;
+    uint64_t st_blocks;
+    uint64_t st_atime_sec;
+    uint64_t st_atime_nsec;
+    uint64_t st_mtime_sec;
+    uint64_t st_mtime_nsec;
+    uint64_t st_ctime_sec;
+    uint64_t st_ctime_nsec;
+    uint64_t st_btime_sec;
+    uint64_t st_btime_nsec;
+    uint64_t st_gen;
+    uint64_t st_data_version;
+} V9fsStatDotl;
+
+extern void v9fs_string_init(V9fsString *str);
+extern void v9fs_string_free(V9fsString *str);
+extern void v9fs_string_null(V9fsString *str);
+extern void v9fs_string_sprintf(V9fsString *str, const char *fmt, ...);
+extern void v9fs_string_copy(V9fsString *lhs, V9fsString *rhs);
+
+size_t v9fs_pack(struct iovec *in_sg, int in_num, size_t offset,
+                 const void *src, size_t size);
+size_t v9fs_unmarshal(struct iovec *out_sg, int out_num, size_t offset,
+                      int bswap, const char *fmt, ...);
+size_t v9fs_marshal(struct iovec *in_sg, int in_num, size_t offset,
+                    int bswap, const char *fmt, ...);
+
+#endif
diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c
index dd43209..1e02da0 100644
--- a/hw/9pfs/virtio-9p.c
+++ b/hw/9pfs/virtio-9p.c
@@ -11,9 +11,6 @@
  *
  */
 
-#include <glib.h>
-#include <glib/gprintf.h>
-
 #include "hw/virtio.h"
 #include "hw/pc.h"
 #include "qemu_socket.h"
@@ -138,42 +135,6 @@ static int get_dotl_openflags(V9fsState *s, int oflags)
     return flags;
 }
 
-void v9fs_string_init(V9fsString *str)
-{
-    str->data = NULL;
-    str->size = 0;
-}
-
-void v9fs_string_free(V9fsString *str)
-{
-    g_free(str->data);
-    str->data = NULL;
-    str->size = 0;
-}
-
-void v9fs_string_null(V9fsString *str)
-{
-    v9fs_string_free(str);
-}
-
-void GCC_FMT_ATTR(2, 3)
-v9fs_string_sprintf(V9fsString *str, const char *fmt, ...)
-{
-    va_list ap;
-
-    v9fs_string_free(str);
-
-    va_start(ap, fmt);
-    str->size = g_vasprintf(&str->data, fmt, ap);
-    va_end(ap);
-}
-
-void v9fs_string_copy(V9fsString *lhs, V9fsString *rhs)
-{
-    v9fs_string_free(lhs);
-    v9fs_string_sprintf(lhs, "%s", rhs->data);
-}
-
 void v9fs_path_init(V9fsPath *path)
 {
     path->data = NULL;
@@ -629,257 +590,6 @@ static void free_pdu(V9fsState *s, V9fsPDU *pdu)
     }
 }
 
-size_t pdu_packunpack(void *addr, struct iovec *sg, int sg_count,
-                        size_t offset, size_t size, int pack)
-{
-    int i = 0;
-    size_t copied = 0;
-
-    for (i = 0; size && i < sg_count; i++) {
-        size_t len;
-        if (offset >= sg[i].iov_len) {
-            /* skip this sg */
-            offset -= sg[i].iov_len;
-            continue;
-        } else {
-            len = MIN(sg[i].iov_len - offset, size);
-            if (pack) {
-                memcpy(sg[i].iov_base + offset, addr, len);
-            } else {
-                memcpy(addr, sg[i].iov_base + offset, len);
-            }
-            size -= len;
-            copied += len;
-            addr += len;
-            if (size) {
-                offset = 0;
-                continue;
-            }
-        }
-    }
-
-    return copied;
-}
-
-static size_t pdu_unpack(void *dst, V9fsPDU *pdu, size_t offset, size_t size)
-{
-    return pdu_packunpack(dst, pdu->elem.out_sg, pdu->elem.out_num,
-                         offset, size, 0);
-}
-
-static size_t pdu_pack(V9fsPDU *pdu, size_t offset, const void *src,
-                        size_t size)
-{
-    return pdu_packunpack((void *)src, pdu->elem.in_sg, pdu->elem.in_num,
-                             offset, size, 1);
-}
-
-static int pdu_copy_sg(V9fsPDU *pdu, size_t offset, int rx, struct iovec *sg)
-{
-    size_t pos = 0;
-    int i, j;
-    struct iovec *src_sg;
-    unsigned int num;
-
-    if (rx) {
-        src_sg = pdu->elem.in_sg;
-        num = pdu->elem.in_num;
-    } else {
-        src_sg = pdu->elem.out_sg;
-        num = pdu->elem.out_num;
-    }
-
-    j = 0;
-    for (i = 0; i < num; i++) {
-        if (offset <= pos) {
-            sg[j].iov_base = src_sg[i].iov_base;
-            sg[j].iov_len = src_sg[i].iov_len;
-            j++;
-        } else if (offset < (src_sg[i].iov_len + pos)) {
-            sg[j].iov_base = src_sg[i].iov_base;
-            sg[j].iov_len = src_sg[i].iov_len;
-            sg[j].iov_base += (offset - pos);
-            sg[j].iov_len -= (offset - pos);
-            j++;
-        }
-        pos += src_sg[i].iov_len;
-    }
-
-    return j;
-}
-
-static size_t pdu_unmarshal(V9fsPDU *pdu, size_t offset, const char *fmt, ...)
-{
-    size_t old_offset = offset;
-    va_list ap;
-    int i;
-
-    va_start(ap, fmt);
-    for (i = 0; fmt[i]; i++) {
-        switch (fmt[i]) {
-        case 'b': {
-            uint8_t *valp = va_arg(ap, uint8_t *);
-            offset += pdu_unpack(valp, pdu, offset, sizeof(*valp));
-            break;
-        }
-        case 'w': {
-            uint16_t val, *valp;
-            valp = va_arg(ap, uint16_t *);
-            offset += pdu_unpack(&val, pdu, offset, sizeof(val));
-            *valp = le16_to_cpu(val);
-            break;
-        }
-        case 'd': {
-            uint32_t val, *valp;
-            valp = va_arg(ap, uint32_t *);
-            offset += pdu_unpack(&val, pdu, offset, sizeof(val));
-            *valp = le32_to_cpu(val);
-            break;
-        }
-        case 'q': {
-            uint64_t val, *valp;
-            valp = va_arg(ap, uint64_t *);
-            offset += pdu_unpack(&val, pdu, offset, sizeof(val));
-            *valp = le64_to_cpu(val);
-            break;
-        }
-        case 'v': {
-            struct iovec *iov = va_arg(ap, struct iovec *);
-            int *iovcnt = va_arg(ap, int *);
-            *iovcnt = pdu_copy_sg(pdu, offset, 0, iov);
-            break;
-        }
-        case 's': {
-            V9fsString *str = va_arg(ap, V9fsString *);
-            offset += pdu_unmarshal(pdu, offset, "w", &str->size);
-            /* FIXME: sanity check str->size */
-            str->data = g_malloc(str->size + 1);
-            offset += pdu_unpack(str->data, pdu, offset, str->size);
-            str->data[str->size] = 0;
-            break;
-        }
-        case 'Q': {
-            V9fsQID *qidp = va_arg(ap, V9fsQID *);
-            offset += pdu_unmarshal(pdu, offset, "bdq",
-                        &qidp->type, &qidp->version, &qidp->path);
-            break;
-        }
-        case 'S': {
-            V9fsStat *statp = va_arg(ap, V9fsStat *);
-            offset += pdu_unmarshal(pdu, offset, "wwdQdddqsssssddd",
-                        &statp->size, &statp->type, &statp->dev,
-                        &statp->qid, &statp->mode, &statp->atime,
-                        &statp->mtime, &statp->length,
-                        &statp->name, &statp->uid, &statp->gid,
-                        &statp->muid, &statp->extension,
-                        &statp->n_uid, &statp->n_gid,
-                        &statp->n_muid);
-            break;
-        }
-        case 'I': {
-            V9fsIattr *iattr = va_arg(ap, V9fsIattr *);
-            offset += pdu_unmarshal(pdu, offset, "ddddqqqqq",
-                        &iattr->valid, &iattr->mode,
-                        &iattr->uid, &iattr->gid, &iattr->size,
-                        &iattr->atime_sec, &iattr->atime_nsec,
-                        &iattr->mtime_sec, &iattr->mtime_nsec);
-            break;
-        }
-        default:
-            break;
-        }
-    }
-
-    va_end(ap);
-
-    return offset - old_offset;
-}
-
-static size_t pdu_marshal(V9fsPDU *pdu, size_t offset, const char *fmt, ...)
-{
-    size_t old_offset = offset;
-    va_list ap;
-    int i;
-
-    va_start(ap, fmt);
-    for (i = 0; fmt[i]; i++) {
-        switch (fmt[i]) {
-        case 'b': {
-            uint8_t val = va_arg(ap, int);
-            offset += pdu_pack(pdu, offset, &val, sizeof(val));
-            break;
-        }
-        case 'w': {
-            uint16_t val;
-            cpu_to_le16w(&val, va_arg(ap, int));
-            offset += pdu_pack(pdu, offset, &val, sizeof(val));
-            break;
-        }
-        case 'd': {
-            uint32_t val;
-            cpu_to_le32w(&val, va_arg(ap, uint32_t));
-            offset += pdu_pack(pdu, offset, &val, sizeof(val));
-            break;
-        }
-        case 'q': {
-            uint64_t val;
-            cpu_to_le64w(&val, va_arg(ap, uint64_t));
-            offset += pdu_pack(pdu, offset, &val, sizeof(val));
-            break;
-        }
-        case 'v': {
-            struct iovec *iov = va_arg(ap, struct iovec *);
-            int *iovcnt = va_arg(ap, int *);
-            *iovcnt = pdu_copy_sg(pdu, offset, 1, iov);
-            break;
-        }
-        case 's': {
-            V9fsString *str = va_arg(ap, V9fsString *);
-            offset += pdu_marshal(pdu, offset, "w", str->size);
-            offset += pdu_pack(pdu, offset, str->data, str->size);
-            break;
-        }
-        case 'Q': {
-            V9fsQID *qidp = va_arg(ap, V9fsQID *);
-            offset += pdu_marshal(pdu, offset, "bdq",
-                        qidp->type, qidp->version, qidp->path);
-            break;
-        }
-        case 'S': {
-            V9fsStat *statp = va_arg(ap, V9fsStat *);
-            offset += pdu_marshal(pdu, offset, "wwdQdddqsssssddd",
-                        statp->size, statp->type, statp->dev,
-                        &statp->qid, statp->mode, statp->atime,
-                        statp->mtime, statp->length, &statp->name,
-                        &statp->uid, &statp->gid, &statp->muid,
-                        &statp->extension, statp->n_uid,
-                        statp->n_gid, statp->n_muid);
-            break;
-        }
-        case 'A': {
-            V9fsStatDotl *statp = va_arg(ap, V9fsStatDotl *);
-            offset += pdu_marshal(pdu, offset, "qQdddqqqqqqqqqqqqqqq",
-                        statp->st_result_mask,
-                        &statp->qid, statp->st_mode,
-                        statp->st_uid, statp->st_gid,
-                        statp->st_nlink, statp->st_rdev,
-                        statp->st_size, statp->st_blksize, statp->st_blocks,
-                        statp->st_atime_sec, statp->st_atime_nsec,
-                        statp->st_mtime_sec, statp->st_mtime_nsec,
-                        statp->st_ctime_sec, statp->st_ctime_nsec,
-                        statp->st_btime_sec, statp->st_btime_nsec,
-                        statp->st_gen, statp->st_data_version);
-            break;
-        }
-        default:
-            break;
-        }
-    }
-    va_end(ap);
-
-    return offset - old_offset;
-}
-
 static void complete_pdu(V9fsState *s, V9fsPDU *pdu, ssize_t len)
 {
     int8_t id = pdu->id + 1; /* Response */
@@ -1794,9 +1504,10 @@ static int v9fs_xattr_read(V9fsState *s, V9fsPDU *pdu,
         read_count = 0;
     }
     offset += pdu_marshal(pdu, offset, "d", read_count);
-    offset += pdu_pack(pdu, offset,
-                       ((char *)fidp->fs.xattr.value) + off,
-                       read_count);
+    offset += v9fs_pack(pdu->elem.in_sg, pdu->elem.in_num, offset,
+                        ((char *)fidp->fs.xattr.value) + off,
+                        read_count);
+
     return offset;
 }
 
diff --git a/hw/9pfs/virtio-9p.h b/hw/9pfs/virtio-9p.h
index 8b612da..5797944 100644
--- a/hw/9pfs/virtio-9p.h
+++ b/hw/9pfs/virtio-9p.h
@@ -8,9 +8,11 @@
 #include <sys/resource.h>
 #include "hw/virtio.h"
 #include "fsdev/file-op-9p.h"
+#include "fsdev/virtio-9p-marshal.h"
 #include "qemu-thread.h"
 #include "qemu-coroutine.h"
 
+
 /* The feature bitmap for virtio 9P */
 /* The mount point is specified in a config variable */
 #define VIRTIO_9P_MOUNT_TAG 0
@@ -154,40 +156,6 @@ struct V9fsPDU
 
 typedef struct V9fsFidState V9fsFidState;
 
-typedef struct V9fsString
-{
-    int16_t size;
-    char *data;
-} V9fsString;
-
-typedef struct V9fsQID
-{
-    int8_t type;
-    int32_t version;
-    int64_t path;
-} V9fsQID;
-
-typedef struct V9fsStat
-{
-    int16_t size;
-    int16_t type;
-    int32_t dev;
-    V9fsQID qid;
-    int32_t mode;
-    int32_t atime;
-    int32_t mtime;
-    int64_t length;
-    V9fsString name;
-    V9fsString uid;
-    V9fsString gid;
-    V9fsString muid;
-    /* 9p2000.u */
-    V9fsString extension;
-   int32_t n_uid;
-    int32_t n_gid;
-    int32_t n_muid;
-} V9fsStat;
-
 enum {
     P9_FID_NONE = 0,
     P9_FID_FILE,
@@ -267,29 +235,6 @@ typedef struct V9fsStatState {
     struct stat stbuf;
 } V9fsStatState;
 
-typedef struct V9fsStatDotl {
-    uint64_t st_result_mask;
-    V9fsQID qid;
-    uint32_t st_mode;
-    uint32_t st_uid;
-    uint32_t st_gid;
-    uint64_t st_nlink;
-    uint64_t st_rdev;
-    uint64_t st_size;
-    uint64_t st_blksize;
-    uint64_t st_blocks;
-    uint64_t st_atime_sec;
-    uint64_t st_atime_nsec;
-    uint64_t st_mtime_sec;
-    uint64_t st_mtime_nsec;
-    uint64_t st_ctime_sec;
-    uint64_t st_ctime_nsec;
-    uint64_t st_btime_sec;
-    uint64_t st_btime_nsec;
-    uint64_t st_gen;
-    uint64_t st_data_version;
-} V9fsStatDotl;
-
 typedef struct V9fsOpenState {
     V9fsPDU *pdu;
     size_t offset;
@@ -332,19 +277,6 @@ typedef struct V9fsWriteState {
     int cnt;
 } V9fsWriteState;
 
-typedef struct V9fsIattr
-{
-    int32_t valid;
-    int32_t mode;
-    int32_t uid;
-    int32_t gid;
-    int64_t size;
-    int64_t atime_sec;
-    int64_t atime_nsec;
-    int64_t mtime_sec;
-    int64_t mtime_nsec;
-} V9fsIattr;
-
 struct virtio_9p_config
 {
     /* number of characters in tag */
@@ -459,14 +391,15 @@ static inline uint8_t v9fs_request_cancelled(V9fsPDU *pdu)
 extern void handle_9p_output(VirtIODevice *vdev, VirtQueue *vq);
 extern void virtio_9p_set_fd_limit(void);
 extern void v9fs_reclaim_fd(V9fsPDU *pdu);
-extern void v9fs_string_init(V9fsString *str);
-extern void v9fs_string_free(V9fsString *str);
-extern void v9fs_string_null(V9fsString *str);
-extern void v9fs_string_sprintf(V9fsString *str, const char *fmt, ...);
-extern void v9fs_string_copy(V9fsString *lhs, V9fsString *rhs);
 extern void v9fs_path_init(V9fsPath *path);
 extern void v9fs_path_free(V9fsPath *path);
 extern void v9fs_path_copy(V9fsPath *lhs, V9fsPath *rhs);
 extern int v9fs_name_to_path(V9fsState *s, V9fsPath *dirpath,
                              const char *name, V9fsPath *path);
+
+#define pdu_marshal(pdu, offset, fmt, args...)  \
+    v9fs_marshal(pdu->elem.in_sg, pdu->elem.in_num, offset, 1, fmt, ##args)
+#define pdu_unmarshal(pdu, offset, fmt, args...)  \
+    v9fs_unmarshal(pdu->elem.out_sg, pdu->elem.out_num, offset, 1, fmt, ##args)
+
 #endif
-- 
1.7.6

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [Qemu-devel] [PATCH V4 02/13] hw/9pfs: Add validation to {un}marshal code
  2011-12-05 16:18 [Qemu-devel] [PATCH V4 00/13] Proxy FS driver for VirtFS M. Mohan Kumar
  2011-12-05 16:18 ` [Qemu-devel] [PATCH V4 01/13] hw/9pfs: Move pdu_marshal/unmarshal code to a seperate file M. Mohan Kumar
@ 2011-12-05 16:18 ` M. Mohan Kumar
  2011-12-08 18:10   ` Stefan Hajnoczi
  2011-12-05 16:18 ` [Qemu-devel] [PATCH V4 03/13] hw/9pfs: Add new proxy filesystem driver M. Mohan Kumar
                   ` (11 subsequent siblings)
  13 siblings, 1 reply; 22+ messages in thread
From: M. Mohan Kumar @ 2011-12-05 16:18 UTC (permalink / raw)
  To: qemu-devel, aneesh.kumar, stefanha; +Cc: M. Mohan Kumar

From: "M. Mohan Kumar" <mohan@in.ibm.com>

Signed-off-by: M. Mohan Kumar <mohan@in.ibm.com>
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
 fsdev/virtio-9p-marshal.c |  148 ++++++++------
 fsdev/virtio-9p-marshal.h |   19 +-
 hw/9pfs/virtio-9p.c       |  467 +++++++++++++++++++++++++++++++++------------
 3 files changed, 438 insertions(+), 196 deletions(-)

diff --git a/fsdev/virtio-9p-marshal.c b/fsdev/virtio-9p-marshal.c
index 029c29e..198665d 100644
--- a/fsdev/virtio-9p-marshal.c
+++ b/fsdev/virtio-9p-marshal.c
@@ -20,17 +20,12 @@
 #include <sys/uio.h>
 #include <string.h>
 #include <stdint.h>
+#include <errno.h>
 
 #include "compiler.h"
 #include "virtio-9p-marshal.h"
 #include "bswap.h"
 
-void v9fs_string_init(V9fsString *str)
-{
-    str->data = NULL;
-    str->size = 0;
-}
-
 void v9fs_string_free(V9fsString *str)
 {
     g_free(str->data);
@@ -62,11 +57,13 @@ void v9fs_string_copy(V9fsString *lhs, V9fsString *rhs)
 }
 
 
-static size_t v9fs_packunpack(void *addr, struct iovec *sg, int sg_count,
-                              size_t offset, size_t size, int pack)
+static ssize_t v9fs_packunpack(void *addr, struct iovec *sg, int sg_count,
+                               size_t offset, size_t size, int pack)
 {
     int i = 0;
     size_t copied = 0;
+    size_t req_size = size;
+
 
     for (i = 0; size && i < sg_count; i++) {
         size_t len;
@@ -90,18 +87,23 @@ static size_t v9fs_packunpack(void *addr, struct iovec *sg, int sg_count,
             }
         }
     }
-
+    if (copied < req_size) {
+        /*
+         * We copied less that requested size. error out
+         */
+        return -ENOBUFS;
+    }
     return copied;
 }
 
-static size_t v9fs_unpack(void *dst, struct iovec *out_sg, int out_num,
-                          size_t offset, size_t size)
+static ssize_t v9fs_unpack(void *dst, struct iovec *out_sg, int out_num,
+                           size_t offset, size_t size)
 {
     return v9fs_packunpack(dst, out_sg, out_num, offset, size, 0);
 }
 
-size_t v9fs_pack(struct iovec *in_sg, int in_num, size_t offset,
-                const void *src, size_t size)
+ssize_t v9fs_pack(struct iovec *in_sg, int in_num, size_t offset,
+                  const void *src, size_t size)
 {
     return v9fs_packunpack((void *)src, in_sg, in_num, offset, size, 1);
 }
@@ -131,11 +133,12 @@ static int v9fs_copy_sg(struct iovec *src_sg, unsigned int num,
     return j;
 }
 
-size_t v9fs_unmarshal(struct iovec *out_sg, int out_num, size_t offset,
-                      int bswap, const char *fmt, ...)
+ssize_t v9fs_unmarshal(struct iovec *out_sg, int out_num, size_t offset,
+                       int bswap, const char *fmt, ...)
 {
     int i;
     va_list ap;
+    ssize_t copied;
     size_t old_offset = offset;
 
     va_start(ap, fmt);
@@ -143,13 +146,13 @@ size_t v9fs_unmarshal(struct iovec *out_sg, int out_num, size_t offset,
         switch (fmt[i]) {
         case 'b': {
             uint8_t *valp = va_arg(ap, uint8_t *);
-            offset += v9fs_unpack(valp, out_sg, out_num, offset, sizeof(*valp));
+            copied = v9fs_unpack(valp, out_sg, out_num, offset, sizeof(*valp));
             break;
         }
         case 'w': {
             uint16_t val, *valp;
             valp = va_arg(ap, uint16_t *);
-            offset += v9fs_unpack(&val, out_sg, out_num, offset, sizeof(val));
+            copied = v9fs_unpack(&val, out_sg, out_num, offset, sizeof(val));
             if (bswap) {
                 *valp = le16_to_cpu(val);
             } else {
@@ -160,7 +163,7 @@ size_t v9fs_unmarshal(struct iovec *out_sg, int out_num, size_t offset,
         case 'd': {
             uint32_t val, *valp;
             valp = va_arg(ap, uint32_t *);
-            offset += v9fs_unpack(&val, out_sg, out_num, offset, sizeof(val));
+            copied = v9fs_unpack(&val, out_sg, out_num, offset, sizeof(val));
             if (bswap) {
                 *valp = le32_to_cpu(val);
             } else {
@@ -171,7 +174,7 @@ size_t v9fs_unmarshal(struct iovec *out_sg, int out_num, size_t offset,
         case 'q': {
             uint64_t val, *valp;
             valp = va_arg(ap, uint64_t *);
-            offset += v9fs_unpack(&val, out_sg, out_num, offset, sizeof(val));
+            copied = v9fs_unpack(&val, out_sg, out_num, offset, sizeof(val));
             if (bswap) {
                 *valp = le64_to_cpu(val);
             } else {
@@ -187,59 +190,70 @@ size_t v9fs_unmarshal(struct iovec *out_sg, int out_num, size_t offset,
         }
         case 's': {
             V9fsString *str = va_arg(ap, V9fsString *);
-            offset += v9fs_unmarshal(out_sg, out_num, offset, bswap,
-                            "w", &str->size);
-            /* FIXME: sanity check str->size */
-            str->data = g_malloc(str->size + 1);
-            offset += v9fs_unpack(str->data, out_sg, out_num, offset,
-                            str->size);
-            str->data[str->size] = 0;
+            copied = v9fs_unmarshal(out_sg, out_num, offset, bswap,
+                                    "w", &str->size);
+            if (copied > 0) {
+                offset += copied;
+                str->data = g_malloc(str->size + 1);
+                copied = v9fs_unpack(str->data, out_sg, out_num, offset,
+                                     str->size);
+                if (copied > 0) {
+                    str->data[str->size] = 0;
+                } else {
+                    v9fs_string_free(str);
+                }
+            }
             break;
         }
         case 'Q': {
             V9fsQID *qidp = va_arg(ap, V9fsQID *);
-            offset += v9fs_unmarshal(out_sg, out_num, offset, bswap, "bdq",
-                                     &qidp->type, &qidp->version, &qidp->path);
+            copied = v9fs_unmarshal(out_sg, out_num, offset, bswap, "bdq",
+                                    &qidp->type, &qidp->version, &qidp->path);
             break;
         }
         case 'S': {
             V9fsStat *statp = va_arg(ap, V9fsStat *);
-            offset += v9fs_unmarshal(out_sg, out_num, offset, bswap,
+            copied = v9fs_unmarshal(out_sg, out_num, offset, bswap,
                                     "wwdQdddqsssssddd",
-                                     &statp->size, &statp->type, &statp->dev,
-                                     &statp->qid, &statp->mode, &statp->atime,
-                                     &statp->mtime, &statp->length,
-                                     &statp->name, &statp->uid, &statp->gid,
-                                     &statp->muid, &statp->extension,
-                                     &statp->n_uid, &statp->n_gid,
-                                     &statp->n_muid);
+                                    &statp->size, &statp->type, &statp->dev,
+                                    &statp->qid, &statp->mode, &statp->atime,
+                                    &statp->mtime, &statp->length,
+                                    &statp->name, &statp->uid, &statp->gid,
+                                    &statp->muid, &statp->extension,
+                                    &statp->n_uid, &statp->n_gid,
+                                    &statp->n_muid);
             break;
         }
         case 'I': {
             V9fsIattr *iattr = va_arg(ap, V9fsIattr *);
-            offset += v9fs_unmarshal(out_sg, out_num, offset, bswap,
-                                     "ddddqqqqq",
-                                     &iattr->valid, &iattr->mode,
-                                     &iattr->uid, &iattr->gid, &iattr->size,
-                                     &iattr->atime_sec, &iattr->atime_nsec,
-                                     &iattr->mtime_sec, &iattr->mtime_nsec);
+            copied = v9fs_unmarshal(out_sg, out_num, offset, bswap,
+                                    "ddddqqqqq",
+                                    &iattr->valid, &iattr->mode,
+                                    &iattr->uid, &iattr->gid, &iattr->size,
+                                    &iattr->atime_sec, &iattr->atime_nsec,
+                                    &iattr->mtime_sec, &iattr->mtime_nsec);
             break;
         }
         default:
             break;
         }
+        if (copied < 0) {
+            va_end(ap);
+            return copied;
+        }
+        offset += copied;
     }
-
     va_end(ap);
 
     return offset - old_offset;
 }
 
-size_t v9fs_marshal(struct iovec *in_sg, int in_num, size_t offset,
-                    int bswap, const char *fmt, ...)
+ssize_t v9fs_marshal(struct iovec *in_sg, int in_num, size_t offset,
+                     int bswap, const char *fmt, ...)
 {
     int i;
     va_list ap;
+    ssize_t copied;
     size_t old_offset = offset;
 
     va_start(ap, fmt);
@@ -247,7 +261,7 @@ size_t v9fs_marshal(struct iovec *in_sg, int in_num, size_t offset,
         switch (fmt[i]) {
         case 'b': {
             uint8_t val = va_arg(ap, int);
-            offset += v9fs_pack(in_sg, in_num, offset, &val, sizeof(val));
+            copied = v9fs_pack(in_sg, in_num, offset, &val, sizeof(val));
             break;
         }
         case 'w': {
@@ -257,7 +271,7 @@ size_t v9fs_marshal(struct iovec *in_sg, int in_num, size_t offset,
             } else {
                 val =  va_arg(ap, int);
             }
-            offset += v9fs_pack(in_sg, in_num, offset, &val, sizeof(val));
+            copied = v9fs_pack(in_sg, in_num, offset, &val, sizeof(val));
             break;
         }
         case 'd': {
@@ -267,7 +281,7 @@ size_t v9fs_marshal(struct iovec *in_sg, int in_num, size_t offset,
             } else {
                 val =  va_arg(ap, uint32_t);
             }
-            offset += v9fs_pack(in_sg, in_num, offset, &val, sizeof(val));
+            copied = v9fs_pack(in_sg, in_num, offset, &val, sizeof(val));
             break;
         }
         case 'q': {
@@ -277,7 +291,7 @@ size_t v9fs_marshal(struct iovec *in_sg, int in_num, size_t offset,
             } else {
                 val =  va_arg(ap, uint64_t);
             }
-            offset += v9fs_pack(in_sg, in_num, offset, &val, sizeof(val));
+            copied = v9fs_pack(in_sg, in_num, offset, &val, sizeof(val));
             break;
         }
         case 'v': {
@@ -288,32 +302,35 @@ size_t v9fs_marshal(struct iovec *in_sg, int in_num, size_t offset,
         }
         case 's': {
             V9fsString *str = va_arg(ap, V9fsString *);
-            offset += v9fs_marshal(in_sg, in_num, offset, bswap,
-                            "w", str->size);
-            offset += v9fs_pack(in_sg, in_num, offset, str->data, str->size);
+            copied = v9fs_marshal(in_sg, in_num, offset, bswap,
+                                  "w", str->size);
+            if (copied > 0) {
+                offset += copied;
+                copied = v9fs_pack(in_sg, in_num, offset, str->data, str->size);
+            }
             break;
         }
         case 'Q': {
             V9fsQID *qidp = va_arg(ap, V9fsQID *);
-            offset += v9fs_marshal(in_sg, in_num, offset, bswap, "bdq",
-                                   qidp->type, qidp->version, qidp->path);
+            copied = v9fs_marshal(in_sg, in_num, offset, bswap, "bdq",
+                                  qidp->type, qidp->version, qidp->path);
             break;
         }
         case 'S': {
             V9fsStat *statp = va_arg(ap, V9fsStat *);
-            offset += v9fs_marshal(in_sg, in_num, offset, bswap,
-                                   "wwdQdddqsssssddd",
-                                   statp->size, statp->type, statp->dev,
-                                   &statp->qid, statp->mode, statp->atime,
-                                   statp->mtime, statp->length, &statp->name,
-                                   &statp->uid, &statp->gid, &statp->muid,
-                                   &statp->extension, statp->n_uid,
-                                   statp->n_gid, statp->n_muid);
+            copied = v9fs_marshal(in_sg, in_num, offset, bswap,
+                                  "wwdQdddqsssssddd",
+                                  statp->size, statp->type, statp->dev,
+                                  &statp->qid, statp->mode, statp->atime,
+                                  statp->mtime, statp->length, &statp->name,
+                                  &statp->uid, &statp->gid, &statp->muid,
+                                  &statp->extension, statp->n_uid,
+                                  statp->n_gid, statp->n_muid);
             break;
         }
         case 'A': {
             V9fsStatDotl *statp = va_arg(ap, V9fsStatDotl *);
-            offset += v9fs_marshal(in_sg, in_num, offset, bswap,
+            copied = v9fs_marshal(in_sg, in_num, offset, bswap,
                                    "qQdddqqqqqqqqqqqqqqq",
                                    statp->st_result_mask,
                                    &statp->qid, statp->st_mode,
@@ -331,6 +348,11 @@ size_t v9fs_marshal(struct iovec *in_sg, int in_num, size_t offset,
         default:
             break;
         }
+        if (copied < 0) {
+            va_end(ap);
+            return copied;
+        }
+        offset += copied;
     }
     va_end(ap);
 
diff --git a/fsdev/virtio-9p-marshal.h b/fsdev/virtio-9p-marshal.h
index 0960abf..7e86817 100644
--- a/fsdev/virtio-9p-marshal.h
+++ b/fsdev/virtio-9p-marshal.h
@@ -71,17 +71,20 @@ typedef struct V9fsStatDotl {
     uint64_t st_data_version;
 } V9fsStatDotl;
 
-extern void v9fs_string_init(V9fsString *str);
+static inline void v9fs_string_init(V9fsString *str)
+{
+    str->data = NULL;
+    str->size = 0;
+}
 extern void v9fs_string_free(V9fsString *str);
 extern void v9fs_string_null(V9fsString *str);
 extern void v9fs_string_sprintf(V9fsString *str, const char *fmt, ...);
 extern void v9fs_string_copy(V9fsString *lhs, V9fsString *rhs);
 
-size_t v9fs_pack(struct iovec *in_sg, int in_num, size_t offset,
-                 const void *src, size_t size);
-size_t v9fs_unmarshal(struct iovec *out_sg, int out_num, size_t offset,
-                      int bswap, const char *fmt, ...);
-size_t v9fs_marshal(struct iovec *in_sg, int in_num, size_t offset,
-                    int bswap, const char *fmt, ...);
-
+ssize_t v9fs_pack(struct iovec *in_sg, int in_num, size_t offset,
+                  const void *src, size_t size);
+ssize_t v9fs_unmarshal(struct iovec *out_sg, int out_num, size_t offset,
+                       int bswap, const char *fmt, ...);
+ssize_t v9fs_marshal(struct iovec *in_sg, int in_num, size_t offset,
+                     int bswap, const char *fmt, ...);
 #endif
diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c
index 1e02da0..f7844b4 100644
--- a/hw/9pfs/virtio-9p.c
+++ b/hw/9pfs/virtio-9p.c
@@ -590,6 +590,11 @@ static void free_pdu(V9fsState *s, V9fsPDU *pdu)
     }
 }
 
+/*
+ * We don't do error checking for pdu_marshal/unmarshal here
+ * because we always expect to have enough space to encode
+ * error details
+ */
 static void complete_pdu(V9fsState *s, V9fsPDU *pdu, ssize_t len)
 {
     int8_t id = pdu->id + 1; /* Response */
@@ -702,6 +707,15 @@ static int donttouch_stat(V9fsStat *stat)
     return 0;
 }
 
+static void v9fs_stat_init(V9fsStat *stat)
+{
+    v9fs_string_init(&stat->name);
+    v9fs_string_init(&stat->uid);
+    v9fs_string_init(&stat->gid);
+    v9fs_string_init(&stat->muid);
+    v9fs_string_init(&stat->extension);
+}
+
 static void v9fs_stat_free(V9fsStat *stat)
 {
     v9fs_string_free(&stat->name);
@@ -922,12 +936,18 @@ static inline bool is_ro_export(FsContext *ctx)
 
 static void v9fs_version(void *opaque)
 {
+    ssize_t err;
     V9fsPDU *pdu = opaque;
     V9fsState *s = pdu->s;
     V9fsString version;
     size_t offset = 7;
 
-    pdu_unmarshal(pdu, offset, "ds", &s->msize, &version);
+    v9fs_string_init(&version);
+    err = pdu_unmarshal(pdu, offset, "ds", &s->msize, &version);
+    if (err < 0) {
+        offset = err;
+        goto out;
+    }
     trace_v9fs_version(pdu->tag, pdu->id, s->msize, version.data);
 
     virtfs_reset(pdu);
@@ -940,11 +960,15 @@ static void v9fs_version(void *opaque)
         v9fs_string_sprintf(&version, "unknown");
     }
 
-    offset += pdu_marshal(pdu, offset, "ds", s->msize, &version);
+    err = pdu_marshal(pdu, offset, "ds", s->msize, &version);
+    if (err < 0) {
+        offset = err;
+        goto out;
+    }
+    offset += err;
     trace_v9fs_version_return(pdu->tag, pdu->id, s->msize, version.data);
-
+out:
     complete_pdu(s, pdu, offset);
-
     v9fs_string_free(&version);
     return;
 }
@@ -960,7 +984,13 @@ static void v9fs_attach(void *opaque)
     V9fsQID qid;
     ssize_t err;
 
-    pdu_unmarshal(pdu, offset, "ddssd", &fid, &afid, &uname, &aname, &n_uname);
+    v9fs_string_init(&uname);
+    v9fs_string_init(&aname);
+    err = pdu_unmarshal(pdu, offset, "ddssd", &fid,
+                        &afid, &uname, &aname, &n_uname);
+    if (err < 0) {
+        goto out_nofid;
+    }
     trace_v9fs_attach(pdu->tag, pdu->id, fid, afid, uname.data, aname.data);
 
     fidp = alloc_fid(s, fid);
@@ -981,8 +1011,12 @@ static void v9fs_attach(void *opaque)
         clunk_fid(s, fid);
         goto out;
     }
-    offset += pdu_marshal(pdu, offset, "Q", &qid);
-    err = offset;
+    err = pdu_marshal(pdu, offset, "Q", &qid);
+    if (err < 0) {
+        clunk_fid(s, fid);
+        goto out;
+    }
+    err += offset;
     trace_v9fs_attach_return(pdu->tag, pdu->id,
                              qid.type, qid.version, qid.path);
     s->root_fid = fid;
@@ -1009,7 +1043,10 @@ static void v9fs_stat(void *opaque)
     V9fsPDU *pdu = opaque;
     V9fsState *s = pdu->s;
 
-    pdu_unmarshal(pdu, offset, "d", &fid);
+    err = pdu_unmarshal(pdu, offset, "d", &fid);
+    if (err < 0) {
+        goto out_nofid;
+    }
     trace_v9fs_stat(pdu->tag, pdu->id, fid);
 
     fidp = get_fid(pdu, fid);
@@ -1025,10 +1062,14 @@ static void v9fs_stat(void *opaque)
     if (err < 0) {
         goto out;
     }
-    offset += pdu_marshal(pdu, offset, "wS", 0, &v9stat);
-    err = offset;
+    err = pdu_marshal(pdu, offset, "wS", 0, &v9stat);
+    if (err < 0) {
+        v9fs_stat_free(&v9stat);
+        goto out;
+    }
     trace_v9fs_stat_return(pdu->tag, pdu->id, v9stat.mode,
                            v9stat.atime, v9stat.mtime, v9stat.length);
+    err += offset;
     v9fs_stat_free(&v9stat);
 out:
     put_fid(pdu, fidp);
@@ -1048,7 +1089,10 @@ static void v9fs_getattr(void *opaque)
     V9fsPDU *pdu = opaque;
     V9fsState *s = pdu->s;
 
-    pdu_unmarshal(pdu, offset, "dq", &fid, &request_mask);
+    retval = pdu_unmarshal(pdu, offset, "dq", &fid, &request_mask);
+    if (retval < 0) {
+        goto out_nofid;
+    }
     trace_v9fs_getattr(pdu->tag, pdu->id, fid, request_mask);
 
     fidp = get_fid(pdu, fid);
@@ -1074,8 +1118,11 @@ static void v9fs_getattr(void *opaque)
         }
         v9stat_dotl.st_result_mask |= P9_STATS_GEN;
     }
-    retval = offset;
-    retval += pdu_marshal(pdu, offset, "A", &v9stat_dotl);
+    retval = pdu_marshal(pdu, offset, "A", &v9stat_dotl);
+    if (retval < 0) {
+        goto out;
+    }
+    retval += offset;
     trace_v9fs_getattr_return(pdu->tag, pdu->id, v9stat_dotl.st_result_mask,
                               v9stat_dotl.st_mode, v9stat_dotl.st_uid,
                               v9stat_dotl.st_gid);
@@ -1107,7 +1154,10 @@ static void v9fs_setattr(void *opaque)
     V9fsPDU *pdu = opaque;
     V9fsState *s = pdu->s;
 
-    pdu_unmarshal(pdu, offset, "dI", &fid, &v9iattr);
+    err = pdu_unmarshal(pdu, offset, "dI", &fid, &v9iattr);
+    if (err < 0) {
+        goto out_nofid;
+    }
 
     fidp = get_fid(pdu, fid);
     if (fidp == NULL) {
@@ -1182,10 +1232,20 @@ out_nofid:
 static int v9fs_walk_marshal(V9fsPDU *pdu, uint16_t nwnames, V9fsQID *qids)
 {
     int i;
+    ssize_t err;
     size_t offset = 7;
-    offset += pdu_marshal(pdu, offset, "w", nwnames);
+
+    err = pdu_marshal(pdu, offset, "w", nwnames);
+    if (err < 0) {
+        return err;
+    }
+    offset += err;
     for (i = 0; i < nwnames; i++) {
-        offset += pdu_marshal(pdu, offset, "Q", &qids[i]);
+        err = pdu_marshal(pdu, offset, "Q", &qids[i]);
+        if (err < 0) {
+            return err;
+        }
+        offset += err;
     }
     return offset;
 }
@@ -1206,8 +1266,12 @@ static void v9fs_walk(void *opaque)
     V9fsPDU *pdu = opaque;
     V9fsState *s = pdu->s;
 
-    offset += pdu_unmarshal(pdu, offset, "ddw", &fid,
-                            &newfid, &nwnames);
+    err = pdu_unmarshal(pdu, offset, "ddw", &fid, &newfid, &nwnames);
+    if (err < 0) {
+        complete_pdu(s, pdu, err);
+        return ;
+    }
+    offset += err;
 
     trace_v9fs_walk(pdu->tag, pdu->id, fid, newfid, nwnames);
 
@@ -1215,7 +1279,11 @@ static void v9fs_walk(void *opaque)
         wnames = g_malloc0(sizeof(wnames[0]) * nwnames);
         qids   = g_malloc0(sizeof(qids[0]) * nwnames);
         for (i = 0; i < nwnames; i++) {
-            offset += pdu_unmarshal(pdu, offset, "s", &wnames[i]);
+            err = pdu_unmarshal(pdu, offset, "s", &wnames[i]);
+            if (err < 0) {
+                goto out_nofid;
+            }
+            offset += err;
         }
     } else if (nwnames > P9_MAXWELEM) {
         err = -EINVAL;
@@ -1314,9 +1382,12 @@ static void v9fs_open(void *opaque)
     V9fsState *s = pdu->s;
 
     if (s->proto_version == V9FS_PROTO_2000L) {
-        pdu_unmarshal(pdu, offset, "dd", &fid, &mode);
+        err = pdu_unmarshal(pdu, offset, "dd", &fid, &mode);
     } else {
-        pdu_unmarshal(pdu, offset, "db", &fid, &mode);
+        err = pdu_unmarshal(pdu, offset, "db", &fid, &mode);
+    }
+    if (err < 0) {
+        goto out_nofid;
     }
     trace_v9fs_open(pdu->tag, pdu->id, fid, mode);
 
@@ -1338,8 +1409,11 @@ static void v9fs_open(void *opaque)
             goto out;
         }
         fidp->fid_type = P9_FID_DIR;
-        offset += pdu_marshal(pdu, offset, "Qd", &qid, 0);
-        err = offset;
+        err = pdu_marshal(pdu, offset, "Qd", &qid, 0);
+        if (err < 0) {
+            goto out;
+        }
+        err += offset;
     } else {
         if (s->proto_version == V9FS_PROTO_2000L) {
             flags = get_dotl_openflags(s, mode);
@@ -1368,8 +1442,11 @@ static void v9fs_open(void *opaque)
             fidp->flags |= FID_NON_RECLAIMABLE;
         }
         iounit = get_iounit(pdu, &fidp->path);
-        offset += pdu_marshal(pdu, offset, "Qd", &qid, iounit);
-        err = offset;
+        err = pdu_marshal(pdu, offset, "Qd", &qid, iounit);
+        if (err < 0) {
+            goto out;
+        }
+        err += offset;
     }
     trace_v9fs_open_return(pdu->tag, pdu->id,
                            qid.type, qid.version, qid.path, iounit);
@@ -1392,8 +1469,12 @@ static void v9fs_lcreate(void *opaque)
     int32_t iounit;
     V9fsPDU *pdu = opaque;
 
-    pdu_unmarshal(pdu, offset, "dsddd", &dfid, &name, &flags,
-                  &mode, &gid);
+    v9fs_string_init(&name);
+    err = pdu_unmarshal(pdu, offset, "dsddd", &dfid,
+                        &name, &flags, &mode, &gid);
+    if (err < 0) {
+        goto out_nofid;
+    }
     trace_v9fs_lcreate(pdu->tag, pdu->id, dfid, flags, mode, gid);
 
     fidp = get_fid(pdu, dfid);
@@ -1419,8 +1500,11 @@ static void v9fs_lcreate(void *opaque)
     }
     iounit =  get_iounit(pdu, &fidp->path);
     stat_to_qid(&stbuf, &qid);
-    offset += pdu_marshal(pdu, offset, "Qd", &qid, iounit);
-    err = offset;
+    err = pdu_marshal(pdu, offset, "Qd", &qid, iounit);
+    if (err < 0) {
+        goto out;
+    }
+    err += offset;
     trace_v9fs_lcreate_return(pdu->tag, pdu->id,
                               qid.type, qid.version, qid.path, iounit);
 out:
@@ -1440,7 +1524,10 @@ static void v9fs_fsync(void *opaque)
     V9fsPDU *pdu = opaque;
     V9fsState *s = pdu->s;
 
-    pdu_unmarshal(pdu, offset, "dd", &fid, &datasync);
+    err = pdu_unmarshal(pdu, offset, "dd", &fid, &datasync);
+    if (err < 0) {
+        goto out_nofid;
+    }
     trace_v9fs_fsync(pdu->tag, pdu->id, fid, datasync);
 
     fidp = get_fid(pdu, fid);
@@ -1466,7 +1553,10 @@ static void v9fs_clunk(void *opaque)
     V9fsPDU *pdu = opaque;
     V9fsState *s = pdu->s;
 
-    pdu_unmarshal(pdu, offset, "d", &fid);
+    err = pdu_unmarshal(pdu, offset, "d", &fid);
+    if (err < 0) {
+        goto out_nofid;
+    }
     trace_v9fs_clunk(pdu->tag, pdu->id, fid);
 
     fidp = clunk_fid(s, fid);
@@ -1489,6 +1579,7 @@ out_nofid:
 static int v9fs_xattr_read(V9fsState *s, V9fsPDU *pdu,
                            V9fsFidState *fidp, int64_t off, int32_t max_count)
 {
+    ssize_t err;
     size_t offset = 7;
     int read_count;
     int64_t xattr_len;
@@ -1503,11 +1594,18 @@ static int v9fs_xattr_read(V9fsState *s, V9fsPDU *pdu,
          */
         read_count = 0;
     }
-    offset += pdu_marshal(pdu, offset, "d", read_count);
-    offset += v9fs_pack(pdu->elem.in_sg, pdu->elem.in_num, offset,
-                        ((char *)fidp->fs.xattr.value) + off,
-                        read_count);
-
+    err = pdu_marshal(pdu, offset, "d", read_count);
+    if (err < 0) {
+        return err;
+    }
+    offset += err;
+    err = v9fs_pack(pdu->elem.in_sg, pdu->elem.in_num, offset,
+                    ((char *)fidp->fs.xattr.value) + off,
+                    read_count);
+    if (err < 0) {
+        return err;
+    }
+    offset += err;
     return offset;
 }
 
@@ -1584,7 +1682,10 @@ static void v9fs_read(void *opaque)
     V9fsPDU *pdu = opaque;
     V9fsState *s = pdu->s;
 
-    pdu_unmarshal(pdu, offset, "dqd", &fid, &off, &max_count);
+    err = pdu_unmarshal(pdu, offset, "dqd", &fid, &off, &max_count);
+    if (err < 0) {
+        goto out_nofid;
+    }
     trace_v9fs_read(pdu->tag, pdu->id, fid, off, max_count);
 
     fidp = get_fid(pdu, fid);
@@ -1602,9 +1703,11 @@ static void v9fs_read(void *opaque)
             err = count;
             goto out;
         }
-        err = offset;
-        err += pdu_marshal(pdu, offset, "d", count);
-        err += count;
+        err = pdu_marshal(pdu, offset, "d", count);
+        if (err < 0) {
+            goto out;
+        }
+        err += offset + count;
     } else if (fidp->fid_type == P9_FID_FILE) {
         int32_t cnt;
         int32_t len;
@@ -1612,7 +1715,10 @@ static void v9fs_read(void *opaque)
         struct iovec iov[128]; /* FIXME: bad, bad, bad */
 
         sg = iov;
-        pdu_marshal(pdu, offset + 4, "v", sg, &cnt);
+        err = pdu_marshal(pdu, offset + 4, "v", sg, &cnt);
+        if (err < 0) {
+            goto out;
+        }
         sg = cap_sg(sg, max_count, &cnt);
         do {
             if (0) {
@@ -1633,9 +1739,11 @@ static void v9fs_read(void *opaque)
             }
             sg = adjust_sg(sg, len, &cnt);
         } while (count < max_count && len > 0);
-        err = offset;
-        err += pdu_marshal(pdu, offset, "d", count);
-        err += count;
+        err = pdu_marshal(pdu, offset, "d", count);
+        if (err < 0) {
+            goto out;
+        }
+        err += offset + count;
     } else if (fidp->fid_type == P9_FID_XATTR) {
         err = v9fs_xattr_read(s, pdu, fidp, off, max_count);
     } else {
@@ -1705,6 +1813,12 @@ static int v9fs_do_readdir(V9fsPDU *pdu,
         len = pdu_marshal(pdu, 11 + count, "Qqbs",
                           &qid, dent->d_off,
                           dent->d_type, &name);
+        if (len < 0) {
+            v9fs_co_seekdir(pdu, fidp, saved_dir_pos);
+            v9fs_string_free(&name);
+            g_free(dent);
+            return len;
+        }
         count += len;
         v9fs_string_free(&name);
         saved_dir_pos = dent->d_off;
@@ -1727,8 +1841,11 @@ static void v9fs_readdir(void *opaque)
     V9fsPDU *pdu = opaque;
     V9fsState *s = pdu->s;
 
-    pdu_unmarshal(pdu, offset, "dqd", &fid, &initial_offset, &max_count);
-
+    retval = pdu_unmarshal(pdu, offset, "dqd", &fid,
+                           &initial_offset, &max_count);
+    if (retval < 0) {
+        goto out_nofid;
+    }
     trace_v9fs_readdir(pdu->tag, pdu->id, fid, initial_offset, max_count);
 
     fidp = get_fid(pdu, fid);
@@ -1750,9 +1867,11 @@ static void v9fs_readdir(void *opaque)
         retval = count;
         goto out;
     }
-    retval = offset;
-    retval += pdu_marshal(pdu, offset, "d", count);
-    retval += count;
+    retval = pdu_marshal(pdu, offset, "d", count);
+    if (retval < 0) {
+        goto out;
+    }
+    retval += count + offset;
     trace_v9fs_readdir_return(pdu->tag, pdu->id, count, retval);
 out:
     put_fid(pdu, fidp);
@@ -1783,8 +1902,11 @@ static int v9fs_xattr_write(V9fsState *s, V9fsPDU *pdu, V9fsFidState *fidp,
         err = -ENOSPC;
         goto out;
     }
-    offset += pdu_marshal(pdu, offset, "d", write_count);
-    err = offset;
+    err = pdu_marshal(pdu, offset, "d", write_count);
+    if (err < 0) {
+        return err;
+    }
+    err += offset;
     fidp->fs.xattr.copied_len += write_count;
     /*
      * Now copy the content from sg list
@@ -1820,7 +1942,10 @@ static void v9fs_write(void *opaque)
     V9fsPDU *pdu = opaque;
     V9fsState *s = pdu->s;
 
-    pdu_unmarshal(pdu, offset, "dqdv", &fid, &off, &count, sg, &cnt);
+    err = pdu_unmarshal(pdu, offset, "dqdv", &fid, &off, &count, sg, &cnt);
+    if (err < 0) {
+        goto out_nofid;
+    }
     trace_v9fs_write(pdu->tag, pdu->id, fid, off, count, cnt);
 
     fidp = get_fid(pdu, fid);
@@ -1863,8 +1988,11 @@ static void v9fs_write(void *opaque)
         }
         sg = adjust_sg(sg, len, &cnt);
     } while (total < count && len > 0);
-    offset += pdu_marshal(pdu, offset, "d", total);
-    err = offset;
+    err = pdu_marshal(pdu, offset, "d", total);
+    if (err < 0) {
+        goto out;
+    }
+    err += offset;
     trace_v9fs_write_return(pdu->tag, pdu->id, total, err);
 out:
     put_fid(pdu, fidp);
@@ -1889,10 +2017,13 @@ static void v9fs_create(void *opaque)
     V9fsPDU *pdu = opaque;
 
     v9fs_path_init(&path);
-
-    pdu_unmarshal(pdu, offset, "dsdbs", &fid, &name,
-                  &perm, &mode, &extension);
-
+    v9fs_string_init(&name);
+    v9fs_string_init(&extension);
+    err = pdu_unmarshal(pdu, offset, "dsdbs", &fid, &name,
+                        &perm, &mode, &extension);
+    if (err < 0) {
+        goto out_nofid;
+    }
     trace_v9fs_create(pdu->tag, pdu->id, fid, name.data, perm, mode);
 
     fidp = get_fid(pdu, fid);
@@ -2023,8 +2154,11 @@ static void v9fs_create(void *opaque)
     }
     iounit = get_iounit(pdu, &fidp->path);
     stat_to_qid(&stbuf, &qid);
-    offset += pdu_marshal(pdu, offset, "Qd", &qid, iounit);
-    err = offset;
+    err = pdu_marshal(pdu, offset, "Qd", &qid, iounit);
+    if (err < 0) {
+        goto out;
+    }
+    err += offset;
     trace_v9fs_create_return(pdu->tag, pdu->id,
                              qid.type, qid.version, qid.path, iounit);
 out:
@@ -2049,7 +2183,12 @@ static void v9fs_symlink(void *opaque)
     gid_t gid;
     size_t offset = 7;
 
-    pdu_unmarshal(pdu, offset, "dssd", &dfid, &name, &symname, &gid);
+    v9fs_string_init(&name);
+    v9fs_string_init(&symname);
+    err = pdu_unmarshal(pdu, offset, "dssd", &dfid, &name, &symname, &gid);
+    if (err < 0) {
+        goto out_nofid;
+    }
     trace_v9fs_symlink(pdu->tag, pdu->id, dfid, name.data, symname.data, gid);
 
     dfidp = get_fid(pdu, dfid);
@@ -2062,8 +2201,11 @@ static void v9fs_symlink(void *opaque)
         goto out;
     }
     stat_to_qid(&stbuf, &qid);
-    offset += pdu_marshal(pdu, offset, "Q", &qid);
-    err = offset;
+    err =  pdu_marshal(pdu, offset, "Q", &qid);
+    if (err < 0) {
+        goto out;
+    }
+    err += offset;
     trace_v9fs_symlink_return(pdu->tag, pdu->id,
                               qid.type, qid.version, qid.path);
 out:
@@ -2076,13 +2218,18 @@ out_nofid:
 
 static void v9fs_flush(void *opaque)
 {
+    ssize_t err;
     int16_t tag;
     size_t offset = 7;
     V9fsPDU *cancel_pdu;
     V9fsPDU *pdu = opaque;
     V9fsState *s = pdu->s;
 
-    pdu_unmarshal(pdu, offset, "w", &tag);
+    err = pdu_unmarshal(pdu, offset, "w", &tag);
+    if (err < 0) {
+        complete_pdu(s, pdu, err);
+        return;
+    }
     trace_v9fs_flush(pdu->tag, pdu->id, tag);
 
     QLIST_FOREACH(cancel_pdu, &s->active_list, next) {
@@ -2109,11 +2256,15 @@ static void v9fs_link(void *opaque)
     V9fsState *s = pdu->s;
     int32_t dfid, oldfid;
     V9fsFidState *dfidp, *oldfidp;
-    V9fsString name;;
+    V9fsString name;
     size_t offset = 7;
     int err = 0;
 
-    pdu_unmarshal(pdu, offset, "dds", &dfid, &oldfid, &name);
+    v9fs_string_init(&name);
+    err = pdu_unmarshal(pdu, offset, "dds", &dfid, &oldfid, &name);
+    if (err < 0) {
+        goto out_nofid;
+    }
     trace_v9fs_link(pdu->tag, pdu->id, dfid, oldfid, name.data);
 
     dfidp = get_fid(pdu, dfid);
@@ -2147,7 +2298,10 @@ static void v9fs_remove(void *opaque)
     V9fsFidState *fidp;
     V9fsPDU *pdu = opaque;
 
-    pdu_unmarshal(pdu, offset, "d", &fid);
+    err = pdu_unmarshal(pdu, offset, "d", &fid);
+    if (err < 0) {
+        goto out_nofid;
+    }
     trace_v9fs_remove(pdu->tag, pdu->id, fid);
 
     fidp = get_fid(pdu, fid);
@@ -2190,8 +2344,11 @@ static void v9fs_unlinkat(void *opaque)
     V9fsFidState *dfidp;
     V9fsPDU *pdu = opaque;
 
-    pdu_unmarshal(pdu, offset, "dsd", &dfid, &name, &flags);
-
+    v9fs_string_init(&name);
+    err = pdu_unmarshal(pdu, offset, "dsd", &dfid, &name, &flags);
+    if (err < 0) {
+        goto out_nofid;
+    }
     dfidp = get_fid(pdu, dfid);
     if (dfidp == NULL) {
         err = -EINVAL;
@@ -2293,8 +2450,11 @@ static void v9fs_rename(void *opaque)
     V9fsPDU *pdu = opaque;
     V9fsState *s = pdu->s;
 
-    pdu_unmarshal(pdu, offset, "dds", &fid, &newdirfid, &name);
-
+    v9fs_string_init(&name);
+    err = pdu_unmarshal(pdu, offset, "dds", &fid, &newdirfid, &name);
+    if (err < 0) {
+        goto out_nofid;
+    }
     fidp = get_fid(pdu, fid);
     if (fidp == NULL) {
         err = -ENOENT;
@@ -2399,8 +2559,13 @@ static void v9fs_renameat(void *opaque)
     int32_t olddirfid, newdirfid;
     V9fsString old_name, new_name;
 
-    pdu_unmarshal(pdu, offset, "dsds", &olddirfid,
-                  &old_name, &newdirfid, &new_name);
+    v9fs_string_init(&old_name);
+    v9fs_string_init(&new_name);
+    err = pdu_unmarshal(pdu, offset, "dsds", &olddirfid,
+                        &old_name, &newdirfid, &new_name);
+    if (err < 0) {
+        goto out_err;
+    }
 
     v9fs_path_write_lock(s);
     err = v9fs_complete_renameat(pdu, olddirfid,
@@ -2409,6 +2574,8 @@ static void v9fs_renameat(void *opaque)
     if (!err) {
         err = offset;
     }
+
+out_err:
     complete_pdu(s, pdu, err);
     v9fs_string_free(&old_name);
     v9fs_string_free(&new_name);
@@ -2426,7 +2593,11 @@ static void v9fs_wstat(void *opaque)
     V9fsPDU *pdu = opaque;
     V9fsState *s = pdu->s;
 
-    pdu_unmarshal(pdu, offset, "dwS", &fid, &unused, &v9stat);
+    v9fs_stat_init(&v9stat);
+    err = pdu_unmarshal(pdu, offset, "dwS", &fid, &unused, &v9stat);
+    if (err < 0) {
+        goto out_nofid;
+    }
     trace_v9fs_wstat(pdu->tag, pdu->id, fid,
                      v9stat.mode, v9stat.atime, v9stat.mtime);
 
@@ -2560,7 +2731,10 @@ static void v9fs_statfs(void *opaque)
     V9fsPDU *pdu = opaque;
     V9fsState *s = pdu->s;
 
-    pdu_unmarshal(pdu, offset, "d", &fid);
+    retval = pdu_unmarshal(pdu, offset, "d", &fid);
+    if (retval < 0) {
+        goto out_nofid;
+    }
     fidp = get_fid(pdu, fid);
     if (fidp == NULL) {
         retval = -ENOENT;
@@ -2570,8 +2744,11 @@ static void v9fs_statfs(void *opaque)
     if (retval < 0) {
         goto out;
     }
-    retval = offset;
-    retval += v9fs_fill_statfs(s, pdu, &stbuf);
+    retval = v9fs_fill_statfs(s, pdu, &stbuf);
+    if (retval < 0) {
+        goto out;
+    }
+    retval += offset;
 out:
     put_fid(pdu, fidp);
 out_nofid:
@@ -2595,8 +2772,12 @@ static void v9fs_mknod(void *opaque)
     V9fsPDU *pdu = opaque;
     V9fsState *s = pdu->s;
 
-    pdu_unmarshal(pdu, offset, "dsdddd", &fid, &name, &mode,
-                  &major, &minor, &gid);
+    v9fs_string_init(&name);
+    err = pdu_unmarshal(pdu, offset, "dsdddd", &fid, &name, &mode,
+                        &major, &minor, &gid);
+    if (err < 0) {
+        goto out_nofid;
+    }
     trace_v9fs_mknod(pdu->tag, pdu->id, fid, mode, major, minor);
 
     fidp = get_fid(pdu, fid);
@@ -2610,8 +2791,11 @@ static void v9fs_mknod(void *opaque)
         goto out;
     }
     stat_to_qid(&stbuf, &qid);
-    err = offset;
-    err += pdu_marshal(pdu, offset, "Q", &qid);
+    err = pdu_marshal(pdu, offset, "Q", &qid);
+    if (err < 0) {
+        goto out;
+    }
+    err += offset;
     trace_v9fs_mknod_return(pdu->tag, pdu->id,
                             qid.type, qid.version, qid.path);
 out:
@@ -2632,7 +2816,7 @@ out_nofid:
 static void v9fs_lock(void *opaque)
 {
     int8_t status;
-    V9fsFlock *flock;
+    V9fsFlock flock;
     size_t offset = 7;
     struct stat stbuf;
     V9fsFidState *fidp;
@@ -2640,18 +2824,20 @@ static void v9fs_lock(void *opaque)
     V9fsPDU *pdu = opaque;
     V9fsState *s = pdu->s;
 
-    flock = g_malloc(sizeof(*flock));
-    pdu_unmarshal(pdu, offset, "dbdqqds", &fid, &flock->type,
-                  &flock->flags, &flock->start, &flock->length,
-                  &flock->proc_id, &flock->client_id);
-
+    status = P9_LOCK_ERROR;
+    v9fs_string_init(&flock.client_id);
+    err = pdu_unmarshal(pdu, offset, "dbdqqds", &fid, &flock.type,
+                        &flock.flags, &flock.start, &flock.length,
+                        &flock.proc_id, &flock.client_id);
+    if (err < 0) {
+        goto out_nofid;
+    }
     trace_v9fs_lock(pdu->tag, pdu->id, fid,
-                    flock->type, flock->start, flock->length);
+                    flock.type, flock.start, flock.length);
 
-    status = P9_LOCK_ERROR;
 
     /* We support only block flag now (that too ignored currently) */
-    if (flock->flags & ~P9_LOCK_FLAGS_BLOCK) {
+    if (flock.flags & ~P9_LOCK_FLAGS_BLOCK) {
         err = -EINVAL;
         goto out_nofid;
     }
@@ -2668,12 +2854,13 @@ static void v9fs_lock(void *opaque)
 out:
     put_fid(pdu, fidp);
 out_nofid:
-    err = offset;
-    err += pdu_marshal(pdu, offset, "b", status);
+    err = pdu_marshal(pdu, offset, "b", status);
+    if (err > 0) {
+        err += offset;
+    }
     trace_v9fs_lock_return(pdu->tag, pdu->id, status);
     complete_pdu(s, pdu, err);
-    v9fs_string_free(&flock->client_id);
-    g_free(flock);
+    v9fs_string_free(&flock.client_id);
 }
 
 /*
@@ -2685,18 +2872,20 @@ static void v9fs_getlock(void *opaque)
     size_t offset = 7;
     struct stat stbuf;
     V9fsFidState *fidp;
-    V9fsGetlock *glock;
+    V9fsGetlock glock;
     int32_t fid, err = 0;
     V9fsPDU *pdu = opaque;
     V9fsState *s = pdu->s;
 
-    glock = g_malloc(sizeof(*glock));
-    pdu_unmarshal(pdu, offset, "dbqqds", &fid, &glock->type,
-                  &glock->start, &glock->length, &glock->proc_id,
-                  &glock->client_id);
-
+    v9fs_string_init(&glock.client_id);
+    err = pdu_unmarshal(pdu, offset, "dbqqds", &fid, &glock.type,
+                        &glock.start, &glock.length, &glock.proc_id,
+                        &glock.client_id);
+    if (err < 0) {
+        goto out_nofid;
+    }
     trace_v9fs_getlock(pdu->tag, pdu->id, fid,
-                       glock->type, glock->start, glock->length);
+                       glock.type, glock.start, glock.length);
 
     fidp = get_fid(pdu, fid);
     if (fidp == NULL) {
@@ -2707,19 +2896,21 @@ static void v9fs_getlock(void *opaque)
     if (err < 0) {
         goto out;
     }
-    glock->type = P9_LOCK_TYPE_UNLCK;
-    offset += pdu_marshal(pdu, offset, "bqqds", glock->type,
-                          glock->start, glock->length, glock->proc_id,
-                          &glock->client_id);
-    err = offset;
-    trace_v9fs_getlock_return(pdu->tag, pdu->id, glock->type, glock->start,
-                              glock->length, glock->proc_id);
+    glock.type = P9_LOCK_TYPE_UNLCK;
+    err = pdu_marshal(pdu, offset, "bqqds", glock.type,
+                          glock.start, glock.length, glock.proc_id,
+                          &glock.client_id);
+    if (err < 0) {
+        goto out;
+    }
+    err += offset;
+    trace_v9fs_getlock_return(pdu->tag, pdu->id, glock.type, glock.start,
+                              glock.length, glock.proc_id);
 out:
     put_fid(pdu, fidp);
 out_nofid:
     complete_pdu(s, pdu, err);
-    v9fs_string_free(&glock->client_id);
-    g_free(glock);
+    v9fs_string_free(&glock.client_id);
 }
 
 static void v9fs_mkdir(void *opaque)
@@ -2735,8 +2926,11 @@ static void v9fs_mkdir(void *opaque)
     int mode;
     int err = 0;
 
-    pdu_unmarshal(pdu, offset, "dsdd", &fid, &name, &mode, &gid);
-
+    v9fs_string_init(&name);
+    err = pdu_unmarshal(pdu, offset, "dsdd", &fid, &name, &mode, &gid);
+    if (err < 0) {
+        goto out_nofid;
+    }
     trace_v9fs_mkdir(pdu->tag, pdu->id, fid, name.data, mode, gid);
 
     fidp = get_fid(pdu, fid);
@@ -2749,8 +2943,11 @@ static void v9fs_mkdir(void *opaque)
         goto out;
     }
     stat_to_qid(&stbuf, &qid);
-    offset += pdu_marshal(pdu, offset, "Q", &qid);
-    err = offset;
+    err = pdu_marshal(pdu, offset, "Q", &qid);
+    if (err < 0) {
+        goto out;
+    }
+    err += offset;
     trace_v9fs_mkdir_return(pdu->tag, pdu->id,
                             qid.type, qid.version, qid.path, err);
 out:
@@ -2772,7 +2969,11 @@ static void v9fs_xattrwalk(void *opaque)
     V9fsPDU *pdu = opaque;
     V9fsState *s = pdu->s;
 
-    pdu_unmarshal(pdu, offset, "dds", &fid, &newfid, &name);
+    v9fs_string_init(&name);
+    err = pdu_unmarshal(pdu, offset, "dds", &fid, &newfid, &name);
+    if (err < 0) {
+        goto out_nofid;
+    }
     trace_v9fs_xattrwalk(pdu->tag, pdu->id, fid, newfid, name.data);
 
     file_fidp = get_fid(pdu, fid);
@@ -2786,7 +2987,7 @@ static void v9fs_xattrwalk(void *opaque)
         goto out;
     }
     v9fs_path_copy(&xattr_fidp->path, &file_fidp->path);
-    if (name.data[0] == 0) {
+    if (name.data == NULL) {
         /*
          * listxattr request. Get the size first
          */
@@ -2812,8 +3013,11 @@ static void v9fs_xattrwalk(void *opaque)
                 goto out;
             }
         }
-        offset += pdu_marshal(pdu, offset, "q", size);
-        err = offset;
+        err = pdu_marshal(pdu, offset, "q", size);
+        if (err < 0) {
+            goto out;
+        }
+        err += offset;
     } else {
         /*
          * specific xattr fid. We check for xattr
@@ -2842,8 +3046,11 @@ static void v9fs_xattrwalk(void *opaque)
                 goto out;
             }
         }
-        offset += pdu_marshal(pdu, offset, "q", size);
-        err = offset;
+        err = pdu_marshal(pdu, offset, "q", size);
+        if (err < 0) {
+            goto out;
+        }
+        err += offset;
     }
     trace_v9fs_xattrwalk_return(pdu->tag, pdu->id, size);
 out:
@@ -2869,8 +3076,11 @@ static void v9fs_xattrcreate(void *opaque)
     V9fsPDU *pdu = opaque;
     V9fsState *s = pdu->s;
 
-    pdu_unmarshal(pdu, offset, "dsqd",
-                  &fid, &name, &size, &flags);
+    v9fs_string_init(&name);
+    err = pdu_unmarshal(pdu, offset, "dsqd", &fid, &name, &size, &flags);
+    if (err < 0) {
+        goto out_nofid;
+    }
     trace_v9fs_xattrcreate(pdu->tag, pdu->id, fid, name.data, size, flags);
 
     file_fidp = get_fid(pdu, fid);
@@ -2907,7 +3117,10 @@ static void v9fs_readlink(void *opaque)
     int err = 0;
     V9fsFidState *fidp;
 
-    pdu_unmarshal(pdu, offset, "d", &fid);
+    err = pdu_unmarshal(pdu, offset, "d", &fid);
+    if (err < 0) {
+        goto out_nofid;
+    }
     trace_v9fs_readlink(pdu->tag, pdu->id, fid);
     fidp = get_fid(pdu, fid);
     if (fidp == NULL) {
@@ -2920,8 +3133,12 @@ static void v9fs_readlink(void *opaque)
     if (err < 0) {
         goto out;
     }
-    offset += pdu_marshal(pdu, offset, "s", &target);
-    err = offset;
+    err = pdu_marshal(pdu, offset, "s", &target);
+    if (err < 0) {
+        v9fs_string_free(&target);
+        goto out;
+    }
+    err += offset;
     trace_v9fs_readlink_return(pdu->tag, pdu->id, target.data);
     v9fs_string_free(&target);
 out:
-- 
1.7.6

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [Qemu-devel] [PATCH V4 03/13] hw/9pfs: Add new proxy filesystem driver
  2011-12-05 16:18 [Qemu-devel] [PATCH V4 00/13] Proxy FS driver for VirtFS M. Mohan Kumar
  2011-12-05 16:18 ` [Qemu-devel] [PATCH V4 01/13] hw/9pfs: Move pdu_marshal/unmarshal code to a seperate file M. Mohan Kumar
  2011-12-05 16:18 ` [Qemu-devel] [PATCH V4 02/13] hw/9pfs: Add validation to {un}marshal code M. Mohan Kumar
@ 2011-12-05 16:18 ` M. Mohan Kumar
  2011-12-05 16:18 ` [Qemu-devel] [PATCH V4 04/13] hw/9pfs: File system helper process for qemu 9p proxy FS M. Mohan Kumar
                   ` (10 subsequent siblings)
  13 siblings, 0 replies; 22+ messages in thread
From: M. Mohan Kumar @ 2011-12-05 16:18 UTC (permalink / raw)
  To: qemu-devel, aneesh.kumar, stefanha; +Cc: M. Mohan Kumar

From: "M. Mohan Kumar" <mohan@in.ibm.com>

Add new proxy filesystem driver to add root privilege to qemu process.
It needs a helper process to be started by root user.

Following command line can be used to utilize proxy filesystem driver
-virtfs proxy,id=<id>,mount_tag=<tag>,socket_fd=<socket-fd>

Signed-off-by: M. Mohan Kumar <mohan@in.ibm.com>
---
 Makefile.objs             |    1 +
 fsdev/qemu-fsdev.c        |    1 +
 fsdev/qemu-fsdev.h        |    1 +
 hw/9pfs/virtio-9p-proxy.c |  388 +++++++++++++++++++++++++++++++++++++++++++++
 hw/9pfs/virtio-9p-proxy.h |   24 +++
 qemu-config.c             |    6 +
 vl.c                      |    6 +-
 7 files changed, 426 insertions(+), 1 deletions(-)
 create mode 100644 hw/9pfs/virtio-9p-proxy.c
 create mode 100644 hw/9pfs/virtio-9p-proxy.h

diff --git a/Makefile.objs b/Makefile.objs
index 1ca8369..1696675 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -312,6 +312,7 @@ hw-obj-$(CONFIG_SOUND) += $(sound-obj-y)
 9pfs-nested-$(CONFIG_VIRTFS) += virtio-9p-coth.o cofs.o codir.o cofile.o
 9pfs-nested-$(CONFIG_VIRTFS) += coxattr.o virtio-9p-synth.o
 9pfs-nested-$(CONFIG_OPEN_BY_HANDLE) +=  virtio-9p-handle.o
+9pfs-nested-$(CONFIG_VIRTFS) += virtio-9p-proxy.o
 
 hw-obj-$(CONFIG_REALLY_VIRTFS) += $(addprefix 9pfs/, $(9pfs-nested-y))
 $(addprefix 9pfs/, $(9pfs-nested-y)): QEMU_CFLAGS+=$(GLIB_CFLAGS)
diff --git a/fsdev/qemu-fsdev.c b/fsdev/qemu-fsdev.c
index 00f48ab..e20202a 100644
--- a/fsdev/qemu-fsdev.c
+++ b/fsdev/qemu-fsdev.c
@@ -27,6 +27,7 @@ static FsDriverTable FsDrivers[] = {
     { .name = "handle", .ops = &handle_ops},
 #endif
     { .name = "synth", .ops = &synth_ops},
+    { .name = "proxy", .ops = &proxy_ops},
 };
 
 int qemu_fsdev_add(QemuOpts *opts)
diff --git a/fsdev/qemu-fsdev.h b/fsdev/qemu-fsdev.h
index 921452d..1af1f54 100644
--- a/fsdev/qemu-fsdev.h
+++ b/fsdev/qemu-fsdev.h
@@ -44,4 +44,5 @@ FsDriverEntry *get_fsdev_fsentry(char *id);
 extern FileOperations local_ops;
 extern FileOperations handle_ops;
 extern FileOperations synth_ops;
+extern FileOperations proxy_ops;
 #endif
diff --git a/hw/9pfs/virtio-9p-proxy.c b/hw/9pfs/virtio-9p-proxy.c
new file mode 100644
index 0000000..3e4dc68
--- /dev/null
+++ b/hw/9pfs/virtio-9p-proxy.c
@@ -0,0 +1,388 @@
+/*
+ * Virtio 9p Proxy callback
+ *
+ * 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/socket.h>
+#include <sys/un.h>
+#include "hw/virtio.h"
+#include "virtio-9p.h"
+#include "fsdev/qemu-fsdev.h"
+#include "virtio-9p-proxy.h"
+
+typedef struct V9fsProxy {
+    int sockfd;
+    QemuMutex mutex;
+    struct iovec iovec;
+} V9fsProxy;
+
+static int proxy_lstat(FsContext *fs_ctx, V9fsPath *fs_path, struct stat *stbuf)
+{
+    errno = EOPNOTSUPP;
+    return -1;
+}
+
+static ssize_t proxy_readlink(FsContext *fs_ctx, V9fsPath *fs_path,
+                              char *buf, size_t bufsz)
+{
+    errno = EOPNOTSUPP;
+    return -1;
+}
+
+static int proxy_close(FsContext *ctx, V9fsFidOpenState *fs)
+{
+    return close(fs->fd);
+}
+
+static int proxy_closedir(FsContext *ctx, V9fsFidOpenState *fs)
+{
+    return closedir(fs->dir);
+}
+
+static int proxy_open(FsContext *ctx, V9fsPath *fs_path,
+                      int flags, V9fsFidOpenState *fs)
+{
+    fs->fd = -1;
+    return fs->fd;
+}
+
+static int proxy_opendir(FsContext *ctx,
+                         V9fsPath *fs_path, V9fsFidOpenState *fs)
+{
+    fs->dir = NULL;
+    errno = EOPNOTSUPP;
+    return -1;
+}
+
+static void proxy_rewinddir(FsContext *ctx, V9fsFidOpenState *fs)
+{
+    return rewinddir(fs->dir);
+}
+
+static off_t proxy_telldir(FsContext *ctx, V9fsFidOpenState *fs)
+{
+    return telldir(fs->dir);
+}
+
+static int proxy_readdir_r(FsContext *ctx, V9fsFidOpenState *fs,
+                           struct dirent *entry,
+                           struct dirent **result)
+{
+    return readdir_r(fs->dir, entry, result);
+}
+
+static void proxy_seekdir(FsContext *ctx, V9fsFidOpenState *fs, off_t off)
+{
+    return seekdir(fs->dir, off);
+}
+
+static ssize_t proxy_preadv(FsContext *ctx, V9fsFidOpenState *fs,
+                            const struct iovec *iov,
+                            int iovcnt, off_t offset)
+{
+#ifdef CONFIG_PREADV
+    return preadv(fs->fd, iov, iovcnt, offset);
+#else
+    int err = lseek(fs->fd, offset, SEEK_SET);
+    if (err == -1) {
+        return err;
+    } else {
+        return readv(fs->fd, iov, iovcnt);
+    }
+#endif
+}
+
+static ssize_t proxy_pwritev(FsContext *ctx, V9fsFidOpenState *fs,
+                             const struct iovec *iov,
+                             int iovcnt, off_t offset)
+{
+    ssize_t ret;
+
+#ifdef CONFIG_PREADV
+    ret = pwritev(fs->fd, iov, iovcnt, offset);
+#else
+    int err = lseek(fs->fd, offset, SEEK_SET);
+    if (err == -1) {
+        return err;
+    } else {
+        ret = writev(fs->fd, iov, iovcnt);
+    }
+#endif
+#ifdef CONFIG_SYNC_FILE_RANGE
+    if (ret > 0 && ctx->export_flags & V9FS_IMMEDIATE_WRITEOUT) {
+        /*
+         * Initiate a writeback. This is not a data integrity sync.
+         * We want to ensure that we don't leave dirty pages in the cache
+         * after write when writeout=immediate is sepcified.
+         */
+        sync_file_range(fs->fd, offset, ret,
+                        SYNC_FILE_RANGE_WAIT_BEFORE | SYNC_FILE_RANGE_WRITE);
+    }
+#endif
+    return ret;
+}
+
+static int proxy_chmod(FsContext *fs_ctx, V9fsPath *fs_path, FsCred *credp)
+{
+    errno = EOPNOTSUPP;
+    return -1;
+}
+
+static int proxy_mknod(FsContext *fs_ctx, V9fsPath *dir_path,
+                       const char *name, FsCred *credp)
+{
+    errno = EOPNOTSUPP;
+    return -1;
+}
+
+static int proxy_mkdir(FsContext *fs_ctx, V9fsPath *dir_path,
+                       const char *name, FsCred *credp)
+{
+    errno = EOPNOTSUPP;
+    return -1;
+}
+
+static int proxy_fstat(FsContext *fs_ctx, int fid_type,
+                       V9fsFidOpenState *fs, struct stat *stbuf)
+{
+    int fd;
+
+    if (fid_type == P9_FID_DIR) {
+        fd = dirfd(fs->dir);
+    } else {
+        fd = fs->fd;
+    }
+    return fstat(fd, stbuf);
+}
+
+static int proxy_open2(FsContext *fs_ctx, V9fsPath *dir_path, const char *name,
+                       int flags, FsCred *credp, V9fsFidOpenState *fs)
+{
+    fs->fd = -1;
+    errno = EOPNOTSUPP;
+    return -1;
+}
+
+
+static int proxy_symlink(FsContext *fs_ctx, const char *oldpath,
+                         V9fsPath *dir_path, const char *name, FsCred *credp)
+{
+    errno = EOPNOTSUPP;
+    return -1;
+}
+
+static int proxy_link(FsContext *ctx, V9fsPath *oldpath,
+                      V9fsPath *dirpath, const char *name)
+{
+    errno = EOPNOTSUPP;
+    return -1;
+}
+
+static int proxy_truncate(FsContext *ctx, V9fsPath *fs_path, off_t size)
+{
+    errno = EOPNOTSUPP;
+    return -1;
+}
+
+static int proxy_rename(FsContext *ctx, const char *oldpath,
+                        const char *newpath)
+{
+    errno = EOPNOTSUPP;
+    return -1;
+}
+
+static int proxy_chown(FsContext *fs_ctx, V9fsPath *fs_path, FsCred *credp)
+{
+    errno = EOPNOTSUPP;
+    return -1;
+}
+
+static int proxy_utimensat(FsContext *s, V9fsPath *fs_path,
+                           const struct timespec *buf)
+{
+    errno = EOPNOTSUPP;
+    return -1;
+}
+
+static int proxy_remove(FsContext *ctx, const char *path)
+{
+    errno = EOPNOTSUPP;
+    return -1;
+}
+
+static int proxy_fsync(FsContext *ctx, int fid_type,
+                       V9fsFidOpenState *fs, int datasync)
+{
+    int fd;
+
+    if (fid_type == P9_FID_DIR) {
+        fd = dirfd(fs->dir);
+    } else {
+        fd = fs->fd;
+    }
+
+    if (datasync) {
+        return qemu_fdatasync(fd);
+    } else {
+        return fsync(fd);
+    }
+}
+
+static int proxy_statfs(FsContext *s, V9fsPath *fs_path, struct statfs *stbuf)
+{
+    errno = EOPNOTSUPP;
+    return -1;
+}
+
+static ssize_t proxy_lgetxattr(FsContext *ctx, V9fsPath *fs_path,
+                               const char *name, void *value, size_t size)
+{
+    errno = EOPNOTSUPP;
+    return -1;
+}
+
+static ssize_t proxy_llistxattr(FsContext *ctx, V9fsPath *fs_path,
+                                void *value, size_t size)
+{
+    errno = EOPNOTSUPP;
+    return -1;
+}
+
+static int proxy_lsetxattr(FsContext *ctx, V9fsPath *fs_path, const char *name,
+                           void *value, size_t size, int flags)
+{
+    errno = EOPNOTSUPP;
+    return -1;
+}
+
+static int proxy_lremovexattr(FsContext *ctx, V9fsPath *fs_path,
+                              const char *name)
+{
+    errno = EOPNOTSUPP;
+    return -1;
+}
+
+static int proxy_name_to_path(FsContext *ctx, V9fsPath *dir_path,
+                              const char *name, V9fsPath *target)
+{
+    if (dir_path) {
+        v9fs_string_sprintf((V9fsString *)target, "%s/%s",
+                            dir_path->data, name);
+    } else {
+        v9fs_string_sprintf((V9fsString *)target, "%s", name);
+    }
+    /* Bump the size for including terminating NULL */
+    target->size++;
+    return 0;
+}
+
+static int proxy_renameat(FsContext *ctx, V9fsPath *olddir,
+                          const char *old_name, V9fsPath *newdir,
+                          const char *new_name)
+{
+    int ret;
+    V9fsString old_full_name, new_full_name;
+
+    v9fs_string_init(&old_full_name);
+    v9fs_string_init(&new_full_name);
+
+    v9fs_string_sprintf(&old_full_name, "%s/%s", olddir->data, old_name);
+    v9fs_string_sprintf(&new_full_name, "%s/%s", newdir->data, new_name);
+
+    ret = proxy_rename(ctx, old_full_name.data, new_full_name.data);
+    v9fs_string_free(&old_full_name);
+    v9fs_string_free(&new_full_name);
+    return ret;
+}
+
+static int proxy_unlinkat(FsContext *ctx, V9fsPath *dir,
+                          const char *name, int flags)
+{
+    int ret;
+    V9fsString fullname;
+    v9fs_string_init(&fullname);
+
+    v9fs_string_sprintf(&fullname, "%s/%s", dir->data, name);
+    ret = proxy_remove(ctx, fullname.data);
+    v9fs_string_free(&fullname);
+
+    return ret;
+}
+
+static int proxy_parse_opts(QemuOpts *opts, struct FsDriverEntry *fs)
+{
+    const char *sock_fd = qemu_opt_get(opts, "sock_fd");
+
+    if (sock_fd) {
+        fprintf(stderr, "sock_fd option not specified\n");
+        return -1;
+    }
+    fs->path = g_strdup(sock_fd);
+    return 0;
+}
+
+static int proxy_init(FsContext *ctx)
+{
+    V9fsProxy *proxy = g_malloc(sizeof(V9fsProxy));
+    int sock_id;
+
+    sock_id = atoi(ctx->fs_root);
+    if (sock_id < 0) {
+        fprintf(stderr, "socket descriptor not initialized\n");
+        return -1;
+    }
+    g_free(ctx->fs_root);
+
+    proxy->iovec.iov_base = g_malloc(PROXY_MAX_IO_SZ + PROXY_HDR_SZ);
+    proxy->iovec.iov_len = PROXY_MAX_IO_SZ + PROXY_HDR_SZ;
+    ctx->private = proxy;
+    proxy->sockfd = sock_id;
+    qemu_mutex_init(&proxy->mutex);
+
+    ctx->export_flags |= V9FS_PATHNAME_FSCONTEXT;
+    return 0;
+}
+
+FileOperations proxy_ops = {
+    .parse_opts   = proxy_parse_opts,
+    .init         = proxy_init,
+    .lstat        = proxy_lstat,
+    .readlink     = proxy_readlink,
+    .close        = proxy_close,
+    .closedir     = proxy_closedir,
+    .open         = proxy_open,
+    .opendir      = proxy_opendir,
+    .rewinddir    = proxy_rewinddir,
+    .telldir      = proxy_telldir,
+    .readdir_r    = proxy_readdir_r,
+    .seekdir      = proxy_seekdir,
+    .preadv       = proxy_preadv,
+    .pwritev      = proxy_pwritev,
+    .chmod        = proxy_chmod,
+    .mknod        = proxy_mknod,
+    .mkdir        = proxy_mkdir,
+    .fstat        = proxy_fstat,
+    .open2        = proxy_open2,
+    .symlink      = proxy_symlink,
+    .link         = proxy_link,
+    .truncate     = proxy_truncate,
+    .rename       = proxy_rename,
+    .chown        = proxy_chown,
+    .utimensat    = proxy_utimensat,
+    .remove       = proxy_remove,
+    .fsync        = proxy_fsync,
+    .statfs       = proxy_statfs,
+    .lgetxattr    = proxy_lgetxattr,
+    .llistxattr   = proxy_llistxattr,
+    .lsetxattr    = proxy_lsetxattr,
+    .lremovexattr = proxy_lremovexattr,
+    .name_to_path = proxy_name_to_path,
+    .renameat     = proxy_renameat,
+    .unlinkat     = proxy_unlinkat,
+};
diff --git a/hw/9pfs/virtio-9p-proxy.h b/hw/9pfs/virtio-9p-proxy.h
new file mode 100644
index 0000000..b3c59c7
--- /dev/null
+++ b/hw/9pfs/virtio-9p-proxy.h
@@ -0,0 +1,24 @@
+/*
+ * Virtio 9p Proxy callback
+ *
+ * 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.
+ */
+#ifndef _QEMU_VIRTIO_9P_PROXY_H
+#define _QEMU_VIRTIO_9P_PROXY_H
+
+#define PROXY_MAX_IO_SZ (64 * 1024)
+
+typedef struct {
+    int type;
+    int size;
+} ProxyHeader;
+
+#define PROXY_HDR_SZ (sizeof(ProxyHeader))
+
+#endif
diff --git a/qemu-config.c b/qemu-config.c
index 597d7e1..33367fe 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -183,6 +183,9 @@ QemuOptsList qemu_fsdev_opts = {
         }, {
             .name = "readonly",
             .type = QEMU_OPT_BOOL,
+        }, {
+            .name = "sock_fd",
+            .type = QEMU_OPT_NUMBER,
         },
 
         { /*End of list */ }
@@ -212,6 +215,9 @@ QemuOptsList qemu_virtfs_opts = {
         }, {
             .name = "readonly",
             .type = QEMU_OPT_BOOL,
+        }, {
+            .name = "sock_fd",
+            .type = QEMU_OPT_NUMBER,
         },
 
         { /*End of list */ }
diff --git a/vl.c b/vl.c
index 7eff214..fe43b4e 100644
--- a/vl.c
+++ b/vl.c
@@ -2669,7 +2669,7 @@ int main(int argc, char **argv, char **envp)
             case QEMU_OPTION_virtfs: {
                 QemuOpts *fsdev;
                 QemuOpts *device;
-                const char *writeout;
+                const char *writeout, *sock_fd;
 
                 olist = qemu_find_opts("virtfs");
                 if (!olist) {
@@ -2709,6 +2709,10 @@ int main(int argc, char **argv, char **envp)
                 qemu_opt_set(fsdev, "path", qemu_opt_get(opts, "path"));
                 qemu_opt_set(fsdev, "security_model",
                              qemu_opt_get(opts, "security_model"));
+                sock_fd = qemu_opt_get(opts, "sock_fd");
+                if (sock_fd) {
+                    qemu_opt_set(fsdev, "sock_fd", sock_fd);
+                }
 
                 qemu_opt_set_bool(fsdev, "readonly",
                                 qemu_opt_get_bool(opts, "readonly", 0));
-- 
1.7.6

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [Qemu-devel] [PATCH V4 04/13] hw/9pfs: File system helper process for qemu 9p proxy FS
  2011-12-05 16:18 [Qemu-devel] [PATCH V4 00/13] Proxy FS driver for VirtFS M. Mohan Kumar
                   ` (2 preceding siblings ...)
  2011-12-05 16:18 ` [Qemu-devel] [PATCH V4 03/13] hw/9pfs: Add new proxy filesystem driver M. Mohan Kumar
@ 2011-12-05 16:18 ` M. Mohan Kumar
  2011-12-08 18:31   ` Stefan Hajnoczi
  2011-12-05 16:18 ` [Qemu-devel] [PATCH V4 05/13] hw/9pfs: Open and create files M. Mohan Kumar
                   ` (9 subsequent siblings)
  13 siblings, 1 reply; 22+ messages in thread
From: M. Mohan Kumar @ 2011-12-05 16:18 UTC (permalink / raw)
  To: qemu-devel, aneesh.kumar, stefanha; +Cc: M. Mohan Kumar

From: "M. Mohan Kumar" <mohan@in.ibm.com>

Provide root privilege access to QEMU 9p proxy filesystem using socket
communication.

Proxy helper is started by root user as:
~ # virtfs-proxy-helper -f|--fd <socket descriptor> -p|--path <path-to-share>

Signed-off-by: M. Mohan Kumar <mohan@in.ibm.com>
---
 Makefile                    |    3 +
 configure                   |   19 +++
 fsdev/virtfs-proxy-helper.c |  308 +++++++++++++++++++++++++++++++++++++++++++
 hw/9pfs/virtio-9p-proxy.h   |    9 ++
 4 files changed, 339 insertions(+), 0 deletions(-)
 create mode 100644 fsdev/virtfs-proxy-helper.c

diff --git a/Makefile b/Makefile
index 301c75e..1906c5e 100644
--- a/Makefile
+++ b/Makefile
@@ -154,6 +154,9 @@ qemu-img$(EXESUF): qemu-img.o $(tools-obj-y) $(block-obj-y)
 qemu-nbd$(EXESUF): qemu-nbd.o $(tools-obj-y) $(block-obj-y)
 qemu-io$(EXESUF): qemu-io.o cmd.o $(tools-obj-y) $(block-obj-y)
 
+fsdev/virtfs-proxy-helper$(EXESUF): fsdev/virtfs-proxy-helper.o fsdev/virtio-9p-marshal.o
+fsdev/virtfs-proxy-helper$(EXESUF): LIBS += -lcap
+
 qemu-img-cmds.h: $(SRC_PATH)/qemu-img-cmds.hx
 	$(call quiet-command,sh $(SRC_PATH)/scripts/hxtool -h < $< > $@,"  GEN   $@")
 
diff --git a/configure b/configure
index ac4840d..4ecdb1c 100755
--- a/configure
+++ b/configure
@@ -1938,6 +1938,22 @@ else
 fi
 
 ##########################################
+# libcap probe
+
+if test "$cap" != "no" ; then
+  cat > $TMPC <<EOF
+#include <stdio.h>
+#include <sys/capability.h>
+int main(void) { cap_t caps; caps = cap_init(); }
+EOF
+  if compile_prog "" "-lcap" ; then
+    cap=yes
+  else
+    cap=no
+  fi
+fi
+
+##########################################
 # pthread probe
 PTHREADLIBS_LIST="-pthread -lpthread -lpthreadGC2"
 
@@ -2735,6 +2751,9 @@ confdir=$sysconfdir$confsuffix
 tools=
 if test "$softmmu" = yes ; then
   tools="qemu-img\$(EXESUF) qemu-io\$(EXESUF) $tools"
+  if [ "$cap" = "yes" -a "$linux" = "yes" ] ; then
+      tools="$tools fsdev/virtfs-proxy-helper\$(EXESUF)"
+  fi
   if [ "$linux" = "yes" -o "$bsd" = "yes" -o "$solaris" = "yes" ] ; then
       tools="qemu-nbd\$(EXESUF) $tools"
     if [ "$guest_agent" = "yes" ]; then
diff --git a/fsdev/virtfs-proxy-helper.c b/fsdev/virtfs-proxy-helper.c
new file mode 100644
index 0000000..7670a0e
--- /dev/null
+++ b/fsdev/virtfs-proxy-helper.c
@@ -0,0 +1,308 @@
+/*
+ * Helper for QEMU Proxy FS Driver
+ * 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 <stdio.h>
+#include <string.h>
+#include <sys/un.h>
+#include <limits.h>
+#include <signal.h>
+#include <errno.h>
+#include <stdlib.h>
+#include <sys/resource.h>
+#include <sys/stat.h>
+#include <getopt.h>
+#include <unistd.h>
+#include <syslog.h>
+#include <sys/capability.h>
+#include <sys/fsuid.h>
+#include <stdarg.h>
+#include <stdbool.h>
+#include "qemu-common.h"
+#include "virtio-9p-marshal.h"
+#include "hw/9pfs/virtio-9p-proxy.h"
+
+#define PROGNAME "virtfs-proxy-helper"
+
+static struct option helper_opts[] = {
+    {"fd", required_argument, NULL, 'f'},
+    {"path", required_argument, NULL, 'p'},
+    {"nodaemon", no_argument, NULL, 'n'},
+};
+
+static bool is_daemon;
+
+static void do_log(int loglevel, const char *format, ...)
+{
+    va_list ap;
+
+    va_start(ap, format);
+    if (is_daemon) {
+        vsyslog(LOG_CRIT, format, ap);
+    } else {
+        vfprintf(stderr, format, ap);
+    }
+    va_end(ap);
+}
+
+static void do_perror(const char *string)
+{
+    if (is_daemon) {
+        syslog(LOG_CRIT, "%s:%s", string, strerror(errno));
+    } else {
+        fprintf(stderr, "%s:%s\n", string, strerror(errno));
+    }
+}
+
+static int do_cap_set(cap_value_t *cap_value, int size, int reset)
+{
+    cap_t caps;
+    if (reset) {
+        /*
+         * Start with an empty set and set permitted and effective
+         */
+        caps = cap_init();
+        if (caps == NULL) {
+            do_perror("cap_init");
+            return -1;
+        }
+        if (cap_set_flag(caps, CAP_PERMITTED, size, cap_value, CAP_SET) < 0) {
+            do_perror("cap_set_flag");
+            goto error;
+        }
+    } else {
+        caps = cap_get_proc();
+        if (!caps) {
+            do_perror("cap_get_proc");
+            return -1;
+        }
+    }
+    if (cap_set_flag(caps, CAP_EFFECTIVE, size, cap_value, CAP_SET) < 0) {
+        do_perror("cap_set_flag");
+        goto error;
+    }
+    if (cap_set_proc(caps) < 0) {
+        do_perror("cap_set_proc");
+        goto error;
+    }
+    cap_free(caps);
+    return 0;
+
+error:
+    cap_free(caps);
+    return -1;
+}
+
+static int init_capabilities(void)
+{
+    /* helper needs following capbabilities only */
+    cap_value_t cap_list[] = {
+        CAP_CHOWN,
+        CAP_DAC_OVERRIDE,
+        CAP_FOWNER,
+        CAP_FSETID,
+        CAP_SETGID,
+        CAP_MKNOD,
+        CAP_SETUID,
+    };
+    return do_cap_set(cap_list, ARRAY_SIZE(cap_list), 1);
+}
+
+static int socket_read(int sockfd, void *buff, ssize_t size)
+{
+    ssize_t retval, total = 0;
+
+    while (size) {
+        retval = read(sockfd, buff, size);
+        if (retval == 0) {
+            return -EIO;
+        }
+        if (retval < 0) {
+            if (errno == EINTR) {
+                continue;
+            }
+            return -errno;
+        }
+        size -= retval;
+        buff += retval;
+        total += retval;
+    }
+    return total;
+}
+
+static int socket_write(int sockfd, void *buff, ssize_t size)
+{
+    ssize_t retval, total = 0;
+
+    while (size) {
+        retval = write(sockfd, buff, size);
+        if (retval < 0) {
+            if (errno == EINTR) {
+                continue;
+            }
+            return -errno;
+        }
+        size -= retval;
+        buff += retval;
+        total += retval;
+    }
+    return total;
+}
+
+static int read_request(int sockfd, struct iovec *iovec, ProxyHeader *header)
+{
+    int retval;
+
+    /*
+     * read the request header.
+     */
+    iovec->iov_len = 0;
+    retval = socket_read(sockfd, iovec->iov_base, PROXY_HDR_SZ);
+    if (retval < 0) {
+        return retval;
+    }
+    iovec->iov_len = PROXY_HDR_SZ;
+    retval = proxy_unmarshal(iovec, 0, "dd", &header->type, &header->size);
+    if (retval < 0) {
+        return retval;
+    }
+    /*
+     * We can't process message.size > PROXY_MAX_IO_SZ, read the complete
+     * message from the socket and ignore it. This ensures that
+     * we can correctly handle the next request. We also return
+     * ENOBUFS as error to indicate we ran out of buffer space.
+     */
+    if (header->size > PROXY_MAX_IO_SZ) {
+        int count, size;
+        size = header->size;
+        while (size > 0) {
+            count = MIN(PROXY_MAX_IO_SZ, size);
+            count = socket_read(sockfd, iovec->iov_base + PROXY_HDR_SZ, count);
+            if (count < 0) {
+                return count;
+            }
+            size -= count;
+        }
+        return -ENOBUFS;
+    }
+    retval = socket_read(sockfd, iovec->iov_base + PROXY_HDR_SZ, header->size);
+    if (retval < 0) {
+        return retval;
+    }
+    iovec->iov_len += header->size;
+    return 0;
+}
+
+static void usage(char *prog)
+{
+    fprintf(stderr, "usage: %s\n"
+            " -p|--path <path> 9p path to export\n"
+            " {-f|--fd <socket-descriptor>} socket file descriptor to be used\n"
+            " [-n|--nodaemon] Run as a normal program\n",
+            basename(prog));
+}
+
+static int process_requests(int sock)
+{
+    int retval;
+    ProxyHeader header;
+    struct iovec in_iovec;
+
+    in_iovec.iov_base = g_malloc(PROXY_MAX_IO_SZ + PROXY_HDR_SZ);
+    in_iovec.iov_len  = PROXY_MAX_IO_SZ + PROXY_HDR_SZ;
+    while (1) {
+        retval = read_request(sock, &in_iovec, &header);
+        if (retval < 0) {
+            goto error;
+        }
+    }
+    (void)socket_write;
+error:
+    g_free(in_iovec.iov_base);
+    return -1;
+}
+
+int main(int argc, char **argv)
+{
+    int sock;
+    char *rpath = NULL;
+    struct stat stbuf;
+    int c, option_index;
+
+    is_daemon = true;
+    sock = -1;
+    while (1) {
+        option_index = 0;
+        c = getopt_long(argc, argv, "p:nh?f:", helper_opts,
+                        &option_index);
+        if (c == -1) {
+            break;
+        }
+        switch (c) {
+        case 'p':
+            rpath = strdup(optarg);
+            break;
+        case 'n':
+            is_daemon = false;
+            break;
+        case 'f':
+            sock = atoi(optarg);
+            break;
+        case '?':
+        case 'h':
+        default:
+            usage(argv[0]);
+            exit(EXIT_FAILURE);
+        }
+    }
+
+    /* Parameter validation */
+    if (sock == -1 || rpath == NULL) {
+        fprintf(stderr, "socket descriptor or path not specified\n");
+        usage(argv[0]);
+        exit(EXIT_FAILURE);
+    }
+
+    if (lstat(rpath, &stbuf) < 0) {
+        fprintf(stderr, "invalid path \"%s\" specified, %s\n",
+                rpath, strerror(errno));
+        exit(EXIT_FAILURE);
+    }
+
+    if (!S_ISDIR(stbuf.st_mode)) {
+        fprintf(stderr, "specified path \"%s\" is not directory\n", rpath);
+        exit(EXIT_FAILURE);
+    }
+
+    if (is_daemon) {
+        if (daemon(0, 0) < 0) {
+            fprintf(stderr, "daemon call failed\n");
+            exit(EXIT_FAILURE);
+        }
+        openlog(PROGNAME, LOG_PID, LOG_DAEMON);
+    }
+
+    do_log(LOG_INFO, "Started\n");
+
+    if (chroot(rpath) < 0) {
+        do_perror("chroot");
+        goto error;
+    }
+    umask(0);
+
+    if (init_capabilities() < 0) {
+        goto error;
+    }
+
+    process_requests(sock);
+error:
+    do_log(LOG_INFO, "Done\n");
+    closelog();
+    return 0;
+}
diff --git a/hw/9pfs/virtio-9p-proxy.h b/hw/9pfs/virtio-9p-proxy.h
index b3c59c7..c33a84b 100644
--- a/hw/9pfs/virtio-9p-proxy.h
+++ b/hw/9pfs/virtio-9p-proxy.h
@@ -14,6 +14,15 @@
 
 #define PROXY_MAX_IO_SZ (64 * 1024)
 
+/*
+ * proxy iovec only support one element and
+ * marsha/unmarshal doesn't do little endian conversion.
+ */
+#define proxy_unmarshal(in_sg, offset, fmt, args...) \
+    v9fs_unmarshal(in_sg, 1, offset, 0, fmt, ##args)
+#define proxy_marshal(out_sg, offset, fmt, args...) \
+    v9fs_marshal(out_sg, 1, offset, 0, fmt, ##args)
+
 typedef struct {
     int type;
     int size;
-- 
1.7.6

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [Qemu-devel] [PATCH V4 05/13] hw/9pfs: Open and create files
  2011-12-05 16:18 [Qemu-devel] [PATCH V4 00/13] Proxy FS driver for VirtFS M. Mohan Kumar
                   ` (3 preceding siblings ...)
  2011-12-05 16:18 ` [Qemu-devel] [PATCH V4 04/13] hw/9pfs: File system helper process for qemu 9p proxy FS M. Mohan Kumar
@ 2011-12-05 16:18 ` M. Mohan Kumar
  2011-12-05 16:18 ` [Qemu-devel] [PATCH V4 06/13] hw/9pfs: Create other filesystem objects M. Mohan Kumar
                   ` (8 subsequent siblings)
  13 siblings, 0 replies; 22+ messages in thread
From: M. Mohan Kumar @ 2011-12-05 16:18 UTC (permalink / raw)
  To: qemu-devel, aneesh.kumar, stefanha; +Cc: M. Mohan Kumar

From: "M. Mohan Kumar" <mohan@in.ibm.com>

Add interfaces to open and create files for proxy file system driver.

Signed-off-by: M. Mohan Kumar <mohan@in.ibm.com>
---
 fsdev/virtfs-proxy-helper.c |  191 ++++++++++++++++++++++++++++++++++++++++++-
 hw/9pfs/virtio-9p-proxy.c   |  189 +++++++++++++++++++++++++++++++++++++++++--
 hw/9pfs/virtio-9p-proxy.h   |   11 +++
 3 files changed, 382 insertions(+), 9 deletions(-)

diff --git a/fsdev/virtfs-proxy-helper.c b/fsdev/virtfs-proxy-helper.c
index 7670a0e..241fcff 100644
--- a/fsdev/virtfs-proxy-helper.c
+++ b/fsdev/virtfs-proxy-helper.c
@@ -9,6 +9,7 @@
  * the COPYING file in the top-level directory.
  */
 #include <stdio.h>
+#include <sys/socket.h>
 #include <string.h>
 #include <sys/un.h>
 #include <limits.h>
@@ -27,6 +28,7 @@
 #include "qemu-common.h"
 #include "virtio-9p-marshal.h"
 #include "hw/9pfs/virtio-9p-proxy.h"
+#include "fsdev/virtio-9p-marshal.h"
 
 #define PROGNAME "virtfs-proxy-helper"
 
@@ -199,6 +201,139 @@ static int read_request(int sockfd, struct iovec *iovec, ProxyHeader *header)
     return 0;
 }
 
+static int send_fd(int sockfd, int fd)
+{
+    struct msghdr msg;
+    struct iovec iov;
+    int retval, data;
+    struct cmsghdr *cmsg;
+    union MsgControl msg_control;
+
+    iov.iov_base = &data;
+    iov.iov_len = sizeof(data);
+
+    memset(&msg, 0, sizeof(msg));
+    msg.msg_iov = &iov;
+    msg.msg_iovlen = 1;
+    /* No ancillary data on error */
+    if (fd < 0) {
+        /* fd is really negative errno if the request failed  */
+        data = fd;
+    } else {
+        data = V9FS_FD_VALID;
+        msg.msg_control = &msg_control;
+        msg.msg_controllen = sizeof(msg_control);
+
+        cmsg = &msg_control.cmsg;
+        cmsg->cmsg_len = CMSG_LEN(sizeof(fd));
+        cmsg->cmsg_level = SOL_SOCKET;
+        cmsg->cmsg_type = SCM_RIGHTS;
+        memcpy(CMSG_DATA(cmsg), &fd, sizeof(fd));
+    }
+
+    do {
+        retval = sendmsg(sockfd, &msg, 0);
+    } while (retval < 0 && errno == EINTR);
+    if (fd >= 0) {
+        close(fd);
+    }
+    if (retval < 0) {
+        return retval;
+    }
+    return 0;
+}
+
+/*
+ * from man 7 capabilities, section
+ * Effect of User ID Changes on Capabilities:
+ * 4. If the file system user ID is changed from 0 to nonzero (see setfsuid(2))
+ * then the following capabilities are cleared from the effective set:
+ * CAP_CHOWN, CAP_DAC_OVERRIDE, CAP_DAC_READ_SEARCH,  CAP_FOWNER, CAP_FSETID,
+ * CAP_LINUX_IMMUTABLE  (since  Linux 2.2.30), CAP_MAC_OVERRIDE, and CAP_MKNOD
+ * (since Linux 2.2.30). If the file system UID is changed from nonzero to 0,
+ * then any of these capabilities that are enabled in the permitted set
+ * are enabled in the effective set.
+ */
+static int setfsugid(int uid, int gid)
+{
+    /*
+     * We still need DAC_OVERRIDE because  we don't change
+     * supplementary group ids, and hence may be subjected DAC rules
+     */
+    cap_value_t cap_list[] = {
+        CAP_DAC_OVERRIDE,
+    };
+
+    setfsgid(gid);
+    setfsuid(uid);
+
+    if (uid != 0 || gid != 0) {
+        return do_cap_set(cap_list, ARRAY_SIZE(cap_list), 0);
+    }
+    return 0;
+}
+
+/*
+ * create a file and send fd on success
+ * return -errno on error
+ */
+static int do_create(struct iovec *iovec)
+{
+    int ret;
+    V9fsString path;
+    int flags, mode, uid, gid, cur_uid, cur_gid;
+
+    v9fs_string_init(&path);
+    ret = proxy_unmarshal(iovec, PROXY_HDR_SZ, "sdddd",
+                          &path, &flags, &mode, &uid, &gid);
+    if (ret < 0) {
+        goto err_out;
+    }
+    cur_uid = geteuid();
+    cur_gid = getegid();
+    ret = setfsugid(uid, gid);
+    if (ret < 0) {
+        /*
+         * On failure reset back to the
+         * old uid/gid
+         */
+        ret = -errno;
+        goto err_out;
+    }
+    ret = open(path.data, flags, mode);
+    if (ret < 0) {
+        ret = -errno;
+    }
+
+err_out:
+    setfsugid(cur_uid, cur_gid);
+    v9fs_string_free(&path);
+    return ret;
+}
+
+/*
+ * open a file and send fd on success
+ * return -errno on error
+ */
+static int do_open(struct iovec *iovec)
+{
+    int flags, ret;
+    V9fsString path;
+
+    v9fs_string_init(&path);
+    ret = proxy_unmarshal(iovec, PROXY_HDR_SZ, "sd", &path, &flags);
+    if (ret < 0) {
+        goto err_out;
+    }
+    ret = open(path.data, flags);
+    if (ret < 0) {
+        ret = -errno;
+    }
+err_out:
+    v9fs_string_free(&path);
+    return ret;
+}
+
 static void usage(char *prog)
 {
     fprintf(stderr, "usage: %s\n"
@@ -208,22 +343,72 @@ static void usage(char *prog)
             basename(prog));
 }
 
+static int process_reply(int sock, int type, int retval)
+{
+    switch (type) {
+    case T_OPEN:
+    case T_CREATE:
+        if (send_fd(sock, retval) < 0) {
+            return -1;
+        }
+        break;
+    default:
+        return -1;
+        break;
+    }
+    return 0;
+}
+
 static int process_requests(int sock)
 {
-    int retval;
+    int retval = 0;
     ProxyHeader header;
     struct iovec in_iovec;
 
     in_iovec.iov_base = g_malloc(PROXY_MAX_IO_SZ + PROXY_HDR_SZ);
     in_iovec.iov_len  = PROXY_MAX_IO_SZ + PROXY_HDR_SZ;
     while (1) {
+        /*
+         * initialize the header type, so that we send
+         * response to proper request type.
+         */
+        header.type = 0;
         retval = read_request(sock, &in_iovec, &header);
         if (retval < 0) {
-            goto error;
+            if (retval == -ENOBUFS) {
+                /*
+                 * The request size was too long, we don't
+                 * terminate for such errors. We will have
+                 * header.type update properly when we reach
+                 * here.
+                 */
+                if (process_reply(sock, header.type, retval) < 0) {
+                    goto err_out;
+                }
+                continue;
+            } else {
+                goto err_out;
+            }
+        }
+
+        switch (header.type) {
+        case T_OPEN:
+            retval = do_open(&in_iovec);
+            break;
+        case T_CREATE:
+            retval = do_create(&in_iovec);
+            break;
+        default:
+            goto err_out;
+            break;
+        }
+
+        if (process_reply(sock, header.type, retval) < 0) {
+            goto err_out;
         }
     }
     (void)socket_write;
-error:
+err_out:
     g_free(in_iovec.iov_base);
     return -1;
 }
diff --git a/hw/9pfs/virtio-9p-proxy.c b/hw/9pfs/virtio-9p-proxy.c
index 3e4dc68..6a74666 100644
--- a/hw/9pfs/virtio-9p-proxy.c
+++ b/hw/9pfs/virtio-9p-proxy.c
@@ -22,6 +22,155 @@ typedef struct V9fsProxy {
     struct iovec iovec;
 } V9fsProxy;
 
+/*
+ * Return received file descriptor on success in *status.
+ * errno is also returned on *status (which will be < 0)
+ * return < 0 on transport error.
+ */
+static int v9fs_receivefd(int sockfd, int *status)
+{
+    struct iovec iov;
+    struct msghdr msg;
+    struct cmsghdr *cmsg;
+    int retval, data, fd;
+    union MsgControl msg_control;
+
+    iov.iov_base = &data;
+    iov.iov_len = sizeof(data);
+
+    memset(&msg, 0, sizeof(msg));
+    msg.msg_iov = &iov;
+    msg.msg_iovlen = 1;
+    msg.msg_control = &msg_control;
+    msg.msg_controllen = sizeof(msg_control);
+
+    do {
+        retval = recvmsg(sockfd, &msg, 0);
+    } while (retval < 0 && errno == EINTR);
+    if (retval <= 0) {
+        return retval;
+    }
+    /*
+     * data is set to V9FS_FD_VALID, if ancillary data is sent.  If this
+     * request doesn't need ancillary data (fd) or an error occurred,
+     * data is set to negative errno value.
+     */
+    if (data != V9FS_FD_VALID) {
+        *status = data;
+        return 0;
+    }
+    /*
+     * File descriptor (fd) is sent in the ancillary data. Check if we
+     * indeed received it. One of the reasons to fail to receive it is if
+     * we exceeded the maximum number of file descriptors!
+     */
+    for (cmsg = CMSG_FIRSTHDR(&msg); cmsg; cmsg = CMSG_NXTHDR(&msg, cmsg)) {
+        if (cmsg->cmsg_len != CMSG_LEN(sizeof(int)) ||
+            cmsg->cmsg_level != SOL_SOCKET ||
+            cmsg->cmsg_type != SCM_RIGHTS) {
+            continue;
+        }
+        fd = *((int *)CMSG_DATA(cmsg));
+        *status = fd;
+        return 0;
+    }
+    *status = -ENFILE;  /* Ancillary data sent but not received */
+    return 0;
+}
+
+/*
+ * Proxy->header and proxy->request written to socket by QEMU process.
+ * This request read by proxy helper process
+ * returns 0 on success and -errno on error
+ */
+static int v9fs_request(V9fsProxy *proxy, int type,
+                        void *response, const char *fmt, ...)
+{
+    va_list ap;
+    int retval = 0;
+    V9fsString *path;
+    ProxyHeader header;
+    struct iovec *iovec = NULL;
+    int flags, mode, uid, gid;
+
+    qemu_mutex_lock(&proxy->mutex);
+
+    if (proxy->sockfd == -1) {
+        retval = -EIO;
+        goto err_out;
+    }
+    iovec = &proxy->iovec;
+
+    va_start(ap, fmt);
+    switch (type) {
+    case T_OPEN:
+        path = va_arg(ap, V9fsString *);
+        flags = va_arg(ap, int);
+        header.size = proxy_marshal(iovec, PROXY_HDR_SZ, "sd", path, flags);
+        if (header.size < 0) {
+            retval = header.size;
+            break;
+        }
+        header.type = T_OPEN;
+        break;
+    case T_CREATE:
+        path = va_arg(ap, V9fsString *);
+        flags = va_arg(ap, int);
+        mode = va_arg(ap, int);
+        uid = va_arg(ap, int);
+        gid = va_arg(ap, int);
+        header.size = proxy_marshal(iovec, PROXY_HDR_SZ, "sdddd", path,
+                                    flags, mode, uid, gid);
+        if (header.size < 0) {
+            retval = header.size;
+            break;
+        }
+        header.type = T_CREATE;
+        break;
+    default:
+        error_report("Invalid type %d\n", type);
+        retval = -EINVAL;
+        break;
+    }
+    va_end(ap);
+
+    if (retval < 0) {
+        goto err_out;
+    }
+
+    /* marshal the header details */
+    proxy_marshal(iovec, 0, "dd", header.type, header.size);
+    header.size += PROXY_HDR_SZ;
+
+    retval = qemu_write_full(proxy->sockfd, iovec->iov_base, header.size);
+    if (retval != header.size) {
+        goto close_error;
+    }
+
+    switch (type) {
+    case T_OPEN:
+    case T_CREATE:
+        /*
+         * A file descriptor is returned as response for
+         * T_OPEN,T_CREATE on success
+         */
+        if (v9fs_receivefd(proxy->sockfd, &retval) < 0) {
+            goto close_error;
+        }
+        break;
+    }
+
+err_out:
+    qemu_mutex_unlock(&proxy->mutex);
+    return retval;
+
+close_error:
+    close(proxy->sockfd);
+    proxy->sockfd = -1;
+    qemu_mutex_unlock(&proxy->mutex);
+    return -EIO;
+}
+
 static int proxy_lstat(FsContext *fs_ctx, V9fsPath *fs_path, struct stat *stbuf)
 {
     errno = EOPNOTSUPP;
@@ -48,16 +197,33 @@ static int proxy_closedir(FsContext *ctx, V9fsFidOpenState *fs)
 static int proxy_open(FsContext *ctx, V9fsPath *fs_path,
                       int flags, V9fsFidOpenState *fs)
 {
-    fs->fd = -1;
+    fs->fd = v9fs_request(ctx->private, T_OPEN, NULL, "sd", fs_path, flags);
+    if (fs->fd < 0) {
+        errno = -fs->fd;
+        fs->fd = -1;
+    }
     return fs->fd;
 }
 
 static int proxy_opendir(FsContext *ctx,
                          V9fsPath *fs_path, V9fsFidOpenState *fs)
 {
+    int serrno, fd;
+
     fs->dir = NULL;
-    errno = EOPNOTSUPP;
-    return -1;
+    fd = v9fs_request(ctx->private, T_OPEN, NULL, "sd", fs_path, O_DIRECTORY);
+    if (fd < 0) {
+        errno = -fd;
+        return -1;
+    }
+    fs->dir = fdopendir(fd);
+    if (!fs->dir) {
+        serrno = errno;
+        close(fd);
+        errno = serrno;
+        return -1;
+    }
+    return 0;
 }
 
 static void proxy_rewinddir(FsContext *ctx, V9fsFidOpenState *fs)
@@ -164,9 +330,20 @@ static int proxy_fstat(FsContext *fs_ctx, int fid_type,
 static int proxy_open2(FsContext *fs_ctx, V9fsPath *dir_path, const char *name,
                        int flags, FsCred *credp, V9fsFidOpenState *fs)
 {
-    fs->fd = -1;
-    errno = EOPNOTSUPP;
-    return -1;
+    V9fsString fullname;
+
+    v9fs_string_init(&fullname);
+    v9fs_string_sprintf(&fullname, "%s/%s", dir_path->data, name);
+
+    fs->fd = v9fs_request(fs_ctx->private, T_CREATE, NULL, "sdddd",
+                          &fullname, flags, credp->fc_mode,
+                          credp->fc_uid, credp->fc_gid);
+    v9fs_string_free(&fullname);
+    if (fs->fd < 0) {
+        errno = -fs->fd;
+        fs->fd = -1;
+    }
+    return fs->fd;
 }
 
 
diff --git a/hw/9pfs/virtio-9p-proxy.h b/hw/9pfs/virtio-9p-proxy.h
index c33a84b..b8719ea 100644
--- a/hw/9pfs/virtio-9p-proxy.h
+++ b/hw/9pfs/virtio-9p-proxy.h
@@ -13,6 +13,7 @@
 #define _QEMU_VIRTIO_9P_PROXY_H
 
 #define PROXY_MAX_IO_SZ (64 * 1024)
+#define V9FS_FD_VALID INT_MAX
 
 /*
  * proxy iovec only support one element and
@@ -23,6 +24,11 @@
 #define proxy_marshal(out_sg, offset, fmt, args...) \
     v9fs_marshal(out_sg, 1, offset, 0, fmt, ##args)
 
+union MsgControl {
+    struct cmsghdr cmsg;
+    char control[CMSG_SPACE(sizeof(int))];
+};
+
 typedef struct {
     int type;
     int size;
@@ -30,4 +36,9 @@ typedef struct {
 
 #define PROXY_HDR_SZ (sizeof(ProxyHeader))
 
+enum {
+    T_OPEN = 1,
+    T_CREATE,
+};
+
 #endif
-- 
1.7.6

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [Qemu-devel] [PATCH V4 06/13] hw/9pfs: Create other filesystem objects
  2011-12-05 16:18 [Qemu-devel] [PATCH V4 00/13] Proxy FS driver for VirtFS M. Mohan Kumar
                   ` (4 preceding siblings ...)
  2011-12-05 16:18 ` [Qemu-devel] [PATCH V4 05/13] hw/9pfs: Open and create files M. Mohan Kumar
@ 2011-12-05 16:18 ` M. Mohan Kumar
  2011-12-05 16:18 ` [Qemu-devel] [PATCH V4 07/13] hw/9pfs: Add stat/readlink/statfs for proxy FS M. Mohan Kumar
                   ` (7 subsequent siblings)
  13 siblings, 0 replies; 22+ messages in thread
From: M. Mohan Kumar @ 2011-12-05 16:18 UTC (permalink / raw)
  To: qemu-devel, aneesh.kumar, stefanha; +Cc: M. Mohan Kumar

From: "M. Mohan Kumar" <mohan@in.ibm.com>

Add interfaces to create filesystem objects like directory,
device nodes, symbolic links, links for proxy filesytem driver

Signed-off-by: M. Mohan Kumar <mohan@in.ibm.com>
---
 fsdev/virtfs-proxy-helper.c |  131 ++++++++++++++++++++++++++--
 hw/9pfs/virtio-9p-proxy.c   |  203 +++++++++++++++++++++++++++++++++++++++----
 hw/9pfs/virtio-9p-proxy.h   |    8 ++-
 3 files changed, 318 insertions(+), 24 deletions(-)

diff --git a/fsdev/virtfs-proxy-helper.c b/fsdev/virtfs-proxy-helper.c
index 241fcff..729c79c 100644
--- a/fsdev/virtfs-proxy-helper.c
+++ b/fsdev/virtfs-proxy-helper.c
@@ -243,6 +243,30 @@ static int send_fd(int sockfd, int fd)
     return 0;
 }
 
+static int send_status(int sockfd, struct iovec *iovec, int status)
+{
+    ProxyHeader header;
+    int retval, msg_size;;
+
+    if (status < 0) {
+        header.type = T_ERROR;
+    } else {
+        header.type = T_SUCCESS;
+    }
+    header.size = sizeof(status);
+    /*
+     * marshal the return status. We don't check error.
+     * because we are sure we have enough space for the status
+     */
+    msg_size = proxy_marshal(iovec, 0, "ddd", header.type,
+                             header.size, status);
+    retval = socket_write(sockfd, iovec->iov_base, msg_size);
+    if (retval < 0) {
+        return retval;
+    }
+    return 0;
+}
+
 /*
  * from man 7 capabilities, section
  * Effect of User ID Changes on Capabilities:
@@ -274,6 +298,67 @@ static int setfsugid(int uid, int gid)
 }
 
 /*
+ * create other filesystem objects and send 0 on success
+ * return -errno on error
+ */
+static int do_create_others(int type, struct iovec *iovec)
+{
+    dev_t rdev;
+    int retval = 0;
+    int offset = PROXY_HDR_SZ;
+    V9fsString oldpath, path;
+    int mode, uid, gid, cur_uid, cur_gid;
+
+    v9fs_string_init(&path);
+    v9fs_string_init(&oldpath);
+    cur_uid = geteuid();
+    cur_gid = getegid();
+
+    retval = proxy_unmarshal(iovec, offset, "dd", &uid, &gid);
+    if (retval < 0) {
+        return retval;
+    }
+    offset += retval;
+    retval = setfsugid(uid, gid);
+    if (retval < 0) {
+        retval = -errno;
+        goto err_out;
+    }
+    switch (type) {
+    case T_MKNOD:
+        retval = proxy_unmarshal(iovec, offset, "sdq", &path, &mode, &rdev);
+        if (retval < 0) {
+            goto err_out;
+        }
+        retval = mknod(path.data, mode, rdev);
+        break;
+    case T_MKDIR:
+        retval = proxy_unmarshal(iovec, offset, "sd", &path, &mode);
+        if (retval < 0) {
+            goto err_out;
+        }
+        retval = mkdir(path.data, mode);
+        break;
+    case T_SYMLINK:
+        retval = proxy_unmarshal(iovec, offset, "ss", &oldpath, &path);
+        if (retval < 0) {
+            goto err_out;
+        }
+        retval = symlink(oldpath.data, path.data);
+        break;
+    }
+    if (retval < 0) {
+        retval = -errno;
+    }
+
+err_out:
+    v9fs_string_free(&path);
+    v9fs_string_free(&oldpath);
+    setfsugid(cur_uid, cur_gid);
+    return retval;
+}
+
+/*
  * create a file and send fd on success
  * return -errno on error
  */
@@ -343,7 +428,8 @@ static void usage(char *prog)
             basename(prog));
 }
 
-static int process_reply(int sock, int type, int retval)
+static int process_reply(int sock, int type,
+                         struct iovec *out_iovec, int retval)
 {
     switch (type) {
     case T_OPEN:
@@ -352,6 +438,14 @@ static int process_reply(int sock, int type, int retval)
             return -1;
         }
         break;
+    case T_MKNOD:
+    case T_MKDIR:
+    case T_SYMLINK:
+    case T_LINK:
+        if (send_status(sock, out_iovec, retval) < 0) {
+            return -1;
+        }
+        break;
     default:
         return -1;
         break;
@@ -363,10 +457,14 @@ static int process_requests(int sock)
 {
     int retval = 0;
     ProxyHeader header;
-    struct iovec in_iovec;
+    V9fsString oldpath, path;
+    struct iovec in_iovec, out_iovec;
+
+    in_iovec.iov_base  = g_malloc(PROXY_MAX_IO_SZ + PROXY_HDR_SZ);
+    in_iovec.iov_len   = PROXY_MAX_IO_SZ + PROXY_HDR_SZ;
+    out_iovec.iov_base = g_malloc(PROXY_MAX_IO_SZ + PROXY_HDR_SZ);
+    out_iovec.iov_len  = PROXY_MAX_IO_SZ + PROXY_HDR_SZ;
 
-    in_iovec.iov_base = g_malloc(PROXY_MAX_IO_SZ + PROXY_HDR_SZ);
-    in_iovec.iov_len  = PROXY_MAX_IO_SZ + PROXY_HDR_SZ;
     while (1) {
         /*
          * initialize the header type, so that we send
@@ -382,7 +480,7 @@ static int process_requests(int sock)
                  * header.type update properly when we reach
                  * here.
                  */
-                if (process_reply(sock, header.type, retval) < 0) {
+                if (process_reply(sock, header.type, &out_iovec, retval) < 0) {
                     goto err_out;
                 }
                 continue;
@@ -398,18 +496,37 @@ static int process_requests(int sock)
         case T_CREATE:
             retval = do_create(&in_iovec);
             break;
+        case T_MKNOD:
+        case T_MKDIR:
+        case T_SYMLINK:
+            retval = do_create_others(header.type, &in_iovec);
+            break;
+        case T_LINK:
+            v9fs_string_init(&path);
+            v9fs_string_init(&oldpath);
+            retval = proxy_unmarshal(&in_iovec, PROXY_HDR_SZ,
+                                     "ss", &oldpath, &path);
+            if (retval > 0) {
+                retval = link(oldpath.data, path.data);
+                if (retval < 0) {
+                    retval = -errno;
+                }
+            }
+            v9fs_string_free(&oldpath);
+            v9fs_string_free(&path);
+            break;
         default:
             goto err_out;
             break;
         }
 
-        if (process_reply(sock, header.type, retval) < 0) {
+        if (process_reply(sock, header.type, &out_iovec, retval) < 0) {
             goto err_out;
         }
     }
-    (void)socket_write;
 err_out:
     g_free(in_iovec.iov_base);
+    g_free(out_iovec.iov_base);
     return -1;
 }
 
diff --git a/hw/9pfs/virtio-9p-proxy.c b/hw/9pfs/virtio-9p-proxy.c
index 6a74666..da6d7f9 100644
--- a/hw/9pfs/virtio-9p-proxy.c
+++ b/hw/9pfs/virtio-9p-proxy.c
@@ -19,7 +19,8 @@
 typedef struct V9fsProxy {
     int sockfd;
     QemuMutex mutex;
-    struct iovec iovec;
+    struct iovec in_iovec;
+    struct iovec out_iovec;
 } V9fsProxy;
 
 /*
@@ -78,6 +79,60 @@ static int v9fs_receivefd(int sockfd, int *status)
     return 0;
 }
 
+static ssize_t socket_read(int sockfd, void *buff, size_t size)
+{
+    ssize_t retval, total = 0;
+
+    while (size) {
+        retval = read(sockfd, buff, size);
+        if (retval == 0) {
+            return -EIO;
+        }
+        if (retval < 0) {
+            if (errno == EINTR) {
+                continue;
+            }
+            return -errno;
+        }
+        size -= retval;
+        buff += retval;
+        total += retval;
+    }
+    return total;
+}
+
+/*
+ * return < 0 on transport error.
+ * *status is valid only if return >= 0
+ */
+static int v9fs_receive_status(V9fsProxy *proxy,
+                               struct iovec *reply, int *status)
+{
+    int retval;
+    ProxyHeader header;
+
+    *status = 0;
+    reply->iov_len = 0;
+    retval = socket_read(proxy->sockfd, reply->iov_base, PROXY_HDR_SZ);
+    if (retval < 0) {
+        return retval;
+    }
+    reply->iov_len = PROXY_HDR_SZ;
+    proxy_unmarshal(reply, 0, "dd", &header.type, &header.size);
+    if (header.size != sizeof(int)) {
+        *status = -ENOBUFS;
+        return 0;
+    }
+    retval = socket_read(proxy->sockfd,
+                         reply->iov_base + PROXY_HDR_SZ, header.size);
+    if (retval < 0) {
+        return retval;
+    }
+    reply->iov_len += header.size;
+    proxy_unmarshal(reply, PROXY_HDR_SZ, "d", status);
+    return 0;
+}
+
 /*
  * Proxy->header and proxy->request written to socket by QEMU process.
  * This request read by proxy helper process
@@ -86,12 +141,13 @@ static int v9fs_receivefd(int sockfd, int *status)
 static int v9fs_request(V9fsProxy *proxy, int type,
                         void *response, const char *fmt, ...)
 {
+    dev_t rdev;
     va_list ap;
     int retval = 0;
-    V9fsString *path;
     ProxyHeader header;
-    struct iovec *iovec = NULL;
     int flags, mode, uid, gid;
+    V9fsString *path, *oldpath;
+    struct iovec *iovec = NULL, *reply = NULL;
 
     qemu_mutex_lock(&proxy->mutex);
 
@@ -99,8 +155,8 @@ static int v9fs_request(V9fsProxy *proxy, int type,
         retval = -EIO;
         goto err_out;
     }
-    iovec = &proxy->iovec;
-
+    iovec = &proxy->out_iovec;
+    reply = &proxy->in_iovec;
     va_start(ap, fmt);
     switch (type) {
     case T_OPEN:
@@ -127,6 +183,57 @@ static int v9fs_request(V9fsProxy *proxy, int type,
         }
         header.type = T_CREATE;
         break;
+    case T_MKNOD:
+        path = va_arg(ap, V9fsString *);
+        mode = va_arg(ap, int);
+        rdev = va_arg(ap, long int);
+        uid = va_arg(ap, int);
+        gid = va_arg(ap, int);
+        header.size = proxy_marshal(iovec, PROXY_HDR_SZ, "ddsdq",
+                                    uid, gid, path, mode, rdev);
+        if (header.size < 0) {
+            retval = header.size;
+            break;
+        }
+        header.type = T_MKNOD;
+        break;
+    case T_MKDIR:
+        path = va_arg(ap, V9fsString *);
+        mode = va_arg(ap, int);
+        uid = va_arg(ap, int);
+        gid = va_arg(ap, int);
+        header.size = proxy_marshal(iovec, PROXY_HDR_SZ, "ddsd",
+                                    uid, gid, path, mode);
+        if (header.size < 0) {
+            retval = header.size;
+            break;
+        }
+        header.type = T_MKDIR;
+        break;
+    case T_SYMLINK:
+        oldpath = va_arg(ap, V9fsString *);
+        path = va_arg(ap, V9fsString *);
+        uid = va_arg(ap, int);
+        gid = va_arg(ap, int);
+        header.size = proxy_marshal(iovec, PROXY_HDR_SZ, "ddss",
+                                    uid, gid, oldpath, path);
+        if (header.size < 0) {
+            retval = header.size;
+            break;
+        }
+        header.type = T_SYMLINK;
+        break;
+    case T_LINK:
+        oldpath = va_arg(ap, V9fsString *);
+        path = va_arg(ap, V9fsString *);
+        header.size = proxy_marshal(iovec, PROXY_HDR_SZ, "ss",
+                                    oldpath, path);
+        if (header.size < 0) {
+            retval = header.size;
+            break;
+        }
+        header.type = T_LINK;
+        break;
     default:
         error_report("Invalid type %d\n", type);
         retval = -EINVAL;
@@ -158,6 +265,14 @@ static int v9fs_request(V9fsProxy *proxy, int type,
             goto close_error;
         }
         break;
+    case T_MKNOD:
+    case T_MKDIR:
+    case T_SYMLINK:
+    case T_LINK:
+        if (v9fs_receive_status(proxy, reply, &retval) < 0) {
+            goto close_error;
+        }
+        break;
     }
 
 err_out:
@@ -303,15 +418,41 @@ static int proxy_chmod(FsContext *fs_ctx, V9fsPath *fs_path, FsCred *credp)
 static int proxy_mknod(FsContext *fs_ctx, V9fsPath *dir_path,
                        const char *name, FsCred *credp)
 {
-    errno = EOPNOTSUPP;
-    return -1;
+    int retval;
+    V9fsString fullname;
+
+    v9fs_string_init(&fullname);
+    v9fs_string_sprintf(&fullname, "%s/%s", dir_path->data, name);
+
+    retval = v9fs_request(fs_ctx->private, T_MKNOD, NULL, "sdqdd",
+                          &fullname, credp->fc_mode, credp->fc_rdev,
+                          credp->fc_uid, credp->fc_gid);
+    v9fs_string_free(&fullname);
+    if (retval < 0) {
+        errno = -retval;
+        retval = -1;
+    }
+    return retval;
 }
 
 static int proxy_mkdir(FsContext *fs_ctx, V9fsPath *dir_path,
                        const char *name, FsCred *credp)
 {
-    errno = EOPNOTSUPP;
-    return -1;
+    int retval;
+    V9fsString fullname;
+
+    v9fs_string_init(&fullname);
+    v9fs_string_sprintf(&fullname, "%s/%s", dir_path->data, name);
+
+    retval = v9fs_request(fs_ctx->private, T_MKDIR, NULL, "sddd", &fullname,
+                          credp->fc_mode, credp->fc_uid, credp->fc_gid);
+    v9fs_string_free(&fullname);
+    if (retval < 0) {
+        errno = -retval;
+        retval = -1;
+    }
+    v9fs_string_free(&fullname);
+    return retval;
 }
 
 static int proxy_fstat(FsContext *fs_ctx, int fid_type,
@@ -346,19 +487,46 @@ static int proxy_open2(FsContext *fs_ctx, V9fsPath *dir_path, const char *name,
     return fs->fd;
 }
 
-
 static int proxy_symlink(FsContext *fs_ctx, const char *oldpath,
                          V9fsPath *dir_path, const char *name, FsCred *credp)
 {
-    errno = EOPNOTSUPP;
-    return -1;
+    int retval;
+    V9fsString fullname, target;
+
+    v9fs_string_init(&fullname);
+    v9fs_string_init(&target);
+
+    v9fs_string_sprintf(&fullname, "%s/%s", dir_path->data, name);
+    v9fs_string_sprintf(&target, "%s", oldpath);
+
+    retval = v9fs_request(fs_ctx->private, T_SYMLINK, NULL, "ssdd",
+                          &target, &fullname, credp->fc_uid, credp->fc_gid);
+    v9fs_string_free(&fullname);
+    v9fs_string_free(&target);
+    if (retval < 0) {
+        errno = -retval;
+        retval = -1;
+    }
+    return retval;
 }
 
 static int proxy_link(FsContext *ctx, V9fsPath *oldpath,
                       V9fsPath *dirpath, const char *name)
 {
-    errno = EOPNOTSUPP;
-    return -1;
+    int retval;
+    V9fsString newpath;
+
+    v9fs_string_init(&newpath);
+    v9fs_string_sprintf(&newpath, "%s/%s", dirpath->data, name);
+
+    retval = v9fs_request(ctx->private, T_LINK, NULL, "ss",
+                          oldpath, &newpath);
+    v9fs_string_free(&newpath);
+    if (retval < 0) {
+        errno = -retval;
+        retval = -1;
+    }
+    return retval;
 }
 
 static int proxy_truncate(FsContext *ctx, V9fsPath *fs_path, off_t size)
@@ -516,8 +684,11 @@ static int proxy_init(FsContext *ctx)
     }
     g_free(ctx->fs_root);
 
-    proxy->iovec.iov_base = g_malloc(PROXY_MAX_IO_SZ + PROXY_HDR_SZ);
-    proxy->iovec.iov_len = PROXY_MAX_IO_SZ + PROXY_HDR_SZ;
+    proxy->in_iovec.iov_base  = g_malloc(PROXY_MAX_IO_SZ + PROXY_HDR_SZ);
+    proxy->in_iovec.iov_len   = PROXY_MAX_IO_SZ + PROXY_HDR_SZ;
+    proxy->out_iovec.iov_base = g_malloc(PROXY_MAX_IO_SZ + PROXY_HDR_SZ);
+    proxy->out_iovec.iov_len  = PROXY_MAX_IO_SZ + PROXY_HDR_SZ;
+
     ctx->private = proxy;
     proxy->sockfd = sock_id;
     qemu_mutex_init(&proxy->mutex);
diff --git a/hw/9pfs/virtio-9p-proxy.h b/hw/9pfs/virtio-9p-proxy.h
index b8719ea..86c7b4b 100644
--- a/hw/9pfs/virtio-9p-proxy.h
+++ b/hw/9pfs/virtio-9p-proxy.h
@@ -37,8 +37,14 @@ typedef struct {
 #define PROXY_HDR_SZ (sizeof(ProxyHeader))
 
 enum {
-    T_OPEN = 1,
+    T_SUCCESS = 0,
+    T_ERROR,
+    T_OPEN,
     T_CREATE,
+    T_MKNOD,
+    T_MKDIR,
+    T_SYMLINK,
+    T_LINK,
 };
 
 #endif
-- 
1.7.6

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [Qemu-devel] [PATCH V4 07/13] hw/9pfs: Add stat/readlink/statfs for proxy FS
  2011-12-05 16:18 [Qemu-devel] [PATCH V4 00/13] Proxy FS driver for VirtFS M. Mohan Kumar
                   ` (5 preceding siblings ...)
  2011-12-05 16:18 ` [Qemu-devel] [PATCH V4 06/13] hw/9pfs: Create other filesystem objects M. Mohan Kumar
@ 2011-12-05 16:18 ` M. Mohan Kumar
  2011-12-05 16:18 ` [Qemu-devel] [PATCH V4 08/13] hw/9pfs: File ownership and others M. Mohan Kumar
                   ` (6 subsequent siblings)
  13 siblings, 0 replies; 22+ messages in thread
From: M. Mohan Kumar @ 2011-12-05 16:18 UTC (permalink / raw)
  To: qemu-devel, aneesh.kumar, stefanha; +Cc: M. Mohan Kumar

From: "M. Mohan Kumar" <mohan@in.ibm.com>

Signed-off-by: M. Mohan Kumar <mohan@in.ibm.com>
---
 fsdev/virtfs-proxy-helper.c |  182 +++++++++++++++++++++++++++++++++++++
 hw/9pfs/virtio-9p-proxy.c   |  209 +++++++++++++++++++++++++++++++++++++++++--
 hw/9pfs/virtio-9p-proxy.h   |   34 +++++++
 3 files changed, 417 insertions(+), 8 deletions(-)

diff --git a/fsdev/virtfs-proxy-helper.c b/fsdev/virtfs-proxy-helper.c
index 729c79c..8bb1e06 100644
--- a/fsdev/virtfs-proxy-helper.c
+++ b/fsdev/virtfs-proxy-helper.c
@@ -25,6 +25,8 @@
 #include <sys/fsuid.h>
 #include <stdarg.h>
 #include <stdbool.h>
+#include <sys/vfs.h>
+#include <sys/stat.h>
 #include "qemu-common.h"
 #include "virtio-9p-marshal.h"
 #include "hw/9pfs/virtio-9p-proxy.h"
@@ -298,6 +300,172 @@ static int setfsugid(int uid, int gid)
 }
 
 /*
+ * send response in two parts
+ * 1) ProxyHeader
+ * 2) Response or error status
+ * This function should be called with marshaled response
+ * send_response constructs header part and error part only.
+ * send response sends {ProxyHeader,Response} if the request was success
+ * otherwise sends {ProxyHeader,error status}
+ */
+static int send_response(int sock, struct iovec *iovec, int size)
+{
+    int retval;
+    ProxyHeader header;
+
+    /*
+     * If response size exceeds available iovec->iov_len,
+     * we return ENOBUFS
+     */
+    if (size > PROXY_MAX_IO_SZ) {
+        size = -ENOBUFS;
+    }
+
+    if (size < 0) {
+        /*
+         * In case of error we would not have got the error encoded
+         * already so encode the error here.
+         */
+        header.type = T_ERROR;
+        header.size = sizeof(size);
+        proxy_marshal(iovec, PROXY_HDR_SZ, "d", size);
+    } else {
+        header.type = T_SUCCESS;
+        header.size = size;
+    }
+    proxy_marshal(iovec, 0, "dd", header.type, header.size);
+    retval = socket_write(sock, iovec->iov_base, header.size + PROXY_HDR_SZ);
+    if (retval < 0) {
+        return retval;;
+    }
+    return 0;
+}
+
+static void stat_to_prstat(ProxyStat *pr_stat, struct stat *stat)
+{
+    memset(pr_stat, 0, sizeof(*pr_stat));
+    pr_stat->st_dev = stat->st_dev;
+    pr_stat->st_ino = stat->st_ino;
+    pr_stat->st_nlink = stat->st_nlink;
+    pr_stat->st_mode = stat->st_mode;
+    pr_stat->st_uid = stat->st_uid;
+    pr_stat->st_gid = stat->st_gid;
+    pr_stat->st_rdev = stat->st_rdev;
+    pr_stat->st_size = stat->st_size;
+    pr_stat->st_blksize = stat->st_blksize;
+    pr_stat->st_blocks = stat->st_blocks;
+    pr_stat->st_atim_sec = stat->st_atim.tv_sec;
+    pr_stat->st_atim_nsec = stat->st_atim.tv_nsec;
+    pr_stat->st_mtim_sec = stat->st_mtim.tv_sec;
+    pr_stat->st_mtim_nsec = stat->st_mtim.tv_nsec;
+    pr_stat->st_ctim_sec = stat->st_ctim.tv_sec;
+    pr_stat->st_ctim_nsec = stat->st_ctim.tv_nsec;
+}
+
+static void statfs_to_prstatfs(ProxyStatFS *pr_stfs, struct statfs *stfs)
+{
+    memset(pr_stfs, 0, sizeof(*pr_stfs));
+    pr_stfs->f_type = stfs->f_type;
+    pr_stfs->f_bsize = stfs->f_bsize;
+    pr_stfs->f_blocks = stfs->f_blocks;
+    pr_stfs->f_bfree = stfs->f_bfree;
+    pr_stfs->f_bavail = stfs->f_bavail;
+    pr_stfs->f_files = stfs->f_files;
+    pr_stfs->f_ffree = stfs->f_ffree;
+    pr_stfs->f_fsid[0] = stfs->f_fsid.__val[0];
+    pr_stfs->f_fsid[1] = stfs->f_fsid.__val[1];
+    pr_stfs->f_namelen = stfs->f_namelen;
+    pr_stfs->f_frsize = stfs->f_frsize;
+}
+
+/*
+ * Gets stat/statfs information and packs in out_iovec structure
+ * on success returns number of bytes packed in out_iovec struture
+ * otherwise returns -errno
+ */
+static int do_stat(int type, struct iovec *iovec, struct iovec *out_iovec)
+{
+    int retval;
+    V9fsString path;
+    ProxyStat pr_stat;
+    ProxyStatFS pr_stfs;
+    struct stat st_buf;
+    struct statfs stfs_buf;
+
+    v9fs_string_init(&path);
+    retval = proxy_unmarshal(iovec, PROXY_HDR_SZ, "s", &path);
+    if (retval < 0) {
+        return retval;
+    }
+
+    switch (type) {
+    case T_LSTAT:
+        retval = lstat(path.data, &st_buf);
+        if (retval < 0) {
+            retval = -errno;
+        } else {
+            stat_to_prstat(&pr_stat, &st_buf);
+            retval = proxy_marshal(out_iovec, PROXY_HDR_SZ,
+                                   "qqqdddqqqqqqqqqq", pr_stat.st_dev,
+                                   pr_stat.st_ino, pr_stat.st_nlink,
+                                   pr_stat.st_mode, pr_stat.st_uid,
+                                   pr_stat.st_gid, pr_stat.st_rdev,
+                                   pr_stat.st_size, pr_stat.st_blksize,
+                                   pr_stat.st_blocks,
+                                   pr_stat.st_atim_sec, pr_stat.st_atim_nsec,
+                                   pr_stat.st_mtim_sec, pr_stat.st_mtim_nsec,
+                                   pr_stat.st_ctim_sec, pr_stat.st_ctim_nsec);
+        }
+        break;
+    case T_STATFS:
+        retval = statfs(path.data, &stfs_buf);
+        if (retval < 0) {
+            retval = -errno;
+        } else {
+            statfs_to_prstatfs(&pr_stfs, &stfs_buf);
+            retval = proxy_marshal(out_iovec, PROXY_HDR_SZ,
+                                   "qqqqqqqqqqq", pr_stfs.f_type,
+                                   pr_stfs.f_bsize, pr_stfs.f_blocks,
+                                   pr_stfs.f_bfree, pr_stfs.f_bavail,
+                                   pr_stfs.f_files, pr_stfs.f_ffree,
+                                   pr_stfs.f_fsid[0], pr_stfs.f_fsid[1],
+                                   pr_stfs.f_namelen, pr_stfs.f_frsize);
+        }
+        break;
+    }
+    v9fs_string_free(&path);
+    return retval;
+}
+
+static int do_readlink(struct iovec *iovec, struct iovec *out_iovec)
+{
+    char *buffer;
+    int size, retval;
+    V9fsString target, path;
+
+    v9fs_string_init(&path);
+    retval = proxy_unmarshal(iovec, PROXY_HDR_SZ, "sd", &path, &size);
+    if (retval < 0) {
+        v9fs_string_free(&path);
+        return retval;
+    }
+    buffer = g_malloc(size);
+    v9fs_string_init(&target);
+    retval = readlink(path.data, buffer, size);
+    if (retval > 0) {
+        buffer[retval] = '\0';
+        v9fs_string_sprintf(&target, "%s", buffer);
+        retval = proxy_marshal(out_iovec, PROXY_HDR_SZ, "s", &target);
+    } else {
+        retval = -errno;
+    }
+    g_free(buffer);
+    v9fs_string_free(&target);
+    v9fs_string_free(&path);
+    return retval;
+}
+
+/*
  * create other filesystem objects and send 0 on success
  * return -errno on error
  */
@@ -446,6 +614,13 @@ static int process_reply(int sock, int type,
             return -1;
         }
         break;
+    case T_LSTAT:
+    case T_STATFS:
+    case T_READLINK:
+        if (send_response(sock, out_iovec, retval) < 0) {
+            return -1;
+        }
+        break;
     default:
         return -1;
         break;
@@ -515,6 +690,13 @@ static int process_requests(int sock)
             v9fs_string_free(&oldpath);
             v9fs_string_free(&path);
             break;
+        case T_LSTAT:
+        case T_STATFS:
+            retval = do_stat(header.type, &in_iovec, &out_iovec);
+            break;
+        case T_READLINK:
+            retval = do_readlink(&in_iovec, &out_iovec);
+            break;
         default:
             goto err_out;
             break;
diff --git a/hw/9pfs/virtio-9p-proxy.c b/hw/9pfs/virtio-9p-proxy.c
index da6d7f9..bfbdcb5 100644
--- a/hw/9pfs/virtio-9p-proxy.c
+++ b/hw/9pfs/virtio-9p-proxy.c
@@ -101,6 +101,148 @@ static ssize_t socket_read(int sockfd, void *buff, size_t size)
     return total;
 }
 
+/* Converts proxy_statfs to VFS statfs structure */
+static void prstatfs_to_statfs(struct statfs *stfs, ProxyStatFS *prstfs)
+{
+    memset(stfs, 0, sizeof(*stfs));
+    stfs->f_type = prstfs->f_type;
+    stfs->f_bsize = prstfs->f_bsize;
+    stfs->f_blocks = prstfs->f_blocks;
+    stfs->f_bfree = prstfs->f_bfree;
+    stfs->f_bavail = prstfs->f_bavail;
+    stfs->f_files = prstfs->f_files;
+    stfs->f_ffree = prstfs->f_ffree;
+    stfs->f_fsid.__val[0] = prstfs->f_fsid[0] & 0xFFFFFFFFUL;
+    stfs->f_fsid.__val[1] = prstfs->f_fsid[1] >> 32 & 0xFFFFFFFFFUL;
+    stfs->f_namelen = prstfs->f_namelen;
+    stfs->f_frsize = prstfs->f_frsize;
+}
+
+/* Converts proxy_stat structure to VFS stat structure */
+static void prstat_to_stat(struct stat *stbuf, ProxyStat *prstat)
+{
+   memset(stbuf, 0, sizeof(*stbuf));
+   stbuf->st_dev = prstat->st_dev;
+   stbuf->st_ino = prstat->st_ino;
+   stbuf->st_nlink = prstat->st_nlink;
+   stbuf->st_mode = prstat->st_mode;
+   stbuf->st_uid = prstat->st_uid;
+   stbuf->st_gid = prstat->st_gid;
+   stbuf->st_rdev = prstat->st_rdev;
+   stbuf->st_size = prstat->st_size;
+   stbuf->st_blksize = prstat->st_blksize;
+   stbuf->st_blocks = prstat->st_blocks;
+   stbuf->st_atim.tv_sec = prstat->st_atim_sec;
+   stbuf->st_atim.tv_nsec = prstat->st_atim_nsec;
+   stbuf->st_mtime = prstat->st_mtim_sec;
+   stbuf->st_mtim.tv_nsec = prstat->st_mtim_nsec;
+   stbuf->st_ctime = prstat->st_ctim_sec;
+   stbuf->st_ctim.tv_nsec = prstat->st_ctim_nsec;
+}
+
+/*
+ * Response contains two parts
+ * {header, data}
+ * header.type == T_ERROR, data -> -errno
+ * header.type == T_SUCCESS, data -> response
+ * size of errno/response is given by header.size
+ * returns < 0, on transport error. response is
+ * valid only if status >= 0.
+ */
+static int v9fs_receive_response(V9fsProxy *proxy, int type,
+                                 int *status, void *response)
+{
+    int retval;
+    ProxyHeader header;
+    struct iovec *reply = &proxy->in_iovec;
+
+    *status = 0;
+    reply->iov_len = 0;
+    retval = socket_read(proxy->sockfd, reply->iov_base, PROXY_HDR_SZ);
+    if (retval < 0) {
+        return retval;
+    }
+    reply->iov_len = PROXY_HDR_SZ;
+    proxy_unmarshal(reply, 0, "dd", &header.type, &header.size);
+    /*
+     * if response size > PROXY_MAX_IO_SZ, read the response but ignore it and
+     * return -ENOBUFS
+     */
+    if (header.size > PROXY_MAX_IO_SZ) {
+        int count;
+        while (header.size > 0) {
+            count = MIN(PROXY_MAX_IO_SZ, header.size);
+            count = socket_read(proxy->sockfd, reply->iov_base, count);
+            if (count < 0) {
+                return count;
+            }
+            header.size -= count;
+        }
+        *status = -ENOBUFS;
+        return 0;
+    }
+
+    retval = socket_read(proxy->sockfd,
+                         reply->iov_base + PROXY_HDR_SZ, header.size);
+    if (retval < 0) {
+        return retval;
+    }
+    reply->iov_len += header.size;
+    /* there was an error during processing request */
+    if (header.type == T_ERROR) {
+        int ret;
+        ret = proxy_unmarshal(reply, PROXY_HDR_SZ, "d", status);
+        if (ret < 0) {
+            *status = ret;
+        }
+        return 0;
+    }
+
+    switch (type) {
+    case T_LSTAT: {
+        ProxyStat prstat;
+        retval = proxy_unmarshal(reply, PROXY_HDR_SZ,
+                                 "qqqdddqqqqqqqqqq", &prstat.st_dev,
+                                 &prstat.st_ino, &prstat.st_nlink,
+                                 &prstat.st_mode, &prstat.st_uid,
+                                 &prstat.st_gid, &prstat.st_rdev,
+                                 &prstat.st_size, &prstat.st_blksize,
+                                 &prstat.st_blocks,
+                                 &prstat.st_atim_sec, &prstat.st_atim_nsec,
+                                 &prstat.st_mtim_sec, &prstat.st_mtim_nsec,
+                                 &prstat.st_ctim_sec, &prstat.st_ctim_nsec);
+        prstat_to_stat(response, &prstat);
+        break;
+    }
+    case T_STATFS: {
+        ProxyStatFS prstfs;
+        retval = proxy_unmarshal(reply, PROXY_HDR_SZ,
+                                 "qqqqqqqqqqq", &prstfs.f_type,
+                                 &prstfs.f_bsize, &prstfs.f_blocks,
+                                 &prstfs.f_bfree, &prstfs.f_bavail,
+                                 &prstfs.f_files, &prstfs.f_ffree,
+                                 &prstfs.f_fsid[0], &prstfs.f_fsid[1],
+                                 &prstfs.f_namelen, &prstfs.f_frsize);
+        prstatfs_to_statfs(response, &prstfs);
+        break;
+    }
+    case T_READLINK: {
+        V9fsString target;
+        v9fs_string_init(&target);
+        retval = proxy_unmarshal(reply, PROXY_HDR_SZ, "s", &target);
+        strcpy(response, target.data);
+        v9fs_string_free(&target);
+        break;
+    }
+    default:
+        return -1;
+    }
+    if (retval < 0) {
+        *status  = retval;
+    }
+    return 0;
+}
+
 /*
  * return < 0 on transport error.
  * *status is valid only if return >= 0
@@ -143,6 +285,7 @@ static int v9fs_request(V9fsProxy *proxy, int type,
 {
     dev_t rdev;
     va_list ap;
+    int size = 0;
     int retval = 0;
     ProxyHeader header;
     int flags, mode, uid, gid;
@@ -234,6 +377,34 @@ static int v9fs_request(V9fsProxy *proxy, int type,
         }
         header.type = T_LINK;
         break;
+    case T_LSTAT:
+        path = va_arg(ap, V9fsString *);
+        header.size = proxy_marshal(iovec, PROXY_HDR_SZ, "s", path);
+        if (header.size < 0) {
+            retval = header.size;
+            break;
+        }
+        header.type = T_LSTAT;
+        break;
+    case T_READLINK:
+        path = va_arg(ap, V9fsString *);
+        size = va_arg(ap, int);
+        header.size = proxy_marshal(iovec, PROXY_HDR_SZ, "sd", path, size);
+        if (header.size < 0) {
+            retval = header.size;
+            break;
+        }
+        header.type = T_READLINK;
+        break;
+    case T_STATFS:
+        path = va_arg(ap, V9fsString *);
+        header.size = proxy_marshal(iovec, PROXY_HDR_SZ, "s", path);
+        if (header.size < 0) {
+            retval = header.size;
+            break;
+        }
+        header.type = T_STATFS;
+        break;
     default:
         error_report("Invalid type %d\n", type);
         retval = -EINVAL;
@@ -273,6 +444,13 @@ static int v9fs_request(V9fsProxy *proxy, int type,
             goto close_error;
         }
         break;
+    case T_LSTAT:
+    case T_READLINK:
+    case T_STATFS:
+        if (v9fs_receive_response(proxy, type, &retval, response) < 0) {
+            goto close_error;
+        }
+        break;
     }
 
 err_out:
@@ -288,15 +466,26 @@ close_error:
 
 static int proxy_lstat(FsContext *fs_ctx, V9fsPath *fs_path, struct stat *stbuf)
 {
-    errno = EOPNOTSUPP;
-    return -1;
+    int retval;
+    retval = v9fs_request(fs_ctx->private, T_LSTAT, stbuf, "s", fs_path);
+    if (retval < 0) {
+        errno = -retval;
+        return -1;
+    }
+    return retval;
 }
 
 static ssize_t proxy_readlink(FsContext *fs_ctx, V9fsPath *fs_path,
                               char *buf, size_t bufsz)
 {
-    errno = EOPNOTSUPP;
-    return -1;
+    int retval;
+    retval = v9fs_request(fs_ctx->private, T_READLINK, buf, "sd",
+                          fs_path, bufsz);
+    if (retval < 0) {
+        errno = -retval;
+        return -1;
+    }
+    return strlen(buf);
 }
 
 static int proxy_close(FsContext *ctx, V9fsFidOpenState *fs)
@@ -519,8 +708,7 @@ static int proxy_link(FsContext *ctx, V9fsPath *oldpath,
     v9fs_string_init(&newpath);
     v9fs_string_sprintf(&newpath, "%s/%s", dirpath->data, name);
 
-    retval = v9fs_request(ctx->private, T_LINK, NULL, "ss",
-                          oldpath, &newpath);
+    retval = v9fs_request(ctx->private, T_LINK, NULL, "ss", oldpath, &newpath);
     v9fs_string_free(&newpath);
     if (retval < 0) {
         errno = -retval;
@@ -581,8 +769,13 @@ static int proxy_fsync(FsContext *ctx, int fid_type,
 
 static int proxy_statfs(FsContext *s, V9fsPath *fs_path, struct statfs *stbuf)
 {
-    errno = EOPNOTSUPP;
-    return -1;
+    int retval;
+    retval = v9fs_request(s->private, T_STATFS, stbuf, "s", fs_path);
+    if (retval < 0) {
+        errno = -retval;
+        return -1;
+    }
+    return retval;
 }
 
 static ssize_t proxy_lgetxattr(FsContext *ctx, V9fsPath *fs_path,
diff --git a/hw/9pfs/virtio-9p-proxy.h b/hw/9pfs/virtio-9p-proxy.h
index 86c7b4b..f08874a 100644
--- a/hw/9pfs/virtio-9p-proxy.h
+++ b/hw/9pfs/virtio-9p-proxy.h
@@ -45,6 +45,40 @@ enum {
     T_MKDIR,
     T_SYMLINK,
     T_LINK,
+    T_LSTAT,
+    T_READLINK,
+    T_STATFS,
 };
 
+typedef struct {
+    uint64_t st_dev;
+    uint64_t st_ino;
+    uint64_t st_nlink;
+    uint32_t st_mode;
+    uint32_t st_uid;
+    uint32_t st_gid;
+    uint64_t st_rdev;
+    uint64_t st_size;
+    uint64_t st_blksize;
+    uint64_t st_blocks;
+    uint64_t st_atim_sec;
+    uint64_t st_atim_nsec;
+    uint64_t st_mtim_sec;
+    uint64_t st_mtim_nsec;
+    uint64_t st_ctim_sec;
+    uint64_t st_ctim_nsec;
+} ProxyStat;
+
+typedef struct {
+    uint64_t f_type;
+    uint64_t f_bsize;
+    uint64_t f_blocks;
+    uint64_t f_bfree;
+    uint64_t f_bavail;
+    uint64_t f_files;
+    uint64_t f_ffree;
+    uint64_t f_fsid[2];
+    uint64_t f_namelen;
+    uint64_t f_frsize;
+} ProxyStatFS;
 #endif
-- 
1.7.6

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [Qemu-devel] [PATCH V4 08/13] hw/9pfs: File ownership and others
  2011-12-05 16:18 [Qemu-devel] [PATCH V4 00/13] Proxy FS driver for VirtFS M. Mohan Kumar
                   ` (6 preceding siblings ...)
  2011-12-05 16:18 ` [Qemu-devel] [PATCH V4 07/13] hw/9pfs: Add stat/readlink/statfs for proxy FS M. Mohan Kumar
@ 2011-12-05 16:18 ` M. Mohan Kumar
  2011-12-05 16:18 ` [Qemu-devel] [PATCH V4 09/13] hw/9pfs: xattr interfaces in proxy filesystem driver M. Mohan Kumar
                   ` (5 subsequent siblings)
  13 siblings, 0 replies; 22+ messages in thread
From: M. Mohan Kumar @ 2011-12-05 16:18 UTC (permalink / raw)
  To: qemu-devel, aneesh.kumar, stefanha; +Cc: M. Mohan Kumar

From: "M. Mohan Kumar" <mohan@in.ibm.com>

Add file ownership interfaces like chmod/chown, utime update, rename,
remove and truncating files for proxy FS

Signed-off-by: M. Mohan Kumar <mohan@in.ibm.com>
---
 Makefile                    |    2 +-
 fsdev/virtfs-proxy-helper.c |   83 +++++++++++++++++++++++++
 hw/9pfs/virtio-9p-proxy.c   |  142 +++++++++++++++++++++++++++++++++++++++----
 hw/9pfs/virtio-9p-proxy.h   |    6 ++
 4 files changed, 220 insertions(+), 13 deletions(-)

diff --git a/Makefile b/Makefile
index 1906c5e..0acad52 100644
--- a/Makefile
+++ b/Makefile
@@ -154,7 +154,7 @@ qemu-img$(EXESUF): qemu-img.o $(tools-obj-y) $(block-obj-y)
 qemu-nbd$(EXESUF): qemu-nbd.o $(tools-obj-y) $(block-obj-y)
 qemu-io$(EXESUF): qemu-io.o cmd.o $(tools-obj-y) $(block-obj-y)
 
-fsdev/virtfs-proxy-helper$(EXESUF): fsdev/virtfs-proxy-helper.o fsdev/virtio-9p-marshal.o
+fsdev/virtfs-proxy-helper$(EXESUF): fsdev/virtfs-proxy-helper.o fsdev/virtio-9p-marshal.o oslib-posix.o $(trace-obj-y)
 fsdev/virtfs-proxy-helper$(EXESUF): LIBS += -lcap
 
 qemu-img-cmds.h: $(SRC_PATH)/qemu-img-cmds.hx
diff --git a/fsdev/virtfs-proxy-helper.c b/fsdev/virtfs-proxy-helper.c
index 8bb1e06..871e3f4 100644
--- a/fsdev/virtfs-proxy-helper.c
+++ b/fsdev/virtfs-proxy-helper.c
@@ -610,6 +610,12 @@ static int process_reply(int sock, int type,
     case T_MKDIR:
     case T_SYMLINK:
     case T_LINK:
+    case T_CHMOD:
+    case T_CHOWN:
+    case T_TRUNCATE:
+    case T_UTIME:
+    case T_RENAME:
+    case T_REMOVE:
         if (send_status(sock, out_iovec, retval) < 0) {
             return -1;
         }
@@ -631,7 +637,10 @@ static int process_reply(int sock, int type,
 static int process_requests(int sock)
 {
     int retval = 0;
+    uint64_t offset;
     ProxyHeader header;
+    int mode, uid, gid;
+    struct timespec spec[2];
     V9fsString oldpath, path;
     struct iovec in_iovec, out_iovec;
 
@@ -697,6 +706,80 @@ static int process_requests(int sock)
         case T_READLINK:
             retval = do_readlink(&in_iovec, &out_iovec);
             break;
+        case T_CHMOD:
+            v9fs_string_init(&path);
+            retval = proxy_unmarshal(&in_iovec, PROXY_HDR_SZ,
+                                     "sd", &path, &mode);
+            if (retval > 0) {
+                retval = chmod(path.data, mode);
+                if (retval < 0) {
+                    retval = -errno;
+                }
+            }
+            v9fs_string_free(&path);
+            break;
+        case T_CHOWN:
+            v9fs_string_init(&path);
+            retval = proxy_unmarshal(&in_iovec, PROXY_HDR_SZ, "sdd", &path,
+                                     &uid, &gid);
+            if (retval > 0) {
+                retval = lchown(path.data, uid, gid);
+                if (retval < 0) {
+                    retval = -errno;
+                }
+            }
+            v9fs_string_free(&path);
+            break;
+        case T_TRUNCATE:
+            v9fs_string_init(&path);
+            retval = proxy_unmarshal(&in_iovec, PROXY_HDR_SZ, "sq",
+                                     &path, &offset);
+            if (retval > 0) {
+                retval = truncate(path.data, offset);
+                if (retval < 0) {
+                    retval = -errno;
+                }
+            }
+            v9fs_string_free(&path);
+            break;
+        case T_UTIME:
+            v9fs_string_init(&path);
+            retval = proxy_unmarshal(&in_iovec, PROXY_HDR_SZ, "sqqqq", &path,
+                                     &spec[0].tv_sec, &spec[0].tv_nsec,
+                                     &spec[1].tv_sec, &spec[1].tv_nsec);
+            if (retval > 0) {
+                retval = qemu_utimens(path.data, spec);
+                if (retval < 0) {
+                    retval = -errno;
+                }
+            }
+            v9fs_string_free(&path);
+            break;
+        case T_RENAME:
+            v9fs_string_init(&path);
+            v9fs_string_init(&oldpath);
+            retval = proxy_unmarshal(&in_iovec, PROXY_HDR_SZ,
+                                     "ss", &oldpath, &path);
+            if (retval > 0) {
+                retval = rename(oldpath.data, path.data);
+                if (retval < 0) {
+                    retval = -errno;
+                }
+            }
+            v9fs_string_free(&oldpath);
+            v9fs_string_free(&path);
+            break;
+        case T_REMOVE:
+            v9fs_string_init(&path);
+            retval = proxy_unmarshal(&in_iovec, PROXY_HDR_SZ, "s", &path);
+            if (retval > 0) {
+                retval = remove(path.data);
+                if (retval < 0) {
+                    retval = -errno;
+                }
+            }
+            v9fs_string_free(&path);
+            break;
         default:
             goto err_out;
             break;
diff --git a/hw/9pfs/virtio-9p-proxy.c b/hw/9pfs/virtio-9p-proxy.c
index bfbdcb5..588d806 100644
--- a/hw/9pfs/virtio-9p-proxy.c
+++ b/hw/9pfs/virtio-9p-proxy.c
@@ -287,7 +287,9 @@ static int v9fs_request(V9fsProxy *proxy, int type,
     va_list ap;
     int size = 0;
     int retval = 0;
+    uint64_t offset;
     ProxyHeader header;
+    struct timespec spec[2];
     int flags, mode, uid, gid;
     V9fsString *path, *oldpath;
     struct iovec *iovec = NULL, *reply = NULL;
@@ -405,6 +407,71 @@ static int v9fs_request(V9fsProxy *proxy, int type,
         }
         header.type = T_STATFS;
         break;
+    case T_CHMOD:
+        path = va_arg(ap, V9fsString *);
+        mode = va_arg(ap, int);
+        header.size = proxy_marshal(iovec, PROXY_HDR_SZ, "sd", path, mode);
+        if (header.size < 0) {
+            retval = header.size;
+            break;
+        }
+        header.type = T_CHMOD;
+        break;
+    case T_CHOWN:
+        path = va_arg(ap, V9fsString *);
+        uid = va_arg(ap, int);
+        gid = va_arg(ap, int);
+        header.size = proxy_marshal(iovec, PROXY_HDR_SZ, "sdd", path, uid, gid);
+        if (header.size < 0) {
+            retval = header.size;
+            break;
+        }
+        header.type = T_CHOWN;
+        break;
+    case T_TRUNCATE:
+        path = va_arg(ap, V9fsString *);
+        offset = va_arg(ap, uint64_t);
+        header.size = proxy_marshal(iovec, PROXY_HDR_SZ, "sq", path, offset);
+        if (header.size < 0) {
+            retval = header.size;
+            break;
+        }
+        header.type = T_TRUNCATE;
+        break;
+    case T_UTIME:
+        path = va_arg(ap, V9fsString *);
+        spec[0].tv_sec = va_arg(ap, long);
+        spec[0].tv_nsec = va_arg(ap, long);
+        spec[1].tv_sec = va_arg(ap, long);
+        spec[1].tv_nsec = va_arg(ap, long);
+        header.size = proxy_marshal(iovec, PROXY_HDR_SZ, "sqqqq", path,
+                                    spec[0].tv_sec, spec[1].tv_nsec,
+                                    spec[1].tv_sec, spec[1].tv_nsec);
+        if (header.size < 0) {
+            retval = header.size;
+            break;
+        }
+        header.type = T_UTIME;
+        break;
+    case T_RENAME:
+        oldpath = va_arg(ap, V9fsString *);
+        path = va_arg(ap, V9fsString *);
+        header.size = proxy_marshal(iovec, PROXY_HDR_SZ, "ss", oldpath, path);
+        if (header.size < 0) {
+            retval = header.size;
+            break;
+        }
+        header.type = T_RENAME;
+        break;
+    case T_REMOVE:
+        path = va_arg(ap, V9fsString *);
+        header.size = proxy_marshal(iovec, PROXY_HDR_SZ, "s", path);
+        if (header.size < 0) {
+            retval = header.size;
+            break;
+        }
+        header.type = T_REMOVE;
+        break;
     default:
         error_report("Invalid type %d\n", type);
         retval = -EINVAL;
@@ -440,6 +507,12 @@ static int v9fs_request(V9fsProxy *proxy, int type,
     case T_MKDIR:
     case T_SYMLINK:
     case T_LINK:
+    case T_CHMOD:
+    case T_CHOWN:
+    case T_RENAME:
+    case T_TRUNCATE:
+    case T_UTIME:
+    case T_REMOVE:
         if (v9fs_receive_status(proxy, reply, &retval) < 0) {
             goto close_error;
         }
@@ -600,8 +673,13 @@ static ssize_t proxy_pwritev(FsContext *ctx, V9fsFidOpenState *fs,
 
 static int proxy_chmod(FsContext *fs_ctx, V9fsPath *fs_path, FsCred *credp)
 {
-    errno = EOPNOTSUPP;
-    return -1;
+    int retval;
+    retval = v9fs_request(fs_ctx->private, T_CHMOD, NULL, "sd",
+                          fs_path, credp->fc_mode);
+    if (retval < 0) {
+        errno = -retval;
+    }
+    return retval;
 }
 
 static int proxy_mknod(FsContext *fs_ctx, V9fsPath *dir_path,
@@ -719,34 +797,74 @@ static int proxy_link(FsContext *ctx, V9fsPath *oldpath,
 
 static int proxy_truncate(FsContext *ctx, V9fsPath *fs_path, off_t size)
 {
-    errno = EOPNOTSUPP;
-    return -1;
+    int retval;
+
+    retval = v9fs_request(ctx->private, T_TRUNCATE, NULL, "sq", fs_path, size);
+    if (retval < 0) {
+        errno = -retval;
+        return -1;
+    }
+    return 0;
 }
 
 static int proxy_rename(FsContext *ctx, const char *oldpath,
                         const char *newpath)
 {
-    errno = EOPNOTSUPP;
-    return -1;
+    int retval;
+    V9fsString oldname, newname;
+
+    v9fs_string_init(&oldname);
+    v9fs_string_init(&newname);
+
+    v9fs_string_sprintf(&oldname, "%s", oldpath);
+    v9fs_string_sprintf(&newname, "%s", newpath);
+    retval = v9fs_request(ctx->private, T_RENAME, NULL, "ss",
+                          &oldname, &newname);
+    v9fs_string_free(&oldname);
+    v9fs_string_free(&newname);
+    if (retval < 0) {
+        errno = -retval;
+    }
+    return retval;
 }
 
 static int proxy_chown(FsContext *fs_ctx, V9fsPath *fs_path, FsCred *credp)
 {
-    errno = EOPNOTSUPP;
-    return -1;
+    int retval;
+    retval = v9fs_request(fs_ctx->private, T_CHOWN, NULL, "sdd",
+                          fs_path, credp->fc_uid, credp->fc_gid);
+    if (retval < 0) {
+        errno = -retval;
+    }
+    return retval;
 }
 
 static int proxy_utimensat(FsContext *s, V9fsPath *fs_path,
                            const struct timespec *buf)
 {
-    errno = EOPNOTSUPP;
-    return -1;
+    int retval;
+    retval = v9fs_request(s->private, T_UTIME, NULL, "sqqqq",
+                          fs_path,
+                          buf[0].tv_sec, buf[0].tv_nsec,
+                          buf[1].tv_sec, buf[1].tv_nsec);
+    if (retval < 0) {
+        errno = -retval;
+    }
+    return retval;
 }
 
 static int proxy_remove(FsContext *ctx, const char *path)
 {
-    errno = EOPNOTSUPP;
-    return -1;
+    int retval;
+    V9fsString name;
+    v9fs_string_init(&name);
+    v9fs_string_sprintf(&name, "%s", path);
+    retval = v9fs_request(ctx->private, T_REMOVE, NULL, "s", &name);
+    v9fs_string_free(&name);
+    if (retval < 0) {
+        errno = -retval;
+    }
+    return retval;
 }
 
 static int proxy_fsync(FsContext *ctx, int fid_type,
diff --git a/hw/9pfs/virtio-9p-proxy.h b/hw/9pfs/virtio-9p-proxy.h
index f08874a..49aae66 100644
--- a/hw/9pfs/virtio-9p-proxy.h
+++ b/hw/9pfs/virtio-9p-proxy.h
@@ -48,6 +48,12 @@ enum {
     T_LSTAT,
     T_READLINK,
     T_STATFS,
+    T_CHMOD,
+    T_CHOWN,
+    T_TRUNCATE,
+    T_UTIME,
+    T_RENAME,
+    T_REMOVE,
 };
 
 typedef struct {
-- 
1.7.6

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [Qemu-devel] [PATCH V4 09/13] hw/9pfs: xattr interfaces in proxy filesystem driver
  2011-12-05 16:18 [Qemu-devel] [PATCH V4 00/13] Proxy FS driver for VirtFS M. Mohan Kumar
                   ` (7 preceding siblings ...)
  2011-12-05 16:18 ` [Qemu-devel] [PATCH V4 08/13] hw/9pfs: File ownership and others M. Mohan Kumar
@ 2011-12-05 16:18 ` M. Mohan Kumar
  2011-12-05 16:18 ` [Qemu-devel] [PATCH V4 10/13] hw/9pfs: Proxy getversion M. Mohan Kumar
                   ` (4 subsequent siblings)
  13 siblings, 0 replies; 22+ messages in thread
From: M. Mohan Kumar @ 2011-12-05 16:18 UTC (permalink / raw)
  To: qemu-devel, aneesh.kumar, stefanha; +Cc: M. Mohan Kumar

From: "M. Mohan Kumar" <mohan@in.ibm.com>

Add xattr support for proxy FS

Signed-off-by: M. Mohan Kumar <mohan@in.ibm.com>
---
 fsdev/virtfs-proxy-helper.c |   99 +++++++++++++++++++++++++++++++++
 hw/9pfs/virtio-9p-proxy.c   |  128 ++++++++++++++++++++++++++++++++++++++++---
 hw/9pfs/virtio-9p-proxy.h   |    4 +
 3 files changed, 223 insertions(+), 8 deletions(-)

diff --git a/fsdev/virtfs-proxy-helper.c b/fsdev/virtfs-proxy-helper.c
index 871e3f4..2a2ea0b 100644
--- a/fsdev/virtfs-proxy-helper.c
+++ b/fsdev/virtfs-proxy-helper.c
@@ -27,6 +27,7 @@
 #include <stdbool.h>
 #include <sys/vfs.h>
 #include <sys/stat.h>
+#include <attr/xattr.h>
 #include "qemu-common.h"
 #include "virtio-9p-marshal.h"
 #include "hw/9pfs/virtio-9p-proxy.h"
@@ -341,6 +342,62 @@ static int send_response(int sock, struct iovec *iovec, int size)
     return 0;
 }
 
+static int do_getxattr(int type, struct iovec *iovec, struct iovec *out_iovec)
+{
+    int size = 0, offset, retval;
+    V9fsString path, name, xattr;
+
+    v9fs_string_init(&xattr);
+    v9fs_string_init(&path);
+    retval = proxy_unmarshal(iovec, PROXY_HDR_SZ, "ds", &size, &path);
+    if (retval < 0) {
+        return retval;
+    }
+    offset = PROXY_HDR_SZ + retval;
+
+    if (size) {
+        xattr.data = g_malloc(size);
+        xattr.size = size;
+    }
+    switch (type) {
+    case T_LGETXATTR:
+        v9fs_string_init(&name);
+        retval = proxy_unmarshal(iovec, offset, "s", &name);
+        if (retval > 0) {
+            retval = lgetxattr(path.data, name.data, xattr.data, size);
+            if (retval < 0) {
+                retval = -errno;
+            } else {
+                xattr.size = retval;
+            }
+        }
+        v9fs_string_free(&name);
+        break;
+    case T_LLISTXATTR:
+        retval = llistxattr(path.data, xattr.data, size);
+        if (retval < 0) {
+            retval = -errno;
+        } else {
+            xattr.size = retval;
+        }
+        break;
+    }
+    if (retval < 0) {
+        goto err_out;
+    }
+
+    if (!size) {
+        proxy_marshal(out_iovec, PROXY_HDR_SZ, "d", retval);
+        retval = sizeof(retval);
+    } else {
+        retval = proxy_marshal(out_iovec, PROXY_HDR_SZ, "s", &xattr);
+    }
+err_out:
+    v9fs_string_free(&xattr);
+    v9fs_string_free(&path);
+    return retval;
+}
+
 static void stat_to_prstat(ProxyStat *pr_stat, struct stat *stat)
 {
     memset(pr_stat, 0, sizeof(*pr_stat));
@@ -616,6 +673,8 @@ static int process_reply(int sock, int type,
     case T_UTIME:
     case T_RENAME:
     case T_REMOVE:
+    case T_LSETXATTR:
+    case T_LREMOVEXATTR:
         if (send_status(sock, out_iovec, retval) < 0) {
             return -1;
         }
@@ -623,6 +682,8 @@ static int process_reply(int sock, int type,
     case T_LSTAT:
     case T_STATFS:
     case T_READLINK:
+    case T_LGETXATTR:
+    case T_LLISTXATTR:
         if (send_response(sock, out_iovec, retval) < 0) {
             return -1;
         }
@@ -636,10 +697,13 @@ static int process_reply(int sock, int type,
 
 static int process_requests(int sock)
 {
+    int flags;
+    int size = 0;
     int retval = 0;
     uint64_t offset;
     ProxyHeader header;
     int mode, uid, gid;
+    V9fsString name, value;
     struct timespec spec[2];
     V9fsString oldpath, path;
     struct iovec in_iovec, out_iovec;
@@ -780,6 +844,41 @@ static int process_requests(int sock)
             }
             v9fs_string_free(&path);
             break;
+        case T_LGETXATTR:
+        case T_LLISTXATTR:
+            retval = do_getxattr(header.type, &in_iovec, &out_iovec);
+            break;
+        case T_LSETXATTR:
+            v9fs_string_init(&path);
+            v9fs_string_init(&name);
+            v9fs_string_init(&value);
+            retval = proxy_unmarshal(&in_iovec, PROXY_HDR_SZ, "sssdd", &path,
+                                     &name, &value, &size, &flags);
+            if (retval > 0) {
+                retval = lsetxattr(path.data,
+                                   name.data, value.data, size, flags);
+                if (retval < 0) {
+                    retval = -errno;
+                }
+            }
+            v9fs_string_free(&path);
+            v9fs_string_free(&name);
+            v9fs_string_free(&value);
+            break;
+        case T_LREMOVEXATTR:
+            v9fs_string_init(&path);
+            v9fs_string_init(&name);
+            retval = proxy_unmarshal(&in_iovec,
+                                     PROXY_HDR_SZ, "ss", &path, &name);
+            if (retval > 0) {
+                retval = lremovexattr(path.data, name.data);
+                if (retval < 0) {
+                    retval = -errno;
+                }
+            }
+            v9fs_string_free(&path);
+            v9fs_string_free(&name);
+            break;
         default:
             goto err_out;
             break;
diff --git a/hw/9pfs/virtio-9p-proxy.c b/hw/9pfs/virtio-9p-proxy.c
index 588d806..ce1c411 100644
--- a/hw/9pfs/virtio-9p-proxy.c
+++ b/hw/9pfs/virtio-9p-proxy.c
@@ -234,6 +234,15 @@ static int v9fs_receive_response(V9fsProxy *proxy, int type,
         v9fs_string_free(&target);
         break;
     }
+    case T_LGETXATTR:
+    case T_LLISTXATTR: {
+        V9fsString xattr;
+        v9fs_string_init(&xattr);
+        retval = proxy_unmarshal(reply, PROXY_HDR_SZ, "s", &xattr);
+        memcpy(response, xattr.data, xattr.size);
+        v9fs_string_free(&xattr);
+        break;
+    }
     default:
         return -1;
     }
@@ -291,6 +300,7 @@ static int v9fs_request(V9fsProxy *proxy, int type,
     ProxyHeader header;
     struct timespec spec[2];
     int flags, mode, uid, gid;
+    V9fsString *name, *value;
     V9fsString *path, *oldpath;
     struct iovec *iovec = NULL, *reply = NULL;
 
@@ -472,6 +482,52 @@ static int v9fs_request(V9fsProxy *proxy, int type,
         }
         header.type = T_REMOVE;
         break;
+    case T_LGETXATTR:
+        size = va_arg(ap, int);
+        path = va_arg(ap, V9fsString *);
+        name = va_arg(ap, V9fsString *);
+        header.size = proxy_marshal(iovec, PROXY_HDR_SZ,
+                                    "dss", size, path, name);
+        if (header.size < 0) {
+            retval = header.size;
+            break;
+        }
+        header.type = T_LGETXATTR;
+        break;
+    case T_LLISTXATTR:
+        size = va_arg(ap, int);
+        path = va_arg(ap, V9fsString *);
+        header.size = proxy_marshal(iovec, PROXY_HDR_SZ, "ds", size, path);
+        if (header.size < 0) {
+            retval = header.size;
+            break;
+        }
+        header.type = T_LLISTXATTR;
+        break;
+    case T_LSETXATTR:
+        path = va_arg(ap, V9fsString *);
+        name = va_arg(ap, V9fsString *);
+        value = va_arg(ap, V9fsString *);
+        size = va_arg(ap, int);
+        flags = va_arg(ap, int);
+        header.size = proxy_marshal(iovec, PROXY_HDR_SZ, "sssdd",
+                                    path, name, value, size, flags);
+        if (header.size < 0) {
+            retval = header.size;
+            break;
+        }
+        header.type = T_LSETXATTR;
+        break;
+    case T_LREMOVEXATTR:
+        path = va_arg(ap, V9fsString *);
+        name = va_arg(ap, V9fsString *);
+        header.size = proxy_marshal(iovec, PROXY_HDR_SZ, "ss", path, name);
+        if (header.size < 0) {
+            retval = header.size;
+            break;
+        }
+        header.type = T_LREMOVEXATTR;
+        break;
     default:
         error_report("Invalid type %d\n", type);
         retval = -EINVAL;
@@ -513,6 +569,8 @@ static int v9fs_request(V9fsProxy *proxy, int type,
     case T_TRUNCATE:
     case T_UTIME:
     case T_REMOVE:
+    case T_LSETXATTR:
+    case T_LREMOVEXATTR:
         if (v9fs_receive_status(proxy, reply, &retval) < 0) {
             goto close_error;
         }
@@ -524,6 +582,18 @@ static int v9fs_request(V9fsProxy *proxy, int type,
             goto close_error;
         }
         break;
+    case T_LGETXATTR:
+    case T_LLISTXATTR:
+        if (!size) {
+            if (v9fs_receive_status(proxy, reply, &retval) < 0) {
+                goto close_error;
+            }
+        } else {
+            if (v9fs_receive_response(proxy, type, &retval, response) < 0) {
+                goto close_error;
+            }
+        }
+        break;
     }
 
 err_out:
@@ -899,29 +969,71 @@ static int proxy_statfs(FsContext *s, V9fsPath *fs_path, struct statfs *stbuf)
 static ssize_t proxy_lgetxattr(FsContext *ctx, V9fsPath *fs_path,
                                const char *name, void *value, size_t size)
 {
-    errno = EOPNOTSUPP;
-    return -1;
+    int retval;
+    V9fsString xname;
+
+    v9fs_string_init(&xname);
+    v9fs_string_sprintf(&xname, "%s", name);
+    retval = v9fs_request(ctx->private, T_LGETXATTR, value, "dss", size,
+                          fs_path, &xname);
+    v9fs_string_free(&xname);
+    if (retval < 0) {
+        errno = -retval;
+    }
+    return retval;
 }
 
 static ssize_t proxy_llistxattr(FsContext *ctx, V9fsPath *fs_path,
                                 void *value, size_t size)
 {
-    errno = EOPNOTSUPP;
-    return -1;
+    int retval;
+    retval = v9fs_request(ctx->private, T_LLISTXATTR, value, "ds", size,
+                        fs_path);
+    if (retval < 0) {
+        errno = -retval;
+    }
+    return retval;
 }
 
 static int proxy_lsetxattr(FsContext *ctx, V9fsPath *fs_path, const char *name,
                            void *value, size_t size, int flags)
 {
-    errno = EOPNOTSUPP;
-    return -1;
+    int retval;
+    V9fsString xname, xvalue;
+
+    v9fs_string_init(&xname);
+    v9fs_string_sprintf(&xname, "%s", name);
+
+    v9fs_string_init(&xvalue);
+    xvalue.size = size;
+    xvalue.data = g_malloc(size);
+    memcpy(xvalue.data, value, size);
+
+    retval = v9fs_request(ctx->private, T_LSETXATTR, value, "sssdd",
+                          fs_path, &xname, &xvalue, size, flags);
+    v9fs_string_free(&xname);
+    v9fs_string_free(&xvalue);
+    if (retval < 0) {
+        errno = -retval;
+    }
+    return retval;
 }
 
 static int proxy_lremovexattr(FsContext *ctx, V9fsPath *fs_path,
                               const char *name)
 {
-    errno = EOPNOTSUPP;
-    return -1;
+    int retval;
+    V9fsString xname;
+
+    v9fs_string_init(&xname);
+    v9fs_string_sprintf(&xname, "%s", name);
+    retval = v9fs_request(ctx->private, T_LREMOVEXATTR, NULL, "ss",
+                          fs_path, &xname);
+    v9fs_string_free(&xname);
+    if (retval < 0) {
+        errno = -retval;
+    }
+    return retval;
 }
 
 static int proxy_name_to_path(FsContext *ctx, V9fsPath *dir_path,
diff --git a/hw/9pfs/virtio-9p-proxy.h b/hw/9pfs/virtio-9p-proxy.h
index 49aae66..519e22f 100644
--- a/hw/9pfs/virtio-9p-proxy.h
+++ b/hw/9pfs/virtio-9p-proxy.h
@@ -54,6 +54,10 @@ enum {
     T_UTIME,
     T_RENAME,
     T_REMOVE,
+    T_LGETXATTR,
+    T_LLISTXATTR,
+    T_LSETXATTR,
+    T_LREMOVEXATTR,
 };
 
 typedef struct {
-- 
1.7.6

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [Qemu-devel] [PATCH V4 10/13] hw/9pfs: Proxy getversion
  2011-12-05 16:18 [Qemu-devel] [PATCH V4 00/13] Proxy FS driver for VirtFS M. Mohan Kumar
                   ` (8 preceding siblings ...)
  2011-12-05 16:18 ` [Qemu-devel] [PATCH V4 09/13] hw/9pfs: xattr interfaces in proxy filesystem driver M. Mohan Kumar
@ 2011-12-05 16:18 ` M. Mohan Kumar
  2011-12-05 16:18 ` [Qemu-devel] [PATCH V4 11/13] hw/9pfs: Documentation changes related to proxy fs M. Mohan Kumar
                   ` (3 subsequent siblings)
  13 siblings, 0 replies; 22+ messages in thread
From: M. Mohan Kumar @ 2011-12-05 16:18 UTC (permalink / raw)
  To: qemu-devel, aneesh.kumar, stefanha; +Cc: M. Mohan Kumar

From: "M. Mohan Kumar" <mohan@in.ibm.com>

Add proxy getversion to get generation number

Signed-off-by: M. Mohan Kumar <mohan@in.ibm.com>
---
 fsdev/virtfs-proxy-helper.c |   80 +++++++++++++++++++++++++++++++++++++++++++
 hw/9pfs/virtio-9p-proxy.c   |   33 ++++++++++++++++++
 hw/9pfs/virtio-9p-proxy.h   |    1 +
 3 files changed, 114 insertions(+), 0 deletions(-)

diff --git a/fsdev/virtfs-proxy-helper.c b/fsdev/virtfs-proxy-helper.c
index 2a2ea0b..a6bd6dc 100644
--- a/fsdev/virtfs-proxy-helper.c
+++ b/fsdev/virtfs-proxy-helper.c
@@ -28,6 +28,11 @@
 #include <sys/vfs.h>
 #include <sys/stat.h>
 #include <attr/xattr.h>
+#include <sys/ioctl.h>
+#include <linux/fs.h>
+#ifdef CONFIG_LINUX_MAGIC_H
+#include <linux/magic.h>
+#endif
 #include "qemu-common.h"
 #include "virtio-9p-marshal.h"
 #include "hw/9pfs/virtio-9p-proxy.h"
@@ -35,6 +40,19 @@
 
 #define PROGNAME "virtfs-proxy-helper"
 
+#ifndef XFS_SUPER_MAGIC
+#define XFS_SUPER_MAGIC  0x58465342
+#endif
+#ifndef EXT2_SUPER_MAGIC
+#define EXT2_SUPER_MAGIC 0xEF53
+#endif
+#ifndef REISERFS_SUPER_MAGIC
+#define REISERFS_SUPER_MAGIC 0x52654973
+#endif
+#ifndef BTRFS_SUPER_MAGIC
+#define BTRFS_SUPER_MAGIC 0x9123683E
+#endif
+
 static struct option helper_opts[] = {
     {"fd", required_argument, NULL, 'f'},
     {"path", required_argument, NULL, 'p'},
@@ -42,6 +60,7 @@ static struct option helper_opts[] = {
 };
 
 static bool is_daemon;
+static bool get_version; /* IOC getversion IOCTL supported */
 
 static void do_log(int loglevel, const char *format, ...)
 {
@@ -342,6 +361,45 @@ static int send_response(int sock, struct iovec *iovec, int size)
     return 0;
 }
 
+/*
+ * gets generation number
+ * returns -errno on failure and sizeof(generation number) on success
+ */
+static int do_getversion(struct iovec *iovec, struct iovec *out_iovec)
+{
+    V9fsString path;
+    uint64_t version;
+    int fd, retval = -ENOTTY;
+
+    /* no need to issue ioctl */
+    if (!get_version) {
+        version = 0;
+        retval = proxy_marshal(out_iovec, PROXY_HDR_SZ, "q", version);
+        return retval;
+    }
+#ifdef FS_IOC_GETVERSION
+    retval = proxy_unmarshal(iovec, PROXY_HDR_SZ, "s", &path);
+    if (retval < 0) {
+        return retval;
+    }
+
+    fd = open(path.data, O_RDONLY);
+    if (fd < 0) {
+        retval = -errno;
+        goto err_out;
+    }
+    if (ioctl(fd, FS_IOC_GETVERSION, &version) < 0) {
+        retval = -errno;
+    } else {
+        retval = proxy_marshal(out_iovec, PROXY_HDR_SZ, "q", version);
+    }
+    close(fd);
+err_out:
+    v9fs_string_free(&path);
+#endif
+    return retval;
+}
+
 static int do_getxattr(int type, struct iovec *iovec, struct iovec *out_iovec)
 {
     int size = 0, offset, retval;
@@ -684,6 +742,7 @@ static int process_reply(int sock, int type,
     case T_READLINK:
     case T_LGETXATTR:
     case T_LLISTXATTR:
+    case T_GETVERSION:
         if (send_response(sock, out_iovec, retval) < 0) {
             return -1;
         }
@@ -879,6 +938,9 @@ static int process_requests(int sock)
             v9fs_string_free(&path);
             v9fs_string_free(&name);
             break;
+        case T_GETVERSION:
+            retval = do_getversion(&in_iovec, &out_iovec);
+            break;
         default:
             goto err_out;
             break;
@@ -900,6 +962,8 @@ int main(int argc, char **argv)
     char *rpath = NULL;
     struct stat stbuf;
     int c, option_index;
+    int retval;
+    struct statfs st_fs;
 
     is_daemon = true;
     sock = -1;
@@ -956,6 +1020,22 @@ int main(int argc, char **argv)
 
     do_log(LOG_INFO, "Started\n");
 
+    get_version = false;
+#ifdef FS_IOC_GETVERSION
+    /* check whether underlying FS support IOC_GETVERSION */
+    retval = statfs(rpath, &st_fs);
+    if (!retval) {
+        switch (st_fs.f_type) {
+        case EXT2_SUPER_MAGIC:
+        case BTRFS_SUPER_MAGIC:
+        case REISERFS_SUPER_MAGIC:
+        case XFS_SUPER_MAGIC:
+            get_version = true;
+            break;
+        }
+    }
+#endif
+
     if (chroot(rpath) < 0) {
         do_perror("chroot");
         goto error;
diff --git a/hw/9pfs/virtio-9p-proxy.c b/hw/9pfs/virtio-9p-proxy.c
index ce1c411..65c17d6 100644
--- a/hw/9pfs/virtio-9p-proxy.c
+++ b/hw/9pfs/virtio-9p-proxy.c
@@ -243,6 +243,9 @@ static int v9fs_receive_response(V9fsProxy *proxy, int type,
         v9fs_string_free(&xattr);
         break;
     }
+    case T_GETVERSION:
+        proxy_unmarshal(reply, PROXY_HDR_SZ, "q", response);
+        break;
     default:
         return -1;
     }
@@ -528,6 +531,15 @@ static int v9fs_request(V9fsProxy *proxy, int type,
         }
         header.type = T_LREMOVEXATTR;
         break;
+    case T_GETVERSION:
+        path = va_arg(ap, V9fsString *);
+        header.size = proxy_marshal(iovec, PROXY_HDR_SZ, "s", path);
+        if (header.size < 0) {
+            retval = header.size;
+            break;
+        }
+        header.type = T_GETVERSION;
+        break;
     default:
         error_report("Invalid type %d\n", type);
         retval = -EINVAL;
@@ -578,6 +590,7 @@ static int v9fs_request(V9fsProxy *proxy, int type,
     case T_LSTAT:
     case T_READLINK:
     case T_STATFS:
+    case T_GETVERSION:
         if (v9fs_receive_response(proxy, type, &retval, response) < 0) {
             goto close_error;
         }
@@ -1083,6 +1096,25 @@ static int proxy_unlinkat(FsContext *ctx, V9fsPath *dir,
     return ret;
 }
 
+static int proxy_ioc_getversion(FsContext *fs_ctx, V9fsPath *path,
+                                mode_t st_mode, uint64_t *st_gen)
+{
+    int err;
+
+    /* Do not try to open special files like device nodes, fifos etc
+     * we can get fd for regular files and directories only
+     */
+    if (!S_ISREG(st_mode) && !S_ISDIR(st_mode)) {
+        return 0;
+    }
+    err = v9fs_request(fs_ctx->private, T_GETVERSION, st_gen, "s", path);
+    if (err < 0) {
+        errno = -err;
+        err = -1;
+    }
+    return err;
+}
+
 static int proxy_parse_opts(QemuOpts *opts, struct FsDriverEntry *fs)
 {
     const char *sock_fd = qemu_opt_get(opts, "sock_fd");
@@ -1117,6 +1149,7 @@ static int proxy_init(FsContext *ctx)
     qemu_mutex_init(&proxy->mutex);
 
     ctx->export_flags |= V9FS_PATHNAME_FSCONTEXT;
+    ctx->exops.get_st_gen = proxy_ioc_getversion;
     return 0;
 }
 
diff --git a/hw/9pfs/virtio-9p-proxy.h b/hw/9pfs/virtio-9p-proxy.h
index 519e22f..b9c0877 100644
--- a/hw/9pfs/virtio-9p-proxy.h
+++ b/hw/9pfs/virtio-9p-proxy.h
@@ -58,6 +58,7 @@ enum {
     T_LLISTXATTR,
     T_LSETXATTR,
     T_LREMOVEXATTR,
+    T_GETVERSION,
 };
 
 typedef struct {
-- 
1.7.6

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [Qemu-devel] [PATCH V4 11/13] hw/9pfs: Documentation changes related to proxy fs
  2011-12-05 16:18 [Qemu-devel] [PATCH V4 00/13] Proxy FS driver for VirtFS M. Mohan Kumar
                   ` (9 preceding siblings ...)
  2011-12-05 16:18 ` [Qemu-devel] [PATCH V4 10/13] hw/9pfs: Proxy getversion M. Mohan Kumar
@ 2011-12-05 16:18 ` M. Mohan Kumar
  2011-12-05 16:18 ` [Qemu-devel] [PATCH V4 12/13] hw/9pfs: man page for proxy helper M. Mohan Kumar
                   ` (2 subsequent siblings)
  13 siblings, 0 replies; 22+ messages in thread
From: M. Mohan Kumar @ 2011-12-05 16:18 UTC (permalink / raw)
  To: qemu-devel, aneesh.kumar, stefanha; +Cc: M. Mohan Kumar

From: "M. Mohan Kumar" <mohan@in.ibm.com>

Signed-off-by: M. Mohan Kumar <mohan@in.ibm.com>
---
 qemu-options.hx |   25 ++++++++++++++++---------
 1 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/qemu-options.hx b/qemu-options.hx
index 681eaf1..cde17ed 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -530,19 +530,19 @@ DEFHEADING()
 DEFHEADING(File system options:)
 
 DEF("fsdev", HAS_ARG, QEMU_OPTION_fsdev,
-    "-fsdev fsdriver,id=id,path=path,[security_model={mapped|passthrough|none}]\n"
-    "       [,writeout=immediate][,readonly]\n",
+    "-fsdev fsdriver,id=id[,path=path,][security_model={mapped|passthrough|none}]\n"
+    " [,writeout=immediate][,readonly][,sock_fd=sock_fd]\n",
     QEMU_ARCH_ALL)
 
 STEXI
 
-@item -fsdev @var{fsdriver},id=@var{id},path=@var{path},[security_model=@var{security_model}][,writeout=@var{writeout}][,readonly]
+@item -fsdev @var{fsdriver},id=@var{id},path=@var{path},[security_model=@var{security_model}][,writeout=@var{writeout}][,readonly][,sock_fd=@var{sock_fd}]
 @findex -fsdev
 Define a new file system device. Valid options are:
 @table @option
 @item @var{fsdriver}
 This option specifies the fs driver backend to use.
-Currently "local" and "handle" file system drivers are supported.
+Currently "local", "handle" and "proxy" file system drivers are supported.
 @item id=@var{id}
 Specifies identifier for this device
 @item path=@var{path}
@@ -559,7 +559,7 @@ file attributes. Directories exported by this security model cannot
 interact with other unix tools. "none" security model is same as
 passthrough except the sever won't report failures if it fails to
 set file attributes like ownership. Security model is mandatory
-only for local fsdriver. Other fsdrivers (like handle) don't take
+only for local fsdriver. Other fsdrivers (like handle, proxy) don't take
 security model as a parameter.
 @item writeout=@var{writeout}
 This is an optional argument. The only supported value is "immediate".
@@ -569,6 +569,10 @@ reported as written by the storage subsystem.
 @item readonly
 Enables exporting 9p share as a readonly mount for guests. By default
 read-write access is given.
+@item sock_fd=@var{sock_fd}
+Enables proxy filesystem driver to use passed socket descriptor for
+communicating with virtfs-proxy-helper. Usually a helper like libvirt
+will create socketpair and pass one of the fds as sock_fd
 @end table
 
 -fsdev option is used along with -device driver "virtio-9p-pci".
@@ -589,19 +593,19 @@ DEFHEADING(Virtual File system pass-through options:)
 
 DEF("virtfs", HAS_ARG, QEMU_OPTION_virtfs,
     "-virtfs local,path=path,mount_tag=tag,security_model=[mapped|passthrough|none]\n"
-    "        [,writeout=immediate][,readonly]\n",
+    "        [,writeout=immediate][,readonly][,sock_fd=sock_fd]\n",
     QEMU_ARCH_ALL)
 
 STEXI
 
-@item -virtfs @var{fsdriver},path=@var{path},mount_tag=@var{mount_tag},security_model=@var{security_model}[,writeout=@var{writeout}][,readonly]
+@item -virtfs @var{fsdriver}[,path=@var{path}],mount_tag=@var{mount_tag}[,security_model=@var{security_model}][,writeout=@var{writeout}][,readonly][,sock_fd=@var{sock_fd}]
 @findex -virtfs
 
 The general form of a Virtual File system pass-through options are:
 @table @option
 @item @var{fsdriver}
 This option specifies the fs driver backend to use.
-Currently "local" and "handle" file system drivers are supported.
+Currently "local", "handle" and "proxy" file system drivers are supported.
 @item id=@var{id}
 Specifies identifier for this device
 @item path=@var{path}
@@ -618,7 +622,7 @@ file attributes. Directories exported by this security model cannot
 interact with other unix tools. "none" security model is same as
 passthrough except the sever won't report failures if it fails to
 set file attributes like ownership. Security model is mandatory only
-for local fsdriver. Other fsdrivers (like handle) don't take security
+for local fsdriver. Other fsdrivers (like handle, proxy) don't take security
 model as a parameter.
 @item writeout=@var{writeout}
 This is an optional argument. The only supported value is "immediate".
@@ -628,6 +632,9 @@ reported as written by the storage subsystem.
 @item readonly
 Enables exporting 9p share as a readonly mount for guests. By default
 read-write access is given.
+@item sock_fd
+Enables proxy filesystem driver to use passed 'sock_fd' as the socket
+descriptor for interfacing with virtfs-proxy-helper
 @end table
 ETEXI
 
-- 
1.7.6

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [Qemu-devel] [PATCH V4 12/13] hw/9pfs: man page for proxy helper
  2011-12-05 16:18 [Qemu-devel] [PATCH V4 00/13] Proxy FS driver for VirtFS M. Mohan Kumar
                   ` (10 preceding siblings ...)
  2011-12-05 16:18 ` [Qemu-devel] [PATCH V4 11/13] hw/9pfs: Documentation changes related to proxy fs M. Mohan Kumar
@ 2011-12-05 16:18 ` M. Mohan Kumar
  2011-12-05 16:18 ` [Qemu-devel] [PATCH V4 13/13] hw/9pfs: Add support to use named socket for proxy FS M. Mohan Kumar
  2011-12-08 17:46 ` [Qemu-devel] [PATCH V4 00/13] Proxy FS driver for VirtFS Stefan Hajnoczi
  13 siblings, 0 replies; 22+ messages in thread
From: M. Mohan Kumar @ 2011-12-05 16:18 UTC (permalink / raw)
  To: qemu-devel, aneesh.kumar, stefanha; +Cc: M. Mohan Kumar

From: "M. Mohan Kumar" <mohan@in.ibm.com>

Signed-off-by: M. Mohan Kumar <mohan@in.ibm.com>
---
 Makefile                       |   12 +++++++-
 fsdev/virtfs-proxy-helper.texi |   59 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 70 insertions(+), 1 deletions(-)
 create mode 100644 fsdev/virtfs-proxy-helper.texi

diff --git a/Makefile b/Makefile
index 0acad52..f5b3524 100644
--- a/Makefile
+++ b/Makefile
@@ -38,6 +38,7 @@ LIBS+=-lz $(LIBS_TOOLS)
 
 ifdef BUILD_DOCS
 DOCS=qemu-doc.html qemu-tech.html qemu.1 qemu-img.1 qemu-nbd.8 QMP/qmp-commands.txt
+DOCS+=fsdev/virtfs-proxy-helper.1
 else
 DOCS=
 endif
@@ -283,7 +284,10 @@ ifdef CONFIG_POSIX
 	$(INSTALL_DIR) "$(DESTDIR)$(mandir)/man8"
 	$(INSTALL_DATA) qemu-nbd.8 "$(DESTDIR)$(mandir)/man8"
 endif
-
+ifdef CONFIG_VIRTFS
+	$(INSTALL_DIR) "$(DESTDIR)$(mandir)/man1"
+	$(INSTALL_DATA) fsdev/virtfs-proxy-helper.1 "$(DESTDIR)$(mandir)/man1"
+endif
 install-sysconfig:
 	$(INSTALL_DIR) "$(DESTDIR)$(sysconfdir)/qemu"
 	$(INSTALL_DATA) $(SRC_PATH)/sysconfigs/target/target-x86_64.conf "$(DESTDIR)$(sysconfdir)/qemu"
@@ -367,6 +371,12 @@ qemu-img.1: qemu-img.texi qemu-img-cmds.texi
 	  pod2man --section=1 --center=" " --release=" " qemu-img.pod > $@, \
 	  "  GEN   $@")
 
+fsdev/virtfs-proxy-helper.1: fsdev/virtfs-proxy-helper.texi
+	$(call quiet-command, \
+	  perl -Ww -- $(SRC_PATH)/scripts/texi2pod.pl $< fsdev/virtfs-proxy-helper.pod && \
+	  pod2man --section=1 --center=" " --release=" " fsdev/virtfs-proxy-helper.pod > $@, \
+	  "  GEN   $@")
+
 qemu-nbd.8: qemu-nbd.texi
 	$(call quiet-command, \
 	  perl -Ww -- $(SRC_PATH)/scripts/texi2pod.pl $< qemu-nbd.pod && \
diff --git a/fsdev/virtfs-proxy-helper.texi b/fsdev/virtfs-proxy-helper.texi
new file mode 100644
index 0000000..3816382
--- /dev/null
+++ b/fsdev/virtfs-proxy-helper.texi
@@ -0,0 +1,59 @@
+@example
+@c man begin SYNOPSIS
+usage: virtfs-proxy-helper options
+@c man end
+@end example
+
+@c man begin DESCRIPTION
+@table @description
+Pass-through security model in QEMU 9p server needs root privilege to do
+few file operations (like chown, chmod to any mode/uid:gid).  There are two
+issues in pass-through security model
+
+1) TOCTTOU vulnerability: Following symbolic links in the server could
+provide access to files beyond 9p export path.
+
+2) Running QEMU with root privilege could be a security issue.
+
+To overcome above issues, following approach is used: A new filesytem
+type 'proxy' is introduced. Proxy FS uses chroot + socket combination
+for securing the vulnerability known with following symbolic links.
+Intention of adding a new filesystem type is to allow qemu to run
+in non-root mode, but doing privileged operations using socket IO.
+
+Proxy helper(a stand alone binary part of qemu) is invoked with
+root privileges. Proxy helper chroots into 9p export path and creates
+a socket pair or a named socket based on the command line parameter.
+Qemu and proxy helper communicate using this socket. QEMU proxy fs
+driver sends filesystem request to proxy helper and receives the
+response from it.
+
+Proxy helper is designed so that it can drop the root privilege with
+retaining capbilities needed for doing filesystem operations only.
+
+@end table
+@c man end
+
+@c man begin OPTIONS
+The following options are supported:
+@table @option
+@item -h
+@findex -h
+Display help and exit
+@item -p|--path path
+Path to export for proxy filesystem driver
+@item -f|--fd socket-id
+Use given file descriptor as socket descriptor for communicating with
+qemu proxy fs drier. Usually a helper like libvirt will create
+socketpair and pass one of the fds as parameter to -f|--fd
+@item -n|--nodaemon
+Run as a normal program. By default program will run in daemon mode
+@end table
+@c man end
+
+@setfilename virtfs-proxy-helper
+@settitle QEMU 9p virtfs proxy filesystem helper
+
+@c man begin AUTHOR
+M. Mohan Kumar
+@c man end
-- 
1.7.6

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [Qemu-devel] [PATCH V4 13/13] hw/9pfs: Add support to use named socket for proxy FS
  2011-12-05 16:18 [Qemu-devel] [PATCH V4 00/13] Proxy FS driver for VirtFS M. Mohan Kumar
                   ` (11 preceding siblings ...)
  2011-12-05 16:18 ` [Qemu-devel] [PATCH V4 12/13] hw/9pfs: man page for proxy helper M. Mohan Kumar
@ 2011-12-05 16:18 ` M. Mohan Kumar
  2011-12-08 17:46 ` [Qemu-devel] [PATCH V4 00/13] Proxy FS driver for VirtFS Stefan Hajnoczi
  13 siblings, 0 replies; 22+ messages in thread
From: M. Mohan Kumar @ 2011-12-05 16:18 UTC (permalink / raw)
  To: qemu-devel, aneesh.kumar, stefanha; +Cc: M. Mohan Kumar

From: "M. Mohan Kumar" <mohan@in.ibm.com>

Add option to use named socket for communicating between proxy helper
and qemu proxy FS. Access to socket can be given by using command line
options -u and -g. We can achive the same using a shell script over
qemu and virtfs-proxy-helper using exec fd<><socket-path>, and then
passing that fd as argument to qemu and virtfs-proxy-helper. Also having
a server like virtfs-proxy-helper listening on a pathname without any
authentication is little bit scary. So we have to decide whether this
patch is really needed.

Signed-off-by: M. Mohan Kumar <mohan@in.ibm.com>
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
 fsdev/file-op-9p.h             |    2 +
 fsdev/virtfs-proxy-helper.c    |   86 ++++++++++++++++++++++++++++++++++++++-
 fsdev/virtfs-proxy-helper.texi |    4 ++
 hw/9pfs/virtio-9p-proxy.c      |   52 +++++++++++++++++++++---
 qemu-config.c                  |    7 +++
 qemu-options.hx                |   15 +++++--
 vl.c                           |    6 ++-
 7 files changed, 157 insertions(+), 15 deletions(-)

diff --git a/fsdev/file-op-9p.h b/fsdev/file-op-9p.h
index 73de79f..4f76f9d 100644
--- a/fsdev/file-op-9p.h
+++ b/fsdev/file-op-9p.h
@@ -57,6 +57,8 @@ typedef struct extended_ops {
  */
 #define V9FS_SM_NONE                0x00000010
 #define V9FS_RDONLY                 0x00000020
+#define V9FS_PROXY_SOCK_FD          0x00000040
+#define V9FS_PROXY_SOCK_NAME        0x00000080
 
 #define V9FS_SEC_MASK               0x0000001C
 
diff --git a/fsdev/virtfs-proxy-helper.c b/fsdev/virtfs-proxy-helper.c
index a6bd6dc..d526973 100644
--- a/fsdev/virtfs-proxy-helper.c
+++ b/fsdev/virtfs-proxy-helper.c
@@ -57,6 +57,9 @@ static struct option helper_opts[] = {
     {"fd", required_argument, NULL, 'f'},
     {"path", required_argument, NULL, 'p'},
     {"nodaemon", no_argument, NULL, 'n'},
+    {"socket", required_argument, NULL, 's'},
+    {"uid", required_argument, NULL, 'u'},
+    {"gid", required_argument, NULL, 'g'},
 };
 
 static bool is_daemon;
@@ -702,11 +705,61 @@ err_out:
     return ret;
 }
 
+/* create unix domain socket and return the descriptor */
+static int proxy_socket(const char *path, uid_t uid, gid_t gid)
+{
+    int sock, client;
+    struct sockaddr_un proxy, qemu;
+    socklen_t size;
+
+    /* requested socket already exists, refuse to start */
+    if (!access(path, F_OK)) {
+        do_log(LOG_CRIT, "socket already exists\n");
+        return -1;
+    }
+
+    sock = socket(AF_UNIX, SOCK_STREAM, 0);
+    if (sock < 0) {
+        do_perror("socket");
+        return -1;
+    }
+
+    /* mask other part of mode bits */
+    umask(7);
+
+    proxy.sun_family = AF_UNIX;
+    strcpy(proxy.sun_path, path);
+    if (bind(sock, (struct sockaddr *)&proxy,
+            sizeof(struct sockaddr_un)) < 0) {
+        do_perror("bind");
+        return -1;
+    }
+    if (chown(proxy.sun_path, uid, gid) < 0) {
+        do_perror("chown");
+        return -1;
+    }
+    if (listen(sock, 1) < 0) {
+        do_perror("listen");
+        return -1;
+    }
+
+    client = accept(sock, (struct sockaddr *)&qemu, &size);
+    if (client < 0) {
+        do_perror("accept");
+        return -1;
+    }
+    return client;
+}
+
 static void usage(char *prog)
 {
     fprintf(stderr, "usage: %s\n"
             " -p|--path <path> 9p path to export\n"
             " {-f|--fd <socket-descriptor>} socket file descriptor to be used\n"
+            " {-s|--socket <socketname> socket file used for communication\n"
+            " \t-u|--uid <uid> -g|--gid <gid>} - uid:gid combination to give "
+            " access to this socket\n"
+            " \tNote: -s & -f can not be used together\n"
             " [-n|--nodaemon] Run as a normal program\n",
             basename(prog));
 }
@@ -960,16 +1013,20 @@ int main(int argc, char **argv)
 {
     int sock;
     char *rpath = NULL;
+    char *sock_name = NULL;
     struct stat stbuf;
     int c, option_index;
     int retval;
     struct statfs st_fs;
+    uid_t own_u;
+    gid_t own_g;
 
     is_daemon = true;
     sock = -1;
+    own_u = own_g = -1;
     while (1) {
         option_index = 0;
-        c = getopt_long(argc, argv, "p:nh?f:", helper_opts,
+        c = getopt_long(argc, argv, "p:nh?f:s:u:g:", helper_opts,
                         &option_index);
         if (c == -1) {
             break;
@@ -984,6 +1041,15 @@ int main(int argc, char **argv)
         case 'f':
             sock = atoi(optarg);
             break;
+        case 's':
+            sock_name = strdup(optarg);
+            break;
+        case 'u':
+            own_u = atoi(optarg);
+            break;
+        case 'g':
+            own_g = atoi(optarg);
+            break;
         case '?':
         case 'h':
         default:
@@ -993,8 +1059,16 @@ int main(int argc, char **argv)
     }
 
     /* Parameter validation */
-    if (sock == -1 || rpath == NULL) {
-        fprintf(stderr, "socket descriptor or path not specified\n");
+    if ((sock_name == NULL && sock == -1) || rpath == NULL) {
+        fprintf(stderr, "socket, socket descriptor or path not specified\n");
+        usage(argv[0]);
+        return -1;
+    }
+
+    if (*sock_name && (own_u == -1 || own_g == -1)) {
+        fprintf(stderr, "owner uid:gid not specified, ");
+        fprintf(stderr,
+                "owner uid:gid specifies who can access the socket file\n");
         usage(argv[0]);
         exit(EXIT_FAILURE);
     }
@@ -1019,6 +1093,12 @@ int main(int argc, char **argv)
     }
 
     do_log(LOG_INFO, "Started\n");
+    if (*sock_name) {
+        sock = proxy_socket(sock_name, own_u, own_g);
+        if (sock < 0) {
+            goto error;
+        }
+    }
 
     get_version = false;
 #ifdef FS_IOC_GETVERSION
diff --git a/fsdev/virtfs-proxy-helper.texi b/fsdev/virtfs-proxy-helper.texi
index 3816382..faa0434 100644
--- a/fsdev/virtfs-proxy-helper.texi
+++ b/fsdev/virtfs-proxy-helper.texi
@@ -46,6 +46,10 @@ Path to export for proxy filesystem driver
 Use given file descriptor as socket descriptor for communicating with
 qemu proxy fs drier. Usually a helper like libvirt will create
 socketpair and pass one of the fds as parameter to -f|--fd
+@item -s|--socket socket-file
+Creates named socket file for communicating with qemu proxy fs driver
+@item -u|--uid uid -g|--gid gid
+uid:gid combination to give access to named socket file
 @item -n|--nodaemon
 Run as a normal program. By default program will run in daemon mode
 @end table
diff --git a/hw/9pfs/virtio-9p-proxy.c b/hw/9pfs/virtio-9p-proxy.c
index 65c17d6..e567b52 100644
--- a/hw/9pfs/virtio-9p-proxy.c
+++ b/hw/9pfs/virtio-9p-proxy.c
@@ -1115,15 +1115,49 @@ static int proxy_ioc_getversion(FsContext *fs_ctx, V9fsPath *path,
     return err;
 }
 
+static int connect_namedsocket(const char *path)
+{
+    int sockfd, size;
+    struct sockaddr_un helper;
+
+    sockfd = socket(AF_UNIX, SOCK_STREAM, 0);
+    if (sockfd < 0) {
+        fprintf(stderr, "socket %s\n", strerror(errno));
+        return -1;
+    }
+    strcpy(helper.sun_path, path);
+    helper.sun_family = AF_UNIX;
+    size = strlen(helper.sun_path) + sizeof(helper.sun_family);
+    if (connect(sockfd, (struct sockaddr *)&helper, size) < 0) {
+        fprintf(stderr, "socket error\n");
+        return -1;
+    }
+
+    /* remove the socket for security reasons */
+    unlink(path);
+    return sockfd;
+}
+
 static int proxy_parse_opts(QemuOpts *opts, struct FsDriverEntry *fs)
 {
+    const char *socket = qemu_opt_get(opts, "socket");
     const char *sock_fd = qemu_opt_get(opts, "sock_fd");
 
-    if (sock_fd) {
-        fprintf(stderr, "sock_fd option not specified\n");
+    if (!socket && !sock_fd) {
+        fprintf(stderr, "socket and sock_fd none of the option specified\n");
+        return -1;
+    }
+    if (socket && sock_fd) {
+        fprintf(stderr, "Both socket and sock_fd options specified\n");
         return -1;
     }
-    fs->path = g_strdup(sock_fd);
+    if (socket) {
+        fs->path = g_strdup(socket);
+        fs->export_flags = V9FS_PROXY_SOCK_NAME;
+    } else {
+        fs->path = g_strdup(sock_fd);
+        fs->export_flags = V9FS_PROXY_SOCK_FD;
+    }
     return 0;
 }
 
@@ -1132,10 +1166,14 @@ static int proxy_init(FsContext *ctx)
     V9fsProxy *proxy = g_malloc(sizeof(V9fsProxy));
     int sock_id;
 
-    sock_id = atoi(ctx->fs_root);
-    if (sock_id < 0) {
-        fprintf(stderr, "socket descriptor not initialized\n");
-        return -1;
+    if (ctx->export_flags & V9FS_PROXY_SOCK_NAME) {
+        sock_id = connect_namedsocket(ctx->fs_root);
+    } else {
+        sock_id = atoi(ctx->fs_root);
+        if (sock_id < 0) {
+            fprintf(stderr, "socket descriptor not initialized\n");
+            return -1;
+        }
     }
     g_free(ctx->fs_root);
 
diff --git a/qemu-config.c b/qemu-config.c
index 33367fe..48093ca 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -183,6 +183,10 @@ QemuOptsList qemu_fsdev_opts = {
         }, {
             .name = "readonly",
             .type = QEMU_OPT_BOOL,
+
+        }, {
+            .name = "socket",
+            .type = QEMU_OPT_STRING,
         }, {
             .name = "sock_fd",
             .type = QEMU_OPT_NUMBER,
@@ -216,6 +220,9 @@ QemuOptsList qemu_virtfs_opts = {
             .name = "readonly",
             .type = QEMU_OPT_BOOL,
         }, {
+            .name = "socket",
+            .type = QEMU_OPT_STRING,
+        }, {
             .name = "sock_fd",
             .type = QEMU_OPT_NUMBER,
         },
diff --git a/qemu-options.hx b/qemu-options.hx
index cde17ed..a686f87 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -531,12 +531,12 @@ DEFHEADING(File system options:)
 
 DEF("fsdev", HAS_ARG, QEMU_OPTION_fsdev,
     "-fsdev fsdriver,id=id[,path=path,][security_model={mapped|passthrough|none}]\n"
-    " [,writeout=immediate][,readonly][,sock_fd=sock_fd]\n",
+    " [,writeout=immediate][,readonly][,socket=socket|sock_fd=sock_fd]\n",
     QEMU_ARCH_ALL)
 
 STEXI
 
-@item -fsdev @var{fsdriver},id=@var{id},path=@var{path},[security_model=@var{security_model}][,writeout=@var{writeout}][,readonly][,sock_fd=@var{sock_fd}]
+@item -fsdev @var{fsdriver},id=@var{id},path=@var{path},[security_model=@var{security_model}][,writeout=@var{writeout}][,readonly][,socket=@var{socket}|sock_fd=@var{sock_fd}]
 @findex -fsdev
 Define a new file system device. Valid options are:
 @table @option
@@ -569,6 +569,9 @@ reported as written by the storage subsystem.
 @item readonly
 Enables exporting 9p share as a readonly mount for guests. By default
 read-write access is given.
+@item socket=@var{socket}
+Enables proxy filesystem driver to use passed socket file for communicating
+with virtfs-proxy-helper
 @item sock_fd=@var{sock_fd}
 Enables proxy filesystem driver to use passed socket descriptor for
 communicating with virtfs-proxy-helper. Usually a helper like libvirt
@@ -593,12 +596,12 @@ DEFHEADING(Virtual File system pass-through options:)
 
 DEF("virtfs", HAS_ARG, QEMU_OPTION_virtfs,
     "-virtfs local,path=path,mount_tag=tag,security_model=[mapped|passthrough|none]\n"
-    "        [,writeout=immediate][,readonly][,sock_fd=sock_fd]\n",
+    "        [,writeout=immediate][,readonly][,socket=socket|sock_fd=sock_fd]\n",
     QEMU_ARCH_ALL)
 
 STEXI
 
-@item -virtfs @var{fsdriver}[,path=@var{path}],mount_tag=@var{mount_tag}[,security_model=@var{security_model}][,writeout=@var{writeout}][,readonly][,sock_fd=@var{sock_fd}]
+@item -virtfs @var{fsdriver}[,path=@var{path}],mount_tag=@var{mount_tag}[,security_model=@var{security_model}][,writeout=@var{writeout}][,readonly][,socket=@var{socket}|sock_fd=@var{sock_fd}]
 @findex -virtfs
 
 The general form of a Virtual File system pass-through options are:
@@ -632,6 +635,10 @@ reported as written by the storage subsystem.
 @item readonly
 Enables exporting 9p share as a readonly mount for guests. By default
 read-write access is given.
+@item socket=@var{socket}
+Enables proxy filesystem driver to use passed socket file for
+communicating with virtfs-proxy-helper. Usually a helper like libvirt
+will create socketpair and pass one of the fds as sock_fd
 @item sock_fd
 Enables proxy filesystem driver to use passed 'sock_fd' as the socket
 descriptor for interfacing with virtfs-proxy-helper
diff --git a/vl.c b/vl.c
index fe43b4e..60f0d2f 100644
--- a/vl.c
+++ b/vl.c
@@ -2669,7 +2669,7 @@ int main(int argc, char **argv, char **envp)
             case QEMU_OPTION_virtfs: {
                 QemuOpts *fsdev;
                 QemuOpts *device;
-                const char *writeout, *sock_fd;
+                const char *writeout, *sock_fd, *socket;
 
                 olist = qemu_find_opts("virtfs");
                 if (!olist) {
@@ -2709,6 +2709,10 @@ int main(int argc, char **argv, char **envp)
                 qemu_opt_set(fsdev, "path", qemu_opt_get(opts, "path"));
                 qemu_opt_set(fsdev, "security_model",
                              qemu_opt_get(opts, "security_model"));
+                socket = qemu_opt_get(opts, "socket");
+                if (socket) {
+                    qemu_opt_set(fsdev, "socket", socket);
+                }
                 sock_fd = qemu_opt_get(opts, "sock_fd");
                 if (sock_fd) {
                     qemu_opt_set(fsdev, "sock_fd", sock_fd);
-- 
1.7.6

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [Qemu-devel] [PATCH V4 00/13] Proxy FS driver for VirtFS
  2011-12-05 16:18 [Qemu-devel] [PATCH V4 00/13] Proxy FS driver for VirtFS M. Mohan Kumar
                   ` (12 preceding siblings ...)
  2011-12-05 16:18 ` [Qemu-devel] [PATCH V4 13/13] hw/9pfs: Add support to use named socket for proxy FS M. Mohan Kumar
@ 2011-12-08 17:46 ` Stefan Hajnoczi
  2011-12-09 14:47   ` M. Mohan Kumar
  13 siblings, 1 reply; 22+ messages in thread
From: Stefan Hajnoczi @ 2011-12-08 17:46 UTC (permalink / raw)
  To: M. Mohan Kumar; +Cc: M. Mohan Kumar, qemu-devel, aneesh.kumar

On Mon, Dec 05, 2011 at 09:48:37PM +0530, M. Mohan Kumar wrote:
> From: "M. Mohan Kumar" <mohan@in.ibm.com>
> 
> Pass-through security model in QEMU 9p server needs root privilege to do
> few file operations (like chown, chmod to any mode/uid:gid).  There are two
> issues in pass-through security model
> 
> 1) TOCTTOU vulnerability: Following symbolic links in the server could
> provide access to files beyond 9p export path.
> 
> 2) Running QEMU with root privilege could be a security issue.
> 
> To overcome above issues, following approach is used: A new filesytem
> type 'proxy' is introduced. Proxy FS uses chroot + socket combination
> for securing the vulnerability known with following symbolic links.
> Intention of adding a new filesystem type is to allow qemu to run
> in non-root mode, but doing privileged operations using socket IO.

Fails to build against qemu.git/master (217bfb4):

  CC    libhw64/9pfs/virtio-9p-proxy.o
hw/9pfs/virtio-9p-proxy.c:1195:5: error: unknown field ‘parse_opts’ specified in initializer
hw/9pfs/virtio-9p-proxy.c:1195:5: warning: initialization from incompatible pointer type [enabled by default]
hw/9pfs/virtio-9p-proxy.c:1195:5: warning: (near initialization for ‘proxy_ops.init’) [enabled by default]

Is this against another public tree?

Stefan

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [Qemu-devel] [PATCH V4 02/13] hw/9pfs: Add validation to {un}marshal code
  2011-12-05 16:18 ` [Qemu-devel] [PATCH V4 02/13] hw/9pfs: Add validation to {un}marshal code M. Mohan Kumar
@ 2011-12-08 18:10   ` Stefan Hajnoczi
  0 siblings, 0 replies; 22+ messages in thread
From: Stefan Hajnoczi @ 2011-12-08 18:10 UTC (permalink / raw)
  To: M. Mohan Kumar; +Cc: M. Mohan Kumar, qemu-devel, aneesh.kumar

On Mon, Dec 05, 2011 at 09:48:39PM +0530, M. Mohan Kumar wrote:
> @@ -187,59 +190,70 @@ size_t v9fs_unmarshal(struct iovec *out_sg, int out_num, size_t offset,
>          }
>          case 's': {
>              V9fsString *str = va_arg(ap, V9fsString *);
> -            offset += v9fs_unmarshal(out_sg, out_num, offset, bswap,
> -                            "w", &str->size);
> -            /* FIXME: sanity check str->size */
> -            str->data = g_malloc(str->size + 1);
> -            offset += v9fs_unpack(str->data, out_sg, out_num, offset,
> -                            str->size);
> -            str->data[str->size] = 0;
> +            copied = v9fs_unmarshal(out_sg, out_num, offset, bswap,
> +                                    "w", &str->size);
> +            if (copied > 0) {
> +                offset += copied;
> +                str->data = g_malloc(str->size + 1);

str->size is signed int16_t, we need a check or the type should be
uint16_t.

Stefan

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [Qemu-devel] [PATCH V4 04/13] hw/9pfs: File system helper process for qemu 9p proxy FS
  2011-12-05 16:18 ` [Qemu-devel] [PATCH V4 04/13] hw/9pfs: File system helper process for qemu 9p proxy FS M. Mohan Kumar
@ 2011-12-08 18:31   ` Stefan Hajnoczi
  2011-12-09 16:42     ` M. Mohan Kumar
  0 siblings, 1 reply; 22+ messages in thread
From: Stefan Hajnoczi @ 2011-12-08 18:31 UTC (permalink / raw)
  To: M. Mohan Kumar; +Cc: M. Mohan Kumar, qemu-devel, aneesh.kumar

On Mon, Dec 05, 2011 at 09:48:41PM +0530, M. Mohan Kumar wrote:
> +static int read_request(int sockfd, struct iovec *iovec, ProxyHeader *header)
> +{
> +    int retval;
> +
> +    /*
> +     * read the request header.
> +     */
> +    iovec->iov_len = 0;
> +    retval = socket_read(sockfd, iovec->iov_base, PROXY_HDR_SZ);
> +    if (retval < 0) {
> +        return retval;
> +    }
> +    iovec->iov_len = PROXY_HDR_SZ;
> +    retval = proxy_unmarshal(iovec, 0, "dd", &header->type, &header->size);
> +    if (retval < 0) {
> +        return retval;
> +    }
> +    /*
> +     * We can't process message.size > PROXY_MAX_IO_SZ, read the complete
> +     * message from the socket and ignore it. This ensures that
> +     * we can correctly handle the next request. We also return
> +     * ENOBUFS as error to indicate we ran out of buffer space.
> +     */
> +    if (header->size > PROXY_MAX_IO_SZ) {
> +        int count, size;
> +        size = header->size;
> +        while (size > 0) {
> +            count = MIN(PROXY_MAX_IO_SZ, size);
> +            count = socket_read(sockfd, iovec->iov_base + PROXY_HDR_SZ, count);
> +            if (count < 0) {
> +                return count;
> +            }
> +            size -= count;
> +        }

I'm not sure recovery attempts are worthwhile here.  The client is
buggy, perhaps just refuse further work.

> +        return -ENOBUFS;
> +    }

header->size is (signed) int and we didn't check for header->size < 0.
Please use an unsigned type.

> +    if (chroot(rpath) < 0) {
> +        do_perror("chroot");
> +        goto error;
> +    }
> +    umask(0);

We haven't changed into the chroot yet, we need chdir("/").  Otherwise
the current working directory is outside the chroot (and allows trivial
escape).

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [Qemu-devel] [PATCH V4 00/13] Proxy FS driver for VirtFS
  2011-12-08 17:46 ` [Qemu-devel] [PATCH V4 00/13] Proxy FS driver for VirtFS Stefan Hajnoczi
@ 2011-12-09 14:47   ` M. Mohan Kumar
  0 siblings, 0 replies; 22+ messages in thread
From: M. Mohan Kumar @ 2011-12-09 14:47 UTC (permalink / raw)
  To: Stefan Hajnoczi; +Cc: M. Mohan Kumar, qemu-devel, aneesh.kumar

On Thursday, December 08, 2011 11:16:33 PM Stefan Hajnoczi wrote:
> On Mon, Dec 05, 2011 at 09:48:37PM +0530, M. Mohan Kumar wrote:
> > From: "M. Mohan Kumar" <mohan@in.ibm.com>
> > 
> > Pass-through security model in QEMU 9p server needs root privilege to do
> > few file operations (like chown, chmod to any mode/uid:gid).  There are
> > two issues in pass-through security model
> > 
> > 1) TOCTTOU vulnerability: Following symbolic links in the server could
> > provide access to files beyond 9p export path.
> > 
> > 2) Running QEMU with root privilege could be a security issue.
> > 
> > To overcome above issues, following approach is used: A new filesytem
> > type 'proxy' is introduced. Proxy FS uses chroot + socket combination
> > for securing the vulnerability known with following symbolic links.
> > Intention of adding a new filesystem type is to allow qemu to run
> > in non-root mode, but doing privileged operations using socket IO.
> 
> Fails to build against qemu.git/master (217bfb4):
> 
>   CC    libhw64/9pfs/virtio-9p-proxy.o
> hw/9pfs/virtio-9p-proxy.c:1195:5: error: unknown field ‘parse_opts’
> specified in initializer hw/9pfs/virtio-9p-proxy.c:1195:5: warning:
> initialization from incompatible pointer type [enabled by default]
> hw/9pfs/virtio-9p-proxy.c:1195:5: warning: (near initialization for
> ‘proxy_ops.init’) [enabled by default]
> 
> Is this against another public tree?
>

Sorry, It depends on Aneesh's patch "Move opt validation to FsDriver callback"  
http://lists.gnu.org/archive/html/qemu-devel/2011-11/msg00275.html

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [Qemu-devel] [PATCH V4 04/13] hw/9pfs: File system helper process for qemu 9p proxy FS
  2011-12-08 18:31   ` Stefan Hajnoczi
@ 2011-12-09 16:42     ` M. Mohan Kumar
  2011-12-12 12:08       ` Stefan Hajnoczi
  0 siblings, 1 reply; 22+ messages in thread
From: M. Mohan Kumar @ 2011-12-09 16:42 UTC (permalink / raw)
  To: Stefan Hajnoczi; +Cc: M. Mohan Kumar, qemu-devel, aneesh.kumar

On Friday, December 09, 2011 12:01:14 AM Stefan Hajnoczi wrote:
> On Mon, Dec 05, 2011 at 09:48:41PM +0530, M. Mohan Kumar wrote:
> > +static int read_request(int sockfd, struct iovec *iovec, ProxyHeader
> > *header) +{
> > +    int retval;
> > +
> > +    /*
> > +     * read the request header.
> > +     */
> > +    iovec->iov_len = 0;
> > +    retval = socket_read(sockfd, iovec->iov_base, PROXY_HDR_SZ);
> > +    if (retval < 0) {
> > +        return retval;
> > +    }
> > +    iovec->iov_len = PROXY_HDR_SZ;
> > +    retval = proxy_unmarshal(iovec, 0, "dd", &header->type,
> > &header->size); +    if (retval < 0) {
> > +        return retval;
> > +    }
> > +    /*
> > +     * We can't process message.size > PROXY_MAX_IO_SZ, read the
> > complete +     * message from the socket and ignore it. This ensures
> > that +     * we can correctly handle the next request. We also return + 
> >    * ENOBUFS as error to indicate we ran out of buffer space. +     */
> > +    if (header->size > PROXY_MAX_IO_SZ) {
> > +        int count, size;
> > +        size = header->size;
> > +        while (size > 0) {
> > +            count = MIN(PROXY_MAX_IO_SZ, size);
> > +            count = socket_read(sockfd, iovec->iov_base + PROXY_HDR_SZ,
> > count); +            if (count < 0) {
> > +                return count;
> > +            }
> > +            size -= count;
> > +        }
> 
> I'm not sure recovery attempts are worthwhile here.  The client is
> buggy, perhaps just refuse further work.

But whats the issue in trying to recover in this case?
> 
> > +        return -ENOBUFS;
> > +    }
> 
> header->size is (signed) int and we didn't check for header->size < 0.
> Please use an unsigned type.

I will fix in next version

> 
> > +    if (chroot(rpath) < 0) {
> > +        do_perror("chroot");
> > +        goto error;
> > +    }
> > +    umask(0);
> 
> We haven't changed into the chroot yet, we need chdir("/").  Otherwise
> the current working directory is outside the chroot (and allows trivial
> escape).
I will fix in next version

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [Qemu-devel] [PATCH V4 04/13] hw/9pfs: File system helper process for qemu 9p proxy FS
  2011-12-09 16:42     ` M. Mohan Kumar
@ 2011-12-12 12:08       ` Stefan Hajnoczi
  2011-12-12 15:21         ` Aneesh Kumar K.V
  0 siblings, 1 reply; 22+ messages in thread
From: Stefan Hajnoczi @ 2011-12-12 12:08 UTC (permalink / raw)
  To: M. Mohan Kumar; +Cc: M. Mohan Kumar, qemu-devel, aneesh.kumar

On Fri, Dec 09, 2011 at 10:12:17PM +0530, M. Mohan Kumar wrote:
> On Friday, December 09, 2011 12:01:14 AM Stefan Hajnoczi wrote:
> > On Mon, Dec 05, 2011 at 09:48:41PM +0530, M. Mohan Kumar wrote:
> > > +static int read_request(int sockfd, struct iovec *iovec, ProxyHeader
> > > *header) +{
> > > +    int retval;
> > > +
> > > +    /*
> > > +     * read the request header.
> > > +     */
> > > +    iovec->iov_len = 0;
> > > +    retval = socket_read(sockfd, iovec->iov_base, PROXY_HDR_SZ);
> > > +    if (retval < 0) {
> > > +        return retval;
> > > +    }
> > > +    iovec->iov_len = PROXY_HDR_SZ;
> > > +    retval = proxy_unmarshal(iovec, 0, "dd", &header->type,
> > > &header->size); +    if (retval < 0) {
> > > +        return retval;
> > > +    }
> > > +    /*
> > > +     * We can't process message.size > PROXY_MAX_IO_SZ, read the
> > > complete +     * message from the socket and ignore it. This ensures
> > > that +     * we can correctly handle the next request. We also return + 
> > >    * ENOBUFS as error to indicate we ran out of buffer space. +     */
> > > +    if (header->size > PROXY_MAX_IO_SZ) {
> > > +        int count, size;
> > > +        size = header->size;
> > > +        while (size > 0) {
> > > +            count = MIN(PROXY_MAX_IO_SZ, size);
> > > +            count = socket_read(sockfd, iovec->iov_base + PROXY_HDR_SZ,
> > > count); +            if (count < 0) {
> > > +                return count;
> > > +            }
> > > +            size -= count;
> > > +        }
> > 
> > I'm not sure recovery attempts are worthwhile here.  The client is
> > buggy, perhaps just refuse further work.
> 
> But whats the issue in trying to recover in this case?

This recovery procedure is not robust because it does not always work.
In fact it only works in the case where the header->size field was
out-of-range but accurate.  That's not a likely case since the QEMU-side
code that you are writing should handle this.

If the nature of the invalid request is different, either a broken or
malicious client which does not send a valid header->size then we're
stuck in this special-case recovery trying to gobble bytes and we never
log an error.

A real recovery would be something like disconnecting and
re-establishing the connection between QEMU and the helper.  This would
allow us to get back to a clean state in all cases.

Stefan

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [Qemu-devel] [PATCH V4 04/13] hw/9pfs: File system helper process for qemu 9p proxy FS
  2011-12-12 12:08       ` Stefan Hajnoczi
@ 2011-12-12 15:21         ` Aneesh Kumar K.V
  2011-12-12 15:56           ` Stefan Hajnoczi
  0 siblings, 1 reply; 22+ messages in thread
From: Aneesh Kumar K.V @ 2011-12-12 15:21 UTC (permalink / raw)
  To: Stefan Hajnoczi, M. Mohan Kumar; +Cc: M. Mohan Kumar, qemu-devel

On Mon, 12 Dec 2011 12:08:33 +0000, Stefan Hajnoczi <stefanha@gmail.com> wrote:
> On Fri, Dec 09, 2011 at 10:12:17PM +0530, M. Mohan Kumar wrote:
> > On Friday, December 09, 2011 12:01:14 AM Stefan Hajnoczi wrote:
> > > On Mon, Dec 05, 2011 at 09:48:41PM +0530, M. Mohan Kumar wrote:
> > > > +static int read_request(int sockfd, struct iovec *iovec, ProxyHeader
> > > > *header) +{
> > > > +    int retval;
> > > > +
> > > > +    /*
> > > > +     * read the request header.
> > > > +     */
> > > > +    iovec->iov_len = 0;
> > > > +    retval = socket_read(sockfd, iovec->iov_base, PROXY_HDR_SZ);
> > > > +    if (retval < 0) {
> > > > +        return retval;
> > > > +    }
> > > > +    iovec->iov_len = PROXY_HDR_SZ;
> > > > +    retval = proxy_unmarshal(iovec, 0, "dd", &header->type,
> > > > &header->size); +    if (retval < 0) {
> > > > +        return retval;
> > > > +    }
> > > > +    /*
> > > > +     * We can't process message.size > PROXY_MAX_IO_SZ, read the
> > > > complete +     * message from the socket and ignore it. This ensures
> > > > that +     * we can correctly handle the next request. We also return + 
> > > >    * ENOBUFS as error to indicate we ran out of buffer space. +     */
> > > > +    if (header->size > PROXY_MAX_IO_SZ) {
> > > > +        int count, size;
> > > > +        size = header->size;
> > > > +        while (size > 0) {
> > > > +            count = MIN(PROXY_MAX_IO_SZ, size);
> > > > +            count = socket_read(sockfd, iovec->iov_base + PROXY_HDR_SZ,
> > > > count); +            if (count < 0) {
> > > > +                return count;
> > > > +            }
> > > > +            size -= count;
> > > > +        }
> > > 
> > > I'm not sure recovery attempts are worthwhile here.  The client is
> > > buggy, perhaps just refuse further work.
> > 
> > But whats the issue in trying to recover in this case?
> 
> This recovery procedure is not robust because it does not always work.
> In fact it only works in the case where the header->size field was
> out-of-range but accurate.  That's not a likely case since the QEMU-side
> code that you are writing should handle this.
> 
> If the nature of the invalid request is different, either a broken or
> malicious client which does not send a valid header->size then we're
> stuck in this special-case recovery trying to gobble bytes and we never
> log an error.
> 
> A real recovery would be something like disconnecting and
> re-establishing the connection between QEMU and the helper.  This would
> allow us to get back to a clean state in all cases.
> 

Since we are not having any state in the proxy helper, returning ENOBUFS
should be similar to the above right ? One of the reason to try to
recover as much as possible, is to make sure the guest can umount the
file system properly. That is if we hit these error condition due to a
bug in proxy FS driver is qemu, we want to make sure we return some
valid error, which will atleast enable the guest/client to do an umount.

-aneesh

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [Qemu-devel] [PATCH V4 04/13] hw/9pfs: File system helper process for qemu 9p proxy FS
  2011-12-12 15:21         ` Aneesh Kumar K.V
@ 2011-12-12 15:56           ` Stefan Hajnoczi
  0 siblings, 0 replies; 22+ messages in thread
From: Stefan Hajnoczi @ 2011-12-12 15:56 UTC (permalink / raw)
  To: Aneesh Kumar K.V; +Cc: M. Mohan Kumar, M. Mohan Kumar, qemu-devel

On Mon, Dec 12, 2011 at 3:21 PM, Aneesh Kumar K.V
<aneesh.kumar@linux.vnet.ibm.com> wrote:
> On Mon, 12 Dec 2011 12:08:33 +0000, Stefan Hajnoczi <stefanha@gmail.com> wrote:
>> On Fri, Dec 09, 2011 at 10:12:17PM +0530, M. Mohan Kumar wrote:
>> > On Friday, December 09, 2011 12:01:14 AM Stefan Hajnoczi wrote:
>> > > On Mon, Dec 05, 2011 at 09:48:41PM +0530, M. Mohan Kumar wrote:
>> > > > +static int read_request(int sockfd, struct iovec *iovec, ProxyHeader
>> > > > *header) +{
>> > > > +    int retval;
>> > > > +
>> > > > +    /*
>> > > > +     * read the request header.
>> > > > +     */
>> > > > +    iovec->iov_len = 0;
>> > > > +    retval = socket_read(sockfd, iovec->iov_base, PROXY_HDR_SZ);
>> > > > +    if (retval < 0) {
>> > > > +        return retval;
>> > > > +    }
>> > > > +    iovec->iov_len = PROXY_HDR_SZ;
>> > > > +    retval = proxy_unmarshal(iovec, 0, "dd", &header->type,
>> > > > &header->size); +    if (retval < 0) {
>> > > > +        return retval;
>> > > > +    }
>> > > > +    /*
>> > > > +     * We can't process message.size > PROXY_MAX_IO_SZ, read the
>> > > > complete +     * message from the socket and ignore it. This ensures
>> > > > that +     * we can correctly handle the next request. We also return +
>> > > >    * ENOBUFS as error to indicate we ran out of buffer space. +     */
>> > > > +    if (header->size > PROXY_MAX_IO_SZ) {
>> > > > +        int count, size;
>> > > > +        size = header->size;
>> > > > +        while (size > 0) {
>> > > > +            count = MIN(PROXY_MAX_IO_SZ, size);
>> > > > +            count = socket_read(sockfd, iovec->iov_base + PROXY_HDR_SZ,
>> > > > count); +            if (count < 0) {
>> > > > +                return count;
>> > > > +            }
>> > > > +            size -= count;
>> > > > +        }
>> > >
>> > > I'm not sure recovery attempts are worthwhile here.  The client is
>> > > buggy, perhaps just refuse further work.
>> >
>> > But whats the issue in trying to recover in this case?
>>
>> This recovery procedure is not robust because it does not always work.
>> In fact it only works in the case where the header->size field was
>> out-of-range but accurate.  That's not a likely case since the QEMU-side
>> code that you are writing should handle this.
>>
>> If the nature of the invalid request is different, either a broken or
>> malicious client which does not send a valid header->size then we're
>> stuck in this special-case recovery trying to gobble bytes and we never
>> log an error.
>>
>> A real recovery would be something like disconnecting and
>> re-establishing the connection between QEMU and the helper.  This would
>> allow us to get back to a clean state in all cases.
>>
>
> Since we are not having any state in the proxy helper, returning ENOBUFS
> should be similar to the above right ? One of the reason to try to
> recover as much as possible, is to make sure the guest can umount the
> file system properly. That is if we hit these error condition due to a
> bug in proxy FS driver is qemu, we want to make sure we return some
> valid error, which will atleast enable the guest/client to do an umount.

When the helper detects something outside the protocol specification
it needs to terminate the connection.  The protocol has no reliable
way to skip the junk coming over the socket so we can't process the
"next" message.

The flipside to "try to recover as much as possible" is "damage as
little as possible".  We don't want to mis-interpret requests on this
broken connection and corrupt the user's data.

I'm happy with any scheme as long as it handles all error cases.  The
problem with the -ENOBUFS case was that it is pretty artificial
(unlikely to happen) and doesn't handle cases where header->size is
inaccurate.

Stefan

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2011-12-12 15:56 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-12-05 16:18 [Qemu-devel] [PATCH V4 00/13] Proxy FS driver for VirtFS M. Mohan Kumar
2011-12-05 16:18 ` [Qemu-devel] [PATCH V4 01/13] hw/9pfs: Move pdu_marshal/unmarshal code to a seperate file M. Mohan Kumar
2011-12-05 16:18 ` [Qemu-devel] [PATCH V4 02/13] hw/9pfs: Add validation to {un}marshal code M. Mohan Kumar
2011-12-08 18:10   ` Stefan Hajnoczi
2011-12-05 16:18 ` [Qemu-devel] [PATCH V4 03/13] hw/9pfs: Add new proxy filesystem driver M. Mohan Kumar
2011-12-05 16:18 ` [Qemu-devel] [PATCH V4 04/13] hw/9pfs: File system helper process for qemu 9p proxy FS M. Mohan Kumar
2011-12-08 18:31   ` Stefan Hajnoczi
2011-12-09 16:42     ` M. Mohan Kumar
2011-12-12 12:08       ` Stefan Hajnoczi
2011-12-12 15:21         ` Aneesh Kumar K.V
2011-12-12 15:56           ` Stefan Hajnoczi
2011-12-05 16:18 ` [Qemu-devel] [PATCH V4 05/13] hw/9pfs: Open and create files M. Mohan Kumar
2011-12-05 16:18 ` [Qemu-devel] [PATCH V4 06/13] hw/9pfs: Create other filesystem objects M. Mohan Kumar
2011-12-05 16:18 ` [Qemu-devel] [PATCH V4 07/13] hw/9pfs: Add stat/readlink/statfs for proxy FS M. Mohan Kumar
2011-12-05 16:18 ` [Qemu-devel] [PATCH V4 08/13] hw/9pfs: File ownership and others M. Mohan Kumar
2011-12-05 16:18 ` [Qemu-devel] [PATCH V4 09/13] hw/9pfs: xattr interfaces in proxy filesystem driver M. Mohan Kumar
2011-12-05 16:18 ` [Qemu-devel] [PATCH V4 10/13] hw/9pfs: Proxy getversion M. Mohan Kumar
2011-12-05 16:18 ` [Qemu-devel] [PATCH V4 11/13] hw/9pfs: Documentation changes related to proxy fs M. Mohan Kumar
2011-12-05 16:18 ` [Qemu-devel] [PATCH V4 12/13] hw/9pfs: man page for proxy helper M. Mohan Kumar
2011-12-05 16:18 ` [Qemu-devel] [PATCH V4 13/13] hw/9pfs: Add support to use named socket for proxy FS M. Mohan Kumar
2011-12-08 17:46 ` [Qemu-devel] [PATCH V4 00/13] Proxy FS driver for VirtFS Stefan Hajnoczi
2011-12-09 14:47   ` M. Mohan Kumar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).