qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v3 00/10] fix three bugs about use-after-free and several api abuse
@ 2014-08-07  8:01 zhanghailiang
  2014-08-07  8:01 ` [Qemu-devel] [PATCH v3 01/10] l2cap: fix access freed memory zhanghailiang
                   ` (9 more replies)
  0 siblings, 10 replies; 20+ messages in thread
From: zhanghailiang @ 2014-08-07  8:01 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, lkurusa, zhanghailiang, mst, jan.kiszka, riku.voipio, mjt,
	peter.huangpeng, lcapitulino, stefanha, luonengjun, pbonzini,
	alex.bennee, rth

Hi,

Besides fstat(), I have also found when call malloc() and fopen(),
there are serveral places that do not check their return value.

Though it is a small probability for the two functions to fail,
but it is better to fix them.

So i added these patches to this patch-set

v2 -> v3:
-ivshmem: change the error message which advised by Levente Kurusa 
-others: add six new patches which check the return value of malloc() and fopen(),
- which may be failed.

v1 -> v2:
-ivshmem: modified the log message according to reviewing suggestion of Michael

Li Liu (3):
  tcg: check return value of fopen()
  block/vvfat: fix setbuf stream parameter may be NULL
  qtest: check the value returned by fopen()

zhanghailiang (7):
  l2cap: fix access freed memory
  monitor: fix access freed memory
  virtio-blk: fix reference a pointer which might be freed
  ivshmem: check the value returned by fstat()
  util/path: check return value of malloc()
  slirp: check return value of malloc()
  linux-user: check return value of malloc()

 block/vvfat.c            | 5 ++++-
 hw/block/virtio-blk.c    | 5 +++--
 hw/bt/l2cap.c            | 2 +-
 hw/misc/ivshmem.c        | 6 +++++-
 linux-user/syscall.c     | 3 +++
 monitor.c                | 4 +++-
 qtest.c                  | 5 +++++
 slirp/misc.c             | 8 ++++++--
 tcg/tcg.c                | 4 ++++
 tests/bios-tables-test.c | 2 ++
 util/path.c              | 9 ++++++---
 11 files changed, 42 insertions(+), 11 deletions(-)

-- 
1.7.12.4

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Qemu-devel] [PATCH v3 01/10] l2cap: fix access freed memory
  2014-08-07  8:01 [Qemu-devel] [PATCH v3 00/10] fix three bugs about use-after-free and several api abuse zhanghailiang
@ 2014-08-07  8:01 ` zhanghailiang
  2014-08-07  8:01 ` [Qemu-devel] [PATCH v3 02/10] monitor: " zhanghailiang
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 20+ messages in thread
From: zhanghailiang @ 2014-08-07  8:01 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, lkurusa, zhanghailiang, mst, jan.kiszka, riku.voipio, mjt,
	peter.huangpeng, lcapitulino, stefanha, luonengjun, pbonzini,
	alex.bennee, rth

Pointer 'ch' will be used in function 'l2cap_channel_open_req_msg' after
it was previously freed in 'l2cap_channel_open'.
Assigned it to NULL after it is freed.

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
---
 hw/bt/l2cap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/bt/l2cap.c b/hw/bt/l2cap.c
index 2301d6f..591e047 100644
--- a/hw/bt/l2cap.c
+++ b/hw/bt/l2cap.c
@@ -429,7 +429,7 @@ static struct l2cap_chan_s *l2cap_channel_open(struct l2cap_instance_s *l2cap,
                 status = L2CAP_CS_NO_INFO;
             } else {
                 g_free(ch);
-
+                ch = NULL;
                 result = L2CAP_CR_NO_MEM;
                 status = L2CAP_CS_NO_INFO;
             }
-- 
1.7.12.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [Qemu-devel] [PATCH v3 02/10] monitor: fix access freed memory
  2014-08-07  8:01 [Qemu-devel] [PATCH v3 00/10] fix three bugs about use-after-free and several api abuse zhanghailiang
  2014-08-07  8:01 ` [Qemu-devel] [PATCH v3 01/10] l2cap: fix access freed memory zhanghailiang
@ 2014-08-07  8:01 ` zhanghailiang
  2014-08-07 11:01   ` Gonglei (Arei)
  2014-08-07  8:01 ` [Qemu-devel] [PATCH v3 03/10] virtio-blk: fix reference a pointer which might be freed zhanghailiang
                   ` (7 subsequent siblings)
  9 siblings, 1 reply; 20+ messages in thread
From: zhanghailiang @ 2014-08-07  8:01 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, lkurusa, zhanghailiang, mst, jan.kiszka, riku.voipio, mjt,
	peter.huangpeng, lcapitulino, stefanha, luonengjun, pbonzini,
	alex.bennee, rth

The function monitor_fdset_dup_fd_find_remove() references member of 'mon_fdset'
which may be freed in function monitor_fdset_cleanup()

Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
---
 monitor.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/monitor.c b/monitor.c
index 5bc70a6..41e46a6 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2532,8 +2532,10 @@ static int monitor_fdset_dup_fd_find_remove(int dup_fd, bool remove)
 {
     MonFdset *mon_fdset;
     MonFdsetFd *mon_fdset_fd_dup;
+    int64_t id = -1;
 
     QLIST_FOREACH(mon_fdset, &mon_fdsets, next) {
+        id = mon_fdset->id;
         QLIST_FOREACH(mon_fdset_fd_dup, &mon_fdset->dup_fds, next) {
             if (mon_fdset_fd_dup->fd == dup_fd) {
                 if (remove) {
@@ -2542,7 +2544,7 @@ static int monitor_fdset_dup_fd_find_remove(int dup_fd, bool remove)
                         monitor_fdset_cleanup(mon_fdset);
                     }
                 }
-                return mon_fdset->id;
+                return id;
             }
         }
     }
-- 
1.7.12.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [Qemu-devel] [PATCH v3 03/10] virtio-blk: fix reference a pointer which might be freed
  2014-08-07  8:01 [Qemu-devel] [PATCH v3 00/10] fix three bugs about use-after-free and several api abuse zhanghailiang
  2014-08-07  8:01 ` [Qemu-devel] [PATCH v3 01/10] l2cap: fix access freed memory zhanghailiang
  2014-08-07  8:01 ` [Qemu-devel] [PATCH v3 02/10] monitor: " zhanghailiang
@ 2014-08-07  8:01 ` zhanghailiang
  2014-08-07  8:01 ` [Qemu-devel] [PATCH v3 04/10] ivshmem: check the value returned by fstat() zhanghailiang
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 20+ messages in thread
From: zhanghailiang @ 2014-08-07  8:01 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, lkurusa, zhanghailiang, mst, jan.kiszka, riku.voipio, mjt,
	peter.huangpeng, lcapitulino, stefanha, luonengjun, pbonzini,
	alex.bennee, rth

In function virtio_blk_handle_request, it may freed memory pointed by req,
So do not access member of req after calling this function.

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
---
 hw/block/virtio-blk.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index c241c50..54a853a 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -458,7 +458,7 @@ static void virtio_blk_handle_output(VirtIODevice *vdev, VirtQueue *vq)
 static void virtio_blk_dma_restart_bh(void *opaque)
 {
     VirtIOBlock *s = opaque;
-    VirtIOBlockReq *req = s->rq;
+    VirtIOBlockReq *req = s->rq, *next = NULL;
     MultiReqBuffer mrb = {
         .num_writes = 0,
     };
@@ -469,8 +469,9 @@ static void virtio_blk_dma_restart_bh(void *opaque)
     s->rq = NULL;
 
     while (req) {
+        next = req->next;
         virtio_blk_handle_request(req, &mrb);
-        req = req->next;
+        req = next;
     }
 
     virtio_submit_multiwrite(s->bs, &mrb);
-- 
1.7.12.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [Qemu-devel] [PATCH v3 04/10] ivshmem: check the value returned by fstat()
  2014-08-07  8:01 [Qemu-devel] [PATCH v3 00/10] fix three bugs about use-after-free and several api abuse zhanghailiang
                   ` (2 preceding siblings ...)
  2014-08-07  8:01 ` [Qemu-devel] [PATCH v3 03/10] virtio-blk: fix reference a pointer which might be freed zhanghailiang
@ 2014-08-07  8:01 ` zhanghailiang
  2014-08-07  8:53   ` Levente Kurusa
  2014-08-07  8:01 ` [Qemu-devel] [PATCH v3 05/10] util/path: check return value of malloc() zhanghailiang
                   ` (5 subsequent siblings)
  9 siblings, 1 reply; 20+ messages in thread
From: zhanghailiang @ 2014-08-07  8:01 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, lkurusa, zhanghailiang, mst, jan.kiszka, riku.voipio, mjt,
	peter.huangpeng, lcapitulino, stefanha, luonengjun, pbonzini,
	alex.bennee, rth

The function fstat() may fail, so check its return value.

Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
---
 hw/misc/ivshmem.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
index 768e528..2be4b86 100644
--- a/hw/misc/ivshmem.c
+++ b/hw/misc/ivshmem.c
@@ -324,7 +324,11 @@ static int check_shm_size(IVShmemState *s, int fd) {
 
     struct stat buf;
 
-    fstat(fd, &buf);
+    if (fstat(fd, &buf) < 0) {
+        fprintf(stderr, "ivshmem: exiting: fstat on fd %d failed: %s\n",
+                fd, strerror(errno));
+        return -1;
+    }
 
     if (s->ivshmem_size > buf.st_size) {
         fprintf(stderr,
-- 
1.7.12.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [Qemu-devel] [PATCH v3 05/10] util/path: check return value of malloc()
  2014-08-07  8:01 [Qemu-devel] [PATCH v3 00/10] fix three bugs about use-after-free and several api abuse zhanghailiang
                   ` (3 preceding siblings ...)
  2014-08-07  8:01 ` [Qemu-devel] [PATCH v3 04/10] ivshmem: check the value returned by fstat() zhanghailiang
@ 2014-08-07  8:01 ` zhanghailiang
  2014-08-07 11:04   ` Gonglei (Arei)
  2014-08-07  8:01 ` [Qemu-devel] [PATCH v3 06/10] slirp: " zhanghailiang
                   ` (4 subsequent siblings)
  9 siblings, 1 reply; 20+ messages in thread
From: zhanghailiang @ 2014-08-07  8:01 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, lkurusa, zhanghailiang, mst, jan.kiszka, riku.voipio, mjt,
	peter.huangpeng, lcapitulino, stefanha, luonengjun, pbonzini,
	alex.bennee, rth

Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
---
 util/path.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/util/path.c b/util/path.c
index 5c59d9f..df1653f 100644
--- a/util/path.c
+++ b/util/path.c
@@ -46,9 +46,12 @@ static struct pathelem *new_entry(const char *root,
                                   const char *name)
 {
     struct pathelem *new = malloc(sizeof(*new));
-    new->name = strdup(name);
-    new->pathname = g_strdup_printf("%s/%s", root, name);
-    new->num_entries = 0;
+
+    if (new) {
+        new->name = strdup(name);
+        new->pathname = g_strdup_printf("%s/%s", root, name);
+        new->num_entries = 0;
+    }
     return new;
 }
 
-- 
1.7.12.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [Qemu-devel] [PATCH v3 06/10] slirp: check return value of malloc()
  2014-08-07  8:01 [Qemu-devel] [PATCH v3 00/10] fix three bugs about use-after-free and several api abuse zhanghailiang
                   ` (4 preceding siblings ...)
  2014-08-07  8:01 ` [Qemu-devel] [PATCH v3 05/10] util/path: check return value of malloc() zhanghailiang
@ 2014-08-07  8:01 ` zhanghailiang
  2014-08-07 11:08   ` Gonglei (Arei)
  2014-08-07  8:01 ` [Qemu-devel] [PATCH v3 07/10] linux-user: " zhanghailiang
                   ` (3 subsequent siblings)
  9 siblings, 1 reply; 20+ messages in thread
From: zhanghailiang @ 2014-08-07  8:01 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, lkurusa, zhanghailiang, mst, jan.kiszka, riku.voipio, mjt,
	peter.huangpeng, lcapitulino, stefanha, luonengjun, pbonzini,
	alex.bennee, rth

Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
---
 slirp/misc.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/slirp/misc.c b/slirp/misc.c
index b8eb74c..0109c9f 100644
--- a/slirp/misc.c
+++ b/slirp/misc.c
@@ -55,6 +55,9 @@ int add_exec(struct ex_list **ex_ptr, int do_pty, char *exec,
 
 	tmp_ptr = *ex_ptr;
 	*ex_ptr = (struct ex_list *)malloc(sizeof(struct ex_list));
+    if (!ex_ptr) {
+        return -1;
+    }
 	(*ex_ptr)->ex_fport = port;
 	(*ex_ptr)->ex_addr = addr;
 	(*ex_ptr)->ex_pty = do_pty;
@@ -236,8 +239,9 @@ strdup(str)
 	char *bptr;
 
 	bptr = (char *)malloc(strlen(str)+1);
-	strcpy(bptr, str);
-
+    if (bptr) {
+        strcpy(bptr, str);
+    }
 	return bptr;
 }
 #endif
-- 
1.7.12.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [Qemu-devel] [PATCH v3 07/10] linux-user: check return value of malloc()
  2014-08-07  8:01 [Qemu-devel] [PATCH v3 00/10] fix three bugs about use-after-free and several api abuse zhanghailiang
                   ` (5 preceding siblings ...)
  2014-08-07  8:01 ` [Qemu-devel] [PATCH v3 06/10] slirp: " zhanghailiang
@ 2014-08-07  8:01 ` zhanghailiang
  2014-08-07 17:19   ` Richard Henderson
  2014-08-07  8:01 ` [Qemu-devel] [PATCH v3 08/10] qtest: check the value returned by fopen() zhanghailiang
                   ` (2 subsequent siblings)
  9 siblings, 1 reply; 20+ messages in thread
From: zhanghailiang @ 2014-08-07  8:01 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, lkurusa, zhanghailiang, mst, jan.kiszka, riku.voipio, mjt,
	peter.huangpeng, lcapitulino, stefanha, luonengjun, pbonzini,
	alex.bennee, rth

Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
---
 linux-user/syscall.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index a50229d..93d9076 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -2870,6 +2870,9 @@ static inline abi_long do_msgsnd(int msqid, abi_long msgp,
     if (!lock_user_struct(VERIFY_READ, target_mb, msgp, 0))
         return -TARGET_EFAULT;
     host_mb = malloc(msgsz+sizeof(long));
+    if (!host_mb) {
+        return -TARGET_ENOMEM;
+    }
     host_mb->mtype = (abi_long) tswapal(target_mb->mtype);
     memcpy(host_mb->mtext, target_mb->mtext, msgsz);
     ret = get_errno(msgsnd(msqid, host_mb, msgsz, msgflg));
-- 
1.7.12.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [Qemu-devel] [PATCH v3 08/10] qtest: check the value returned by fopen()
  2014-08-07  8:01 [Qemu-devel] [PATCH v3 00/10] fix three bugs about use-after-free and several api abuse zhanghailiang
                   ` (6 preceding siblings ...)
  2014-08-07  8:01 ` [Qemu-devel] [PATCH v3 07/10] linux-user: " zhanghailiang
@ 2014-08-07  8:01 ` zhanghailiang
  2014-08-07 11:14   ` Gonglei (Arei)
  2014-08-07  8:01 ` [Qemu-devel] [PATCH v3 09/10] tcg: check return value of fopen() zhanghailiang
  2014-08-07  8:01 ` [Qemu-devel] [PATCH v3 10/10] block/vvfat: fix setbuf stream parameter may be NULL zhanghailiang
  9 siblings, 1 reply; 20+ messages in thread
From: zhanghailiang @ 2014-08-07  8:01 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, lkurusa, zhanghailiang, mst, jan.kiszka, riku.voipio, mjt,
	peter.huangpeng, lcapitulino, stefanha, Li Liu, luonengjun,
	pbonzini, alex.bennee, rth

The function fopen() may fail, so check its return value.

Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
Signed-off-by: Li Liu <john.liuli@huawei.com>
---
 qtest.c                  | 5 +++++
 tests/bios-tables-test.c | 2 ++
 2 files changed, 7 insertions(+)

diff --git a/qtest.c b/qtest.c
index 04a6dc1..ae9b636 100644
--- a/qtest.c
+++ b/qtest.c
@@ -536,6 +536,11 @@ void qtest_init(const char *qtest_chrdev, const char *qtest_log, Error **errp)
     if (qtest_log) {
         if (strcmp(qtest_log, "none") != 0) {
             qtest_log_fp = fopen(qtest_log, "w+");
+            if (qtest_log_fp == NULL) {
+                error_setg(errp, "Failed to open log file for qtest: \"%s\"",
+                           qtest_log);
+                return;
+            }
         }
     } else {
         qtest_log_fp = stderr;
diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c
index 045eb27..6a357c0 100644
--- a/tests/bios-tables-test.c
+++ b/tests/bios-tables-test.c
@@ -790,6 +790,8 @@ int main(int argc, char *argv[])
     const char *arch = qtest_get_arch();
     FILE *f = fopen(disk, "w");
     int ret;
+
+    g_assert(f != NULL);
     fwrite(boot_sector, 1, sizeof boot_sector, f);
     fclose(f);
 
-- 
1.7.12.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [Qemu-devel] [PATCH v3 09/10] tcg: check return value of fopen()
  2014-08-07  8:01 [Qemu-devel] [PATCH v3 00/10] fix three bugs about use-after-free and several api abuse zhanghailiang
                   ` (7 preceding siblings ...)
  2014-08-07  8:01 ` [Qemu-devel] [PATCH v3 08/10] qtest: check the value returned by fopen() zhanghailiang
@ 2014-08-07  8:01 ` zhanghailiang
  2014-08-07  8:01 ` [Qemu-devel] [PATCH v3 10/10] block/vvfat: fix setbuf stream parameter may be NULL zhanghailiang
  9 siblings, 0 replies; 20+ messages in thread
From: zhanghailiang @ 2014-08-07  8:01 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, lkurusa, zhanghailiang, mst, jan.kiszka, riku.voipio, mjt,
	peter.huangpeng, lcapitulino, stefanha, Li Liu, luonengjun,
	pbonzini, alex.bennee, rth

From: Li Liu <john.liuli@huawei.com>

Give a warning message if fopen() failed to open the log file.

Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
Signed-off-by: Li Liu <john.liuli@huawei.com>
---
 tcg/tcg.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tcg/tcg.c b/tcg/tcg.c
index c068990..8f50d2a 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -2406,6 +2406,10 @@ static void dump_op_count(void)
     int i;
     FILE *f;
     f = fopen("/tmp/op.log", "w");
+    if (f == NULL) {
+        fprintf(stderr, "Failed to open /tmp/op.log\n");
+        return;
+    }
     for(i = INDEX_op_end; i < NB_OPS; i++) {
         fprintf(f, "%s %" PRId64 "\n", tcg_op_defs[i].name, tcg_table_op_count[i]);
     }
-- 
1.7.12.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [Qemu-devel] [PATCH v3 10/10] block/vvfat: fix setbuf stream parameter may be NULL
  2014-08-07  8:01 [Qemu-devel] [PATCH v3 00/10] fix three bugs about use-after-free and several api abuse zhanghailiang
                   ` (8 preceding siblings ...)
  2014-08-07  8:01 ` [Qemu-devel] [PATCH v3 09/10] tcg: check return value of fopen() zhanghailiang
@ 2014-08-07  8:01 ` zhanghailiang
  9 siblings, 0 replies; 20+ messages in thread
From: zhanghailiang @ 2014-08-07  8:01 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, lkurusa, zhanghailiang, mst, jan.kiszka, riku.voipio, mjt,
	peter.huangpeng, lcapitulino, stefanha, Li Liu, luonengjun,
	pbonzini, alex.bennee, rth

From: Li Liu <john.liuli@huawei.com>

fopen() may return NULL which will cause setbuf() segmentfault

Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
Signed-off-by: Li Liu <john.liuli@huawei.com>
---
 block/vvfat.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/block/vvfat.c b/block/vvfat.c
index 70176b1..6889ea9 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -1084,7 +1084,10 @@ static int vvfat_open(BlockDriverState *bs, QDict *options, int flags,
 
 DLOG(if (stderr == NULL) {
     stderr = fopen("vvfat.log", "a");
-    setbuf(stderr, NULL);
+
+    if (stderr) {
+        setbuf(stderr, NULL);
+    }
 })
 
     opts = qemu_opts_create(&runtime_opts, NULL, 0, &error_abort);
-- 
1.7.12.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PATCH v3 04/10] ivshmem: check the value returned by fstat()
  2014-08-07  8:01 ` [Qemu-devel] [PATCH v3 04/10] ivshmem: check the value returned by fstat() zhanghailiang
@ 2014-08-07  8:53   ` Levente Kurusa
  0 siblings, 0 replies; 20+ messages in thread
From: Levente Kurusa @ 2014-08-07  8:53 UTC (permalink / raw)
  To: zhanghailiang
  Cc: kwolf, mst, jan kiszka, riku voipio, mjt, qemu-devel,
	peter huangpeng, stefanha, luonengjun, pbonzini, lcapitulino,
	alex bennee, rth

> The function fstat() may fail, so check its return value.
> 
> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>

Acked-by: Levente Kurusa <lkurusa@redhat.com>

Thanks!
Levente Kurusa

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PATCH v3 02/10] monitor: fix access freed memory
  2014-08-07  8:01 ` [Qemu-devel] [PATCH v3 02/10] monitor: " zhanghailiang
@ 2014-08-07 11:01   ` Gonglei (Arei)
  0 siblings, 0 replies; 20+ messages in thread
From: Gonglei (Arei) @ 2014-08-07 11:01 UTC (permalink / raw)
  To: Zhanghailiang, qemu-devel@nongnu.org
  Cc: kwolf@redhat.com, lkurusa@redhat.com, mst@redhat.com,
	jan.kiszka@siemens.com, riku.voipio@iki.fi, mjt@tls.msk.ru,
	Huangpeng (Peter), lcapitulino@redhat.com, stefanha@redhat.com,
	Luonengjun, pbonzini@redhat.com, alex.bennee@linaro.org,
	rth@twiddle.net

> Subject: [Qemu-devel] [PATCH v3 02/10] monitor: fix access freed memory
> 
> The function monitor_fdset_dup_fd_find_remove() references member of
> 'mon_fdset'
> which may be freed in function monitor_fdset_cleanup()
> 
> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
> ---

Reviewed-by: Gonglei <arei.gonglei@huawei.com>

Best regards,
-Gonglei

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PATCH v3 05/10] util/path: check return value of malloc()
  2014-08-07  8:01 ` [Qemu-devel] [PATCH v3 05/10] util/path: check return value of malloc() zhanghailiang
@ 2014-08-07 11:04   ` Gonglei (Arei)
  0 siblings, 0 replies; 20+ messages in thread
From: Gonglei (Arei) @ 2014-08-07 11:04 UTC (permalink / raw)
  To: Zhanghailiang, qemu-devel@nongnu.org
  Cc: kwolf@redhat.com, lkurusa@redhat.com, mst@redhat.com,
	jan.kiszka@siemens.com, riku.voipio@iki.fi, mjt@tls.msk.ru,
	Huangpeng (Peter), lcapitulino@redhat.com, stefanha@redhat.com,
	Luonengjun, pbonzini@redhat.com, alex.bennee@linaro.org,
	rth@twiddle.net

> Subject: [Qemu-devel] [PATCH v3 05/10] util/path: check return value of malloc()
> 
> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
> ---

Reviewed-by: Gonglei <arei.gonglei@huawei.com>

Best regards,
-Gonglei

>  util/path.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/util/path.c b/util/path.c
> index 5c59d9f..df1653f 100644
> --- a/util/path.c
> +++ b/util/path.c
> @@ -46,9 +46,12 @@ static struct pathelem *new_entry(const char *root,
>                                    const char *name)
>  {
>      struct pathelem *new = malloc(sizeof(*new));
> -    new->name = strdup(name);
> -    new->pathname = g_strdup_printf("%s/%s", root, name);
> -    new->num_entries = 0;
> +
> +    if (new) {
> +        new->name = strdup(name);
> +        new->pathname = g_strdup_printf("%s/%s", root, name);
> +        new->num_entries = 0;
> +    }
>      return new;
>  }
> 
> --
> 1.7.12.4
> 
> 

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PATCH v3 06/10] slirp: check return value of malloc()
  2014-08-07  8:01 ` [Qemu-devel] [PATCH v3 06/10] slirp: " zhanghailiang
@ 2014-08-07 11:08   ` Gonglei (Arei)
  2014-08-07 11:25     ` zhanghailiang
  0 siblings, 1 reply; 20+ messages in thread
From: Gonglei (Arei) @ 2014-08-07 11:08 UTC (permalink / raw)
  To: Zhanghailiang, qemu-devel@nongnu.org
  Cc: kwolf@redhat.com, lkurusa@redhat.com, mst@redhat.com,
	jan.kiszka@siemens.com, riku.voipio@iki.fi, mjt@tls.msk.ru,
	Huangpeng (Peter), lcapitulino@redhat.com, stefanha@redhat.com,
	Luonengjun, pbonzini@redhat.com, alex.bennee@linaro.org,
	rth@twiddle.net

> Subject: [Qemu-devel] [PATCH v3 06/10] slirp: check return value of malloc()
> 
> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
> ---
>  slirp/misc.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/slirp/misc.c b/slirp/misc.c
> index b8eb74c..0109c9f 100644
> --- a/slirp/misc.c
> +++ b/slirp/misc.c
> @@ -55,6 +55,9 @@ int add_exec(struct ex_list **ex_ptr, int do_pty, char
> *exec,
> 
>  	tmp_ptr = *ex_ptr;
>  	*ex_ptr = (struct ex_list *)malloc(sizeof(struct ex_list));
> +    if (!ex_ptr) {
> +        return -1;
> +    }

Not (!ex_ptr) but (*ex_ptr == NULL). 

BTW, you'd better add more information when malloc memory filed.

Best regards,
-Gonglei

>  	(*ex_ptr)->ex_fport = port;
>  	(*ex_ptr)->ex_addr = addr;
>  	(*ex_ptr)->ex_pty = do_pty;
> @@ -236,8 +239,9 @@ strdup(str)
>  	char *bptr;
> 
>  	bptr = (char *)malloc(strlen(str)+1);
> -	strcpy(bptr, str);
> -
> +    if (bptr) {
> +        strcpy(bptr, str);
> +    }
>  	return bptr;
>  }
>  #endif
> --
> 1.7.12.4
> 
> 

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PATCH v3 08/10] qtest: check the value returned by fopen()
  2014-08-07  8:01 ` [Qemu-devel] [PATCH v3 08/10] qtest: check the value returned by fopen() zhanghailiang
@ 2014-08-07 11:14   ` Gonglei (Arei)
  2014-08-07 11:33     ` zhanghailiang
  0 siblings, 1 reply; 20+ messages in thread
From: Gonglei (Arei) @ 2014-08-07 11:14 UTC (permalink / raw)
  To: Zhanghailiang, qemu-devel@nongnu.org
  Cc: kwolf@redhat.com, lkurusa@redhat.com, mst@redhat.com,
	jan.kiszka@siemens.com, riku.voipio@iki.fi, mjt@tls.msk.ru,
	Huangpeng (Peter), lcapitulino@redhat.com, stefanha@redhat.com,
	Liuli (I), Luonengjun, pbonzini@redhat.com,
	alex.bennee@linaro.org, rth@twiddle.net

> Subject: [Qemu-devel] [PATCH v3 08/10] qtest: check the value returned by
> fopen()
> 
> The function fopen() may fail, so check its return value.
> 
> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
> Signed-off-by: Li Liu <john.liuli@huawei.com>
> ---
>  qtest.c                  | 5 +++++
>  tests/bios-tables-test.c | 2 ++
>  2 files changed, 7 insertions(+)
> 
> diff --git a/qtest.c b/qtest.c
> index 04a6dc1..ae9b636 100644
> --- a/qtest.c
> +++ b/qtest.c
> @@ -536,6 +536,11 @@ void qtest_init(const char *qtest_chrdev, const char
> *qtest_log, Error **errp)
>      if (qtest_log) {
>          if (strcmp(qtest_log, "none") != 0) {
>              qtest_log_fp = fopen(qtest_log, "w+");
> +            if (qtest_log_fp == NULL) {
> +                error_setg(errp, "Failed to open log file for qtest: \"%s\"",
> +                           qtest_log);
> +                return;
> +            }

Actually I don't think this check is necessary, because the qtest_log_fp will be checked
where it is used.

Best regards,
-Gonglei

>          }
>      } else {
>          qtest_log_fp = stderr;
> diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c
> index 045eb27..6a357c0 100644
> --- a/tests/bios-tables-test.c
> +++ b/tests/bios-tables-test.c
> @@ -790,6 +790,8 @@ int main(int argc, char *argv[])
>      const char *arch = qtest_get_arch();
>      FILE *f = fopen(disk, "w");
>      int ret;
> +
> +    g_assert(f != NULL);
>      fwrite(boot_sector, 1, sizeof boot_sector, f);
>      fclose(f);
> 
> --
> 1.7.12.4
> 
> 

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PATCH v3 06/10] slirp: check return value of malloc()
  2014-08-07 11:08   ` Gonglei (Arei)
@ 2014-08-07 11:25     ` zhanghailiang
  0 siblings, 0 replies; 20+ messages in thread
From: zhanghailiang @ 2014-08-07 11:25 UTC (permalink / raw)
  To: Gonglei (Arei)
  Cc: kwolf@redhat.com, lkurusa@redhat.com, mst@redhat.com,
	jan.kiszka@siemens.com, riku.voipio@iki.fi, mjt@tls.msk.ru,
	qemu-devel@nongnu.org, Huangpeng (Peter), stefanha@redhat.com,
	Luonengjun, pbonzini@redhat.com, lcapitulino@redhat.com,
	alex.bennee@linaro.org, rth@twiddle.net

On 2014/8/7 19:08, Gonglei (Arei) wrote:
>> Subject: [Qemu-devel] [PATCH v3 06/10] slirp: check return value of malloc()
>>
>> Signed-off-by: zhanghailiang<zhang.zhanghailiang@huawei.com>
>> ---
>>   slirp/misc.c | 8 ++++++--
>>   1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/slirp/misc.c b/slirp/misc.c
>> index b8eb74c..0109c9f 100644
>> --- a/slirp/misc.c
>> +++ b/slirp/misc.c
>> @@ -55,6 +55,9 @@ int add_exec(struct ex_list **ex_ptr, int do_pty, char
>> *exec,
>>
>>   	tmp_ptr = *ex_ptr;
>>   	*ex_ptr = (struct ex_list *)malloc(sizeof(struct ex_list));
>> +    if (!ex_ptr) {
>> +        return -1;
>> +    }
>
> Not (!ex_ptr) but (*ex_ptr == NULL).

Sorry, this is my mistake!

> BTW, you'd better add more information when malloc memory filed.
>

Good idea, i will add it. Thanks!

>
>>   	(*ex_ptr)->ex_fport = port;
>>   	(*ex_ptr)->ex_addr = addr;
>>   	(*ex_ptr)->ex_pty = do_pty;
>> @@ -236,8 +239,9 @@ strdup(str)
>>   	char *bptr;
>>
>>   	bptr = (char *)malloc(strlen(str)+1);
>> -	strcpy(bptr, str);
>> -
>> +    if (bptr) {
>> +        strcpy(bptr, str);
>> +    }
>>   	return bptr;
>>   }
>>   #endif

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PATCH v3 08/10] qtest: check the value returned by fopen()
  2014-08-07 11:14   ` Gonglei (Arei)
@ 2014-08-07 11:33     ` zhanghailiang
  0 siblings, 0 replies; 20+ messages in thread
From: zhanghailiang @ 2014-08-07 11:33 UTC (permalink / raw)
  To: Gonglei (Arei)
  Cc: kwolf@redhat.com, lkurusa@redhat.com, mst@redhat.com,
	jan.kiszka@siemens.com, riku.voipio@iki.fi, mjt@tls.msk.ru,
	qemu-devel@nongnu.org, Huangpeng (Peter), stefanha@redhat.com,
	Liuli (I), Luonengjun, pbonzini@redhat.com,
	lcapitulino@redhat.com, alex.bennee@linaro.org, rth@twiddle.net

