On Mon, 21 Nov 2011 19:06:07 +0530, "M. Mohan Kumar" <mo...@in.ibm.com> wrote: > From: "M. Mohan Kumar" <mo...@in.ibm.com> > > Add validatio check to {un}marshal code. > > Signed-off-by: M. Mohan Kumar <mo...@in.ibm.com> > --- > fsdev/virtio-9p-marshal.c | 97 ++++++++++++------- > fsdev/virtio-9p-marshal.h | 8 +- > hw/9pfs/virtio-9p.c | 231 > +++++++++++++++++++++++++++++++++------------ > 3 files changed, 236 insertions(+), 100 deletions(-) > > diff --git a/fsdev/virtio-9p-marshal.c b/fsdev/virtio-9p-marshal.c > index 2da0a34..74161df 100644 > --- a/fsdev/virtio-9p-marshal.c > +++ b/fsdev/virtio-9p-marshal.c > @@ -62,14 +62,14 @@ 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, > + ssize_t offset, ssize_t size, int pack) > { > int i = 0; > - size_t copied = 0; > + ssize_t copied = 0; > > for (i = 0; size && i < sg_count; i++) { > - size_t len; > + ssize_t len; > if (offset >= sg[i].iov_len) { > /* skip this sg */ > offset -= sg[i].iov_len; > @@ -91,25 +91,29 @@ static size_t v9fs_packunpack(void *addr, struct iovec > *sg, int sg_count, > } > } > > + /* could not copy requested 'size' */ > + if (copied < 0) { > + return -1; > + }
I am not sure copied will be < 0 here even in error case > 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, > + ssize_t offset, ssize_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, ssize_t offset, > + const void *src, ssize_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) > + ssize_t offset, struct iovec *sg) > { > - size_t pos = 0; > + ssize_t pos = 0; > int i, j; > > j = 0; > @@ -131,10 +135,10 @@ 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 convert, const char *fmt, ...) > +ssize_t v9fs_unmarshal(struct iovec *out_sg, > + int out_num, ssize_t offset, int convert, const char *fmt, > ...) > { > - size_t old_offset = offset; > + ssize_t old_offset = offset, copied; > va_list ap; > int i; > > @@ -143,13 +147,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; > } Do we really need error checking for unpack ? unpack reads the value from the buffer and store them in arguments passed. So what is the failure condition here ? We always will have enough space to unpack. > 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 (convert) { > *valp = le16_to_cpu(val); > } else { -aneesh