Patchwork [V4,02/13] hw/9pfs: Add validation to {un}marshal code

login
register
mail settings
Submitter M. Mohan Kumar
Date Dec. 5, 2011, 4:18 p.m.
Message ID <1323101930-27163-3-git-send-email-mohan@in.ibm.com>
Download mbox | patch
Permalink /patch/129353/
State New
Headers show

Comments

M. Mohan Kumar - Dec. 5, 2011, 4:18 p.m.
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(-)
Stefan Hajnoczi - Dec. 8, 2011, 6:10 p.m.
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

Patch

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: