* [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).