On 2014/8/7 19:14, Gonglei (Arei) wrote:
>> Subject: [Qemu-devel] [PATCH v3 08/10] qtest: check the value returned by
>> fopen()
>>
>> The function fopen() may fail, so check its return value.
>>
>> Signed-off-by: zhanghailiang<zhang.zhanghailiang@huawei.com>
>> Signed-off-by: Li Liu<john.liuli@huawei.com>
>> ---
>>   qtest.c                  | 5 +++++
>>   tests/bios-tables-test.c | 2 ++
>>   2 files changed, 7 insertions(+)
>>
>> diff --git a/qtest.c b/qtest.c
>> index 04a6dc1..ae9b636 100644
>> --- a/qtest.c
>> +++ b/qtest.c
>> @@ -536,6 +536,11 @@ void qtest_init(const char *qtest_chrdev, const char
>> *qtest_log, Error **errp)
>>       if (qtest_log) {
>>           if (strcmp(qtest_log, "none") != 0) {
>>               qtest_log_fp = fopen(qtest_log, "w+");
>> +            if (qtest_log_fp == NULL) {
>> +                error_setg(errp, "Failed to open log file for qtest: \"%s\"",
>> +                           qtest_log);
>> +                return;
>> +            }
>
> Actually I don't think this check is necessary, because the qtest_log_fp will be checked
> where it is used.
>

Hmm, you are right! I will remove the changes!

Thanks,
zhanghailiang

>>           }
>>       } else {
>>           qtest_log_fp = stderr;
>> diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c
>> index 045eb27..6a357c0 100644
>> --- a/tests/bios-tables-test.c
>> +++ b/tests/bios-tables-test.c
>> @@ -790,6 +790,8 @@ int main(int argc, char *argv[])
>>       const char *arch = qtest_get_arch();
>>       FILE *f = fopen(disk, "w");
>>       int ret;
>> +
>> +    g_assert(f != NULL);
>>       fwrite(boot_sector, 1, sizeof boot_sector, f);
>>       fclose(f);
>>
>> --
>> 1.7.12.4
>>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PATCH v3 07/10] linux-user: check return value of malloc()
  2014-08-07  8:01 ` [Qemu-devel] [PATCH v3 07/10] linux-user: " zhanghailiang
@ 2014-08-07 17:19   ` Richard Henderson
  2014-08-08  1:21     ` zhanghailiang
  0 siblings, 1 reply; 20+ messages in thread
From: Richard Henderson @ 2014-08-07 17:19 UTC (permalink / raw)
  To: zhanghailiang, qemu-devel
  Cc: kwolf, lkurusa, mst, jan.kiszka, riku.voipio, mjt,
	peter.huangpeng, lcapitulino, stefanha, luonengjun, pbonzini,
	alex.bennee

On 08/06/2014 10:01 PM, zhanghailiang wrote:
>      if (!lock_user_struct(VERIFY_READ, target_mb, msgp, 0))
>          return -TARGET_EFAULT;
>      host_mb = malloc(msgsz+sizeof(long));
> +    if (!host_mb) {
> +        return -TARGET_ENOMEM;
> +    }

lock_user allocates memory; returning from the middle leaks it.


r~

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PATCH v3 07/10] linux-user: check return value of malloc()
  2014-08-07 17:19   ` Richard Henderson
@ 2014-08-08  1:21     ` zhanghailiang
  0 siblings, 0 replies; 20+ messages in thread
From: zhanghailiang @ 2014-08-08  1:21 UTC (permalink / raw)
  To: Richard Henderson
  Cc: kwolf, lkurusa, mst, jan.kiszka, riku.voipio, mjt, qemu-devel,
	lcapitulino, stefanha, luonengjun, pbonzini, peter.huangpeng,
	alex.bennee

On 2014/8/8 1:19, Richard Henderson wrote:
> On 08/06/2014 10:01 PM, zhanghailiang wrote:
>>       if (!lock_user_struct(VERIFY_READ, target_mb, msgp, 0))
>>           return -TARGET_EFAULT;
>>       host_mb = malloc(msgsz+sizeof(long));
>> +    if (!host_mb) {
>> +        return -TARGET_ENOMEM;
>> +    }
>
> lock_user allocates memory; returning from the middle leaks it.
>

Hmm, it is my fault, i will correct it. Thanks, Richard.

Best regards,
zhanghailiang

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2014-08-08  1:22 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-08-07  8:01 [Qemu-devel] [PATCH v3 00/10] fix three bugs about use-after-free and several api abuse zhanghailiang
2014-08-07  8:01 ` [Qemu-devel] [PATCH v3 01/10] l2cap: fix access freed memory zhanghailiang
2014-08-07  8:01 ` [Qemu-devel] [PATCH v3 02/10] monitor: " zhanghailiang
2014-08-07 11:01   ` Gonglei (Arei)
2014-08-07  8:01 ` [Qemu-devel] [PATCH v3 03/10] virtio-blk: fix reference a pointer which might be freed zhanghailiang
2014-08-07  8:01 ` [Qemu-devel] [PATCH v3 04/10] ivshmem: check the value returned by fstat() zhanghailiang
2014-08-07  8:53   ` Levente Kurusa
2014-08-07  8:01 ` [Qemu-devel] [PATCH v3 05/10] util/path: check return value of malloc() zhanghailiang
2014-08-07 11:04   ` Gonglei (Arei)
2014-08-07  8:01 ` [Qemu-devel] [PATCH v3 06/10] slirp: " zhanghailiang
2014-08-07 11:08   ` Gonglei (Arei)
2014-08-07 11:25     ` zhanghailiang
2014-08-07  8:01 ` [Qemu-devel] [PATCH v3 07/10] linux-user: " zhanghailiang
2014-08-07 17:19   ` Richard Henderson
2014-08-08  1:21     ` zhanghailiang
2014-08-07  8:01 ` [Qemu-devel] [PATCH v3 08/10] qtest: check the value returned by fopen() zhanghailiang
2014-08-07 11:14   ` Gonglei (Arei)
2014-08-07 11:33     ` zhanghailiang
2014-08-07  8:01 ` [Qemu-devel] [PATCH v3 09/10] tcg: check return value of fopen() zhanghailiang
2014-08-07  8:01 ` [Qemu-devel] [PATCH v3 10/10] block/vvfat: fix setbuf stream parameter may be NULL zhanghailiang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).