From ddca7f86ac022289840e0200fd4050b2b58e9176 Mon Sep 17 00:00:00 2001 From: "M. Mohan Kumar" Date: Wed, 14 Dec 2011 13:49:13 +0530 Subject: [PATCH] hw/9pfs: Add validation to {un}marshal code Signed-off-by: M. Mohan Kumar Signed-off-by: Aneesh Kumar K.V --- fsdev/virtio-9p-marshal.c | 148 ++++++------ fsdev/virtio-9p-marshal.h | 19 +- hw/9pfs/virtio-9p.c | 463 ++++++++++++++++++++++++++++---------- 3 files changed, 435 insertions(+), 195 deletions(-) diff --git a/fsdev/virtio-9p-marshal.c b/fsdev/virtio-9p-marshal.c index 38fb99e72e..bf980bfa38 100644 --- a/fsdev/virtio-9p-marshal.c +++ b/fsdev/virtio-9p-marshal.c @@ -20,17 +20,12 @@ #include #include #include +#include #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,27 +87,33 @@ 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); } -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 = 0; size_t old_offset = offset; va_start(ap, fmt); @@ -118,13 +121,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 { @@ -135,7 +138,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 { @@ -146,7 +149,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 { @@ -156,59 +159,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 = 0; size_t old_offset = offset; va_start(ap, fmt); @@ -216,7 +230,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': { @@ -226,7 +240,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': { @@ -236,7 +250,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': { @@ -246,37 +260,40 @@ 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 '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, @@ -294,6 +311,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 f4414006dc..5df65a8357 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 4bfee6cdf7..e6ba6ba30b 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); @@ -886,12 +900,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); @@ -904,11 +924,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; } @@ -924,7 +948,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); @@ -945,8 +975,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; @@ -973,7 +1007,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); @@ -989,10 +1026,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); @@ -1012,7 +1053,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); @@ -1038,8 +1082,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); @@ -1072,7 +1119,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) { @@ -1147,10 +1197,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; } @@ -1171,8 +1231,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); @@ -1180,7 +1244,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; @@ -1279,9 +1347,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); @@ -1303,8 +1374,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); @@ -1333,8 +1407,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); @@ -1357,8 +1434,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); @@ -1384,8 +1465,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: @@ -1405,7 +1489,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); @@ -1431,7 +1518,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); @@ -1454,6 +1544,7 @@ out_nofid: static int v9fs_xattr_read(V9fsState *s, V9fsPDU *pdu, V9fsFidState *fidp, uint64_t off, uint32_t max_count) { + ssize_t err; size_t offset = 7; int read_count; int64_t xattr_len; @@ -1468,11 +1559,18 @@ static int v9fs_xattr_read(V9fsState *s, V9fsPDU *pdu, V9fsFidState *fidp, */ 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; } @@ -1581,7 +1679,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); @@ -1599,9 +1700,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) { QEMUIOVector qiov_full; QEMUIOVector qiov; @@ -1629,9 +1732,11 @@ static void v9fs_read(void *opaque) goto out; } } 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; qemu_iovec_destroy(&qiov); qemu_iovec_destroy(&qiov_full); } else if (fidp->fid_type == P9_FID_XATTR) { @@ -1703,6 +1808,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; @@ -1726,8 +1837,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); @@ -1749,9 +1863,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); @@ -1782,8 +1898,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 @@ -1818,7 +1937,11 @@ static void v9fs_write(void *opaque) QEMUIOVector qiov_full; QEMUIOVector qiov; - offset += pdu_unmarshal(pdu, offset, "dqd", &fid, &off, &count); + err = pdu_unmarshal(pdu, offset, "dqd", &fid, &off, &count); + if (err < 0) { + return complete_pdu(s, pdu, err); + } + offset += err; v9fs_init_qiov_from_pdu(&qiov_full, pdu, offset, count, true); trace_v9fs_write(pdu->tag, pdu->id, fid, off, count, qiov_full.niov); @@ -1866,8 +1989,11 @@ static void v9fs_write(void *opaque) } while (total < count && len > 0); offset = 7; - 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_qiov: qemu_iovec_destroy(&qiov); @@ -1895,10 +2021,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); @@ -2029,8 +2158,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: @@ -2055,7 +2187,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); @@ -2068,8 +2205,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: @@ -2082,13 +2222,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) { @@ -2119,7 +2264,11 @@ static void v9fs_link(void *opaque) 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); @@ -2153,7 +2302,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); @@ -2196,8 +2348,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; @@ -2299,8 +2454,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; @@ -2405,8 +2563,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, @@ -2415,6 +2578,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); @@ -2432,7 +2597,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); @@ -2566,7 +2735,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; @@ -2576,8 +2748,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: @@ -2601,8 +2776,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); @@ -2616,8 +2795,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: @@ -2638,7 +2820,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; @@ -2646,18 +2828,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); - - trace_v9fs_lock(pdu->tag, pdu->id, fid, - flock->type, flock->start, flock->length); - 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); + /* 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; } @@ -2674,12 +2858,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); } /* @@ -2691,18 +2876,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) { @@ -2713,19 +2900,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) @@ -2741,8 +2930,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); @@ -2755,8 +2947,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: @@ -2778,7 +2973,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); @@ -2792,7 +2991,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 */ @@ -2818,8 +3017,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 @@ -2848,8 +3050,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: @@ -2875,8 +3080,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); @@ -2913,7 +3121,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) { @@ -2926,8 +3137,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: