9pfs: fix integer overflow issue in xattr read/write
PATCH v2 1/1 9pfs: fix integer overflow issue in xattr read/write
From: Li Qiang <[email protected]>
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.
Signed-off-by Li Qiang <[email protected]>
--- hw/9pfs/9p.c | 34 +++++++++++++--------------------- hw/9pfs/9p.h | 4 ++-- 2 files changed, 15 insertions(+), 23 deletions(-)
hw/9pfs/9p.c +13 -21
@@ -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 ;
hw/9pfs/9p.h +2 -2
@@ -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;
Signature · 1 line
1.8.3.1
Quoted 6 lines · On Wed, 12 Oct 2016 19:53:39 -0700 Li Qiang <[email protected]> wrote:
From: Li Qiang <[email protected]>
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.
Quoted 81 lines
Signed-off-by Li Qiang <[email protected]>--- hw/9pfs/9p.c | 34 +++++++++++++--------------------- hw/9pfs/9p.h | 4 ++-- 2 files changed, 15 insertions(+), 23 deletions(-)hw/9pfs/9p.c +13
-21@@ -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 ;hw/9pfs/9p.h +0
-0
hw/9pfs/9p.h typedef struct V9fsConf · line 163
Quoted 7 lines
hw/9pfs/9p.h +2
-2@@ -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.
Quoted 3 lines
hw/9pfs/9p.h +0
-0void *value;V9fsString name;int flags;