| Message ID | 57fef740.ca13c20a.19dbb.98b6@mx.google.com |
|---|---|
| State | New |
| Headers | show |
On Wed, 12 Oct 2016 19:53:39 -0700 Li Qiang <liq3ea@gmail.com> wrote: > From: Li Qiang <liqiang6-s@360.cn> > > In 9pfs xattr read/write function, it mix to use unsigned/signed > ,32/64 bits integers. This will causes oob read/write issues. This patch > fix this. > I suggested a more detailed changelog. > Signed-off-by: Li Qiang <liqiang6-s@360.cn> > --- > hw/9pfs/9p.c | 34 +++++++++++++--------------------- > hw/9pfs/9p.h | 4 ++-- > 2 files changed, 15 insertions(+), 23 deletions(-) > > diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c > index e4040dc..8c7488f 100644 > --- a/hw/9pfs/9p.c > +++ b/hw/9pfs/9p.c > @@ -1642,20 +1642,17 @@ static int v9fs_xattr_read(V9fsState *s, V9fsPDU *pdu, V9fsFidState *fidp, > { > ssize_t err; > size_t offset = 7; > - int read_count; > - int64_t xattr_len; > + uint64_t read_count; > V9fsVirtioState *v = container_of(s, V9fsVirtioState, state); > VirtQueueElement *elem = v->elems[pdu->idx]; > > - xattr_len = fidp->fs.xattr.len; > - read_count = xattr_len - off; > + if (fidp->fs.xattr.len < off) { > + read_count = 0; > + } else { > + read_count = fidp->fs.xattr.len - off; > + } > if (read_count > max_count) { > read_count = max_count; > - } else if (read_count < 0) { > - /* > - * read beyond XATTR value > - */ > - read_count = 0; > } > err = pdu_marshal(pdu, offset, "d", read_count); > if (err < 0) { > @@ -1982,23 +1979,18 @@ static int v9fs_xattr_write(V9fsState *s, V9fsPDU *pdu, V9fsFidState *fidp, > { > int i, to_copy; > ssize_t err = 0; > - int write_count; > - int64_t xattr_len; > + uint64_t write_count; > size_t offset = 7; > > > - xattr_len = fidp->fs.xattr.len; > - write_count = xattr_len - off; > - if (write_count > count) { > - write_count = count; > - } else if (write_count < 0) { > - /* > - * write beyond XATTR value len specified in > - * xattrcreate > - */ > + if (fidp->fs.xattr.len < off) { > err = -ENOSPC; > goto out; > } > + write_count = fidp->fs.xattr.len - off; > + if (write_count > count) { > + write_count = count; > + } > err = pdu_marshal(pdu, offset, "d", write_count); > if (err < 0) { > return err; > @@ -3254,7 +3246,7 @@ static void v9fs_xattrcreate(void *opaque) > { > int flags; > int32_t fid; > - int64_t size; > + uint64_t size; > ssize_t err = 0; > V9fsString name; > size_t offset = 7; > diff --git a/hw/9pfs/9p.h b/hw/9pfs/9p.h > index d539d2e..22198f6 100644 > --- a/hw/9pfs/9p.h > +++ b/hw/9pfs/9p.h > @@ -159,8 +159,8 @@ typedef struct V9fsConf > > typedef struct V9fsXattr > { > - int64_t copied_len; > - int64_t len; > + uint64_t copied_len; > + uint64_t len; I had asked this to go to a separate patch... with the appropriate justifications that these are byte counts and size_t/u64 in the guest. > void *value; > V9fsString name; > int flags;
diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c index e4040dc..8c7488f 100644 --- a/hw/9pfs/9p.c +++ b/hw/9pfs/9p.c @@ -1642,20 +1642,17 @@ static int v9fs_xattr_read(V9fsState *s, V9fsPDU *pdu, V9fsFidState *fidp, { ssize_t err; size_t offset = 7; - int read_count; - int64_t xattr_len; + uint64_t read_count; V9fsVirtioState *v = container_of(s, V9fsVirtioState, state); VirtQueueElement *elem = v->elems[pdu->idx]; - xattr_len = fidp->fs.xattr.len; - read_count = xattr_len - off; + if (fidp->fs.xattr.len < off) { + read_count = 0; + } else { + read_count = fidp->fs.xattr.len - off; + } if (read_count > max_count) { read_count = max_count; - } else if (read_count < 0) { - /* - * read beyond XATTR value - */ - read_count = 0; } err = pdu_marshal(pdu, offset, "d", read_count); if (err < 0) { @@ -1982,23 +1979,18 @@ static int v9fs_xattr_write(V9fsState *s, V9fsPDU *pdu, V9fsFidState *fidp, { int i, to_copy; ssize_t err = 0; - int write_count; - int64_t xattr_len; + uint64_t write_count; size_t offset = 7; - xattr_len = fidp->fs.xattr.len; - write_count = xattr_len - off; - if (write_count > count) { - write_count = count; - } else if (write_count < 0) { - /* - * write beyond XATTR value len specified in - * xattrcreate - */ + if (fidp->fs.xattr.len < off) { err = -ENOSPC; goto out; } + write_count = fidp->fs.xattr.len - off; + if (write_count > count) { + write_count = count; + } err = pdu_marshal(pdu, offset, "d", write_count); if (err < 0) { return err; @@ -3254,7 +3246,7 @@ static void v9fs_xattrcreate(void *opaque) { int flags; int32_t fid; - int64_t size; + uint64_t size; ssize_t err = 0; V9fsString name; size_t offset = 7; diff --git a/hw/9pfs/9p.h b/hw/9pfs/9p.h index d539d2e..22198f6 100644 --- a/hw/9pfs/9p.h +++ b/hw/9pfs/9p.h @@ -159,8 +159,8 @@ typedef struct V9fsConf typedef struct V9fsXattr { - int64_t copied_len; - int64_t len; + uint64_t copied_len; + uint64_t len; void *value; V9fsString name; int flags;