6 changed files with 198 additions and 2 deletions
@ -0,0 +1,21 @@ |
|||
From: Li Qiang <address@hidden> |
|||
|
|||
The 'fs.xattr.value' field in V9fsFidState object doesn't consider the |
|||
situation that this field has been allocated previously. Every time, it |
|||
will be allocated directly. This leads a host memory leak issue. This |
|||
patch fix this. |
|||
|
|||
--
|
|||
1.8.3.1 |
|||
diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
|
|||
index 75ba5f1..a4c7109 100644
|
|||
--- a/hw/9pfs/9p.c
|
|||
+++ b/hw/9pfs/9p.c
|
|||
@@ -3269,6 +3269,7 @@ static void v9fs_xattrcreate(void *opaque)
|
|||
xattr_fidp->fs.xattr.flags = flags; |
|||
v9fs_string_init(&xattr_fidp->fs.xattr.name); |
|||
v9fs_string_copy(&xattr_fidp->fs.xattr.name, &name); |
|||
+ g_free(xattr_fidp->fs.xattr.value);
|
|||
xattr_fidp->fs.xattr.value = g_malloc(size); |
|||
err = offset; |
|||
put_fid(pdu, file_fidp); |
|||
@ -0,0 +1,27 @@ |
|||
Author: Li Qiang <liqiang6-s@360.cn> |
|||
Date: Mon Oct 17 14:13:58 2016 +0200 |
|||
|
|||
9pfs: fix information leak in xattr read |
|||
|
|||
9pfs uses g_malloc() to allocate the xattr memory space, if the guest |
|||
reads this memory before writing to it, this will leak host heap memory |
|||
to the guest. This patch avoid this. |
|||
|
|||
Signed-off-by: Li Qiang <liqiang6-s@360.cn> |
|||
Reviewed-by: Greg Kurz <groug@kaod.org> |
|||
Signed-off-by: Greg Kurz <groug@kaod.org> |
|||
|
|||
diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
|
|||
index 26aa7d5..bf23b01 100644
|
|||
--- a/hw/9pfs/9p.c
|
|||
+++ b/hw/9pfs/9p.c
|
|||
@@ -3269,8 +3269,8 @@ static void coroutine_fn v9fs_xattrcreate(void *opaque)
|
|||
xattr_fidp->fs.xattr.flags = flags; |
|||
v9fs_string_init(&xattr_fidp->fs.xattr.name); |
|||
v9fs_string_copy(&xattr_fidp->fs.xattr.name, &name); |
|||
g_free(xattr_fidp->fs.xattr.value); |
|||
- xattr_fidp->fs.xattr.value = g_malloc(size);
|
|||
+ xattr_fidp->fs.xattr.value = g_malloc0(size);
|
|||
err = offset; |
|||
put_fid(pdu, file_fidp); |
|||
out_nofid: |
|||
@ -0,0 +1,92 @@ |
|||
From 7e55d65c56a03dcd2c5d7c49d37c5a74b55d4bd6 Mon Sep 17 00:00:00 2001 |
|||
From: Li Qiang <liqiang6-s@360.cn> |
|||
Date: Tue, 1 Nov 2016 12:00:40 +0100 |
|||
Subject: [PATCH] 9pfs: fix integer overflow issue in xattr read/write |
|||
MIME-Version: 1.0 |
|||
Content-Type: text/plain; charset=UTF-8 |
|||
Content-Transfer-Encoding: 8bit |
|||
|
|||
The v9fs_xattr_read() and v9fs_xattr_write() are passed a guest |
|||
originated offset: they must ensure this offset does not go beyond |
|||
the size of the extended attribute that was set in v9fs_xattrcreate(). |
|||
Unfortunately, the current code implement these checks with unsafe |
|||
calculations on 32 and 64 bit values, which may allow a malicious |
|||
guest to cause OOB access anyway. |
|||
|
|||
Fix this by comparing the offset and the xattr size, which are |
|||
both uint64_t, before trying to compute the effective number of bytes |
|||
to read or write. |
|||
|
|||
Suggested-by: Greg Kurz <groug@kaod.org> |
|||
Signed-off-by: Li Qiang <liqiang6-s@360.cn> |
|||
Reviewed-by: Greg Kurz <groug@kaod.org> |
|||
Reviewed-By: Guido Günther <agx@sigxcpu.org> |
|||
Signed-off-by: Greg Kurz <groug@kaod.org> |
|||
---
|
|||
hw/9pfs/9p.c | 32 ++++++++++++-------------------- |
|||
1 file changed, 12 insertions(+), 20 deletions(-) |
|||
|
|||
diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
|
|||
index ab18ef2..7705ead 100644
|
|||
--- a/hw/9pfs/9p.c
|
|||
+++ b/hw/9pfs/9p.c
|
|||
@@ -1637,20 +1637,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) { |
|||
@@ -1979,23 +1976,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; |
|||
--
|
|||
2.7.3 |
|||
|
|||
@ -0,0 +1,25 @@ |
|||
From: Li Qiang <address@hidden> |
|||
|
|||
In v9fs_link dispatch function, it doesn't put the 'oldfidp' |
|||
fid object, this will make the 'oldfidp->ref' never reach to 0, |
|||
thus leading a memory leak issue. This patch fix this. |
|||
|
|||
Signed-off-by: Li Qiang <address@hidden> |
|||
---
|
|||
hw/9pfs/9p.c | 1 + |
|||
1 file changed, 1 insertion(+) |
|||
|
|||
diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
|
|||
index 8b50bfb..29f8b7a 100644
|
|||
--- a/hw/9pfs/9p.c
|
|||
+++ b/hw/9pfs/9p.c
|
|||
@@ -2413,6 +2413,7 @@ static void v9fs_link(void *opaque)
|
|||
if (!err) { |
|||
err = offset; |
|||
} |
|||
+ put_fid(pdu, oldfidp);
|
|||
out: |
|||
put_fid(pdu, dfidp); |
|||
out_nofid: |
|||
--
|
|||
1.8.3.1 |
|||
@ -0,0 +1,27 @@ |
|||
Author: Li Qiang <liqiang6-s@360.cn> |
|||
Date: Mon Oct 17 14:13:58 2016 +0200 |
|||
|
|||
9pfs: fix memory leak in v9fs_write |
|||
|
|||
If an error occurs when marshalling the transfer length to the guest, the |
|||
v9fs_write() function doesn't free an IO vector, thus leading to a memory |
|||
leak. This patch fixes the issue. |
|||
|
|||
Signed-off-by: Li Qiang <liqiang6-s@360.cn> |
|||
Reviewed-by: Greg Kurz <groug@kaod.org> |
|||
[groug, rephrased the changelog] |
|||
Signed-off-by: Greg Kurz <groug@kaod.org> |
|||
|
|||
diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
|
|||
index d43a552..e88cf25 100644
|
|||
--- a/hw/9pfs/9p.c
|
|||
+++ b/hw/9pfs/9p.c
|
|||
@@ -2090,7 +2090,7 @@ static void coroutine_fn v9fs_write(void *opaque)
|
|||
offset = 7; |
|||
err = pdu_marshal(pdu, offset, "d", total); |
|||
if (err < 0) { |
|||
- goto out;
|
|||
+ goto out_qiov;
|
|||
} |
|||
err += offset; |
|||
|
|||
Loading…
Reference in new issue