From 5bbebf622897a59db5da4c468e737bfec4d71280 Mon Sep 17 00:00:00 2001 From: zhanghailiang Date: Fri, 14 Nov 2014 10:18:08 +0800 Subject: libcacard: fix resource leak In function connect_to_qemu(), getaddrinfo() will allocate memory that is stored into server, it should be freed by using freeaddrinfo() before connect_to_qemu() return. Cc: qemu-stable@nongnu.org Reviewed-by: Markus Armbruster Signed-off-by: zhanghailiang Signed-off-by: Paolo Bonzini --- libcacard/vscclient.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/libcacard/vscclient.c b/libcacard/vscclient.c index 80111df..fa6041d 100644 --- a/libcacard/vscclient.c +++ b/libcacard/vscclient.c @@ -597,7 +597,7 @@ connect_to_qemu( const char *port ) { struct addrinfo hints; - struct addrinfo *server; + struct addrinfo *server = NULL; int ret, sock; sock = socket(AF_INET, SOCK_STREAM, 0); @@ -629,9 +629,14 @@ connect_to_qemu( if (verbose) { printf("Connected (sizeof Header=%zd)!\n", sizeof(VSCMsgHeader)); } + + freeaddrinfo(server); return sock; cleanup_socket: + if (server) { + freeaddrinfo(server); + } closesocket(sock); return -1; } -- cgit v1.1 From 77374582ab961af2c5e702f767f52179d5f7676c Mon Sep 17 00:00:00 2001 From: zhanghailiang Date: Fri, 14 Nov 2014 09:39:23 +0800 Subject: l2tpv3: fix possible double free freeaddrinfo(result) does not assign result = NULL, after frees it. There will be a double free when it goes error case. It is reported by covertiy. Reviewed-by: Gonglei Cc: qemu-stable@nongnu.org Signed-off-by: zhanghailiang Signed-off-by: Paolo Bonzini --- net/l2tpv3.c | 1 - 1 file changed, 1 deletion(-) diff --git a/net/l2tpv3.c b/net/l2tpv3.c index 528d95b..65db5ef 100644 --- a/net/l2tpv3.c +++ b/net/l2tpv3.c @@ -660,7 +660,6 @@ int net_init_l2tpv3(const NetClientOptions *opts, if (fd == -1) { fd = -errno; error_report("l2tpv3_open : socket creation failed, errno = %d", -fd); - freeaddrinfo(result); goto outerr; } if (bind(fd, (struct sockaddr *) result->ai_addr, result->ai_addrlen)) { -- cgit v1.1 From d4754a953196516b16beef707dcdfdb35c2eec6e Mon Sep 17 00:00:00 2001 From: Gonglei Date: Sat, 15 Nov 2014 18:06:40 +0800 Subject: l2tpv3: fix fd leak In this false branch, fd will leak when it is zero. Change the testing condition. Signed-off-by: Gonglei [Fix net_l2tpv3_cleanup as well. - Paolo] Signed-off-by: Paolo Bonzini --- net/l2tpv3.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/net/l2tpv3.c b/net/l2tpv3.c index 65db5ef..3b805a7 100644 --- a/net/l2tpv3.c +++ b/net/l2tpv3.c @@ -516,7 +516,7 @@ static void net_l2tpv3_cleanup(NetClientState *nc) qemu_purge_queued_packets(nc); l2tpv3_read_poll(s, false); l2tpv3_write_poll(s, false); - if (s->fd > 0) { + if (s->fd >= 0) { close(s->fd); } destroy_vector(s->msgvec, MAX_L2TPV3_MSGCNT, IOVSIZE); @@ -745,7 +745,7 @@ int net_init_l2tpv3(const NetClientOptions *opts, return 0; outerr: qemu_del_net_client(nc); - if (fd > 0) { + if (fd >= 0) { close(fd); } if (result) { -- cgit v1.1 From 77e205a52856adffdd5db70449a8604aa9f66e74 Mon Sep 17 00:00:00 2001 From: Gonglei Date: Sat, 15 Nov 2014 18:06:41 +0800 Subject: mips_mipssim: fix use-after-free for filename May pass freed pointer filename as an argument to error_report. Signed-off-by: Gonglei Signed-off-by: Paolo Bonzini --- hw/mips/mips_mipssim.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/mips/mips_mipssim.c b/hw/mips/mips_mipssim.c index 7ea0b9a..5d44c3f 100644 --- a/hw/mips/mips_mipssim.c +++ b/hw/mips/mips_mipssim.c @@ -197,7 +197,7 @@ mips_mipssim_init(MachineState *machine) !kernel_filename && !qtest_enabled()) { /* Bail out if we have neither a kernel image nor boot vector code. */ error_report("Could not load MIPS bios '%s', and no " - "-kernel argument was specified", filename); + "-kernel argument was specified", bios_name); exit(1); } else { /* We have a boot vector start address. */ -- cgit v1.1 From 1def74548d8013949c7d4704420d4fdd5fb85268 Mon Sep 17 00:00:00 2001 From: Gonglei Date: Sat, 15 Nov 2014 18:06:42 +0800 Subject: qga: fix false negative argument passing Function send_response(s, &qdict->base) returns a negative number when any failures occured. But strerror()'s parameter cannot be negative. Let's change the testing condition and pass '-ret' to strerr(). Signed-off-by: Gonglei Signed-off-by: Paolo Bonzini --- qga/main.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/qga/main.c b/qga/main.c index 227f2bd..9939a2b 100644 --- a/qga/main.c +++ b/qga/main.c @@ -603,8 +603,8 @@ static void process_event(JSONMessageParser *parser, QList *tokens) error_free(err); } ret = send_response(s, QOBJECT(qdict)); - if (ret) { - g_warning("error sending error response: %s", strerror(ret)); + if (ret < 0) { + g_warning("error sending error response: %s", strerror(-ret)); } } -- cgit v1.1 From ddd2eab72fbd383a56f439bf278c6d647abd4f54 Mon Sep 17 00:00:00 2001 From: Gonglei Date: Sat, 15 Nov 2014 18:06:43 +0800 Subject: loader: fix NEGATIVE_RETURNS lseek will return -1 on error, g_malloc0(size) and read(,,size) paramenters cannot be negative. We should add a check for return value of lseek(). Signed-off-by: Gonglei Signed-off-by: Paolo Bonzini --- hw/core/loader.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/hw/core/loader.c b/hw/core/loader.c index bbe6eb3..fc15535 100644 --- a/hw/core/loader.c +++ b/hw/core/loader.c @@ -80,6 +80,13 @@ int load_image(const char *filename, uint8_t *addr) if (fd < 0) return -1; size = lseek(fd, 0, SEEK_END); + if (size == -1) { + fprintf(stderr, "file %-20s: get size error: %s\n", + filename, strerror(errno)); + close(fd); + return -1; + } + lseek(fd, 0, SEEK_SET); if (read(fd, addr, size) != size) { close(fd); @@ -748,6 +755,12 @@ int rom_add_file(const char *file, const char *fw_dir, } rom->addr = addr; rom->romsize = lseek(fd, 0, SEEK_END); + if (rom->romsize == -1) { + fprintf(stderr, "rom: file %-20s: get size error: %s\n", + rom->name, strerror(errno)); + goto err; + } + rom->datasize = rom->romsize; rom->data = g_malloc0(rom->datasize); lseek(fd, 0, SEEK_SET); -- cgit v1.1 From 720fdd6fa92df9041316e94816ab7e56abaed4e9 Mon Sep 17 00:00:00 2001 From: Gonglei Date: Sat, 15 Nov 2014 18:06:44 +0800 Subject: nvme: remove superfluous check Operands don't affect result (CONSTANT_EXPRESSION_RESULT) ((n->bar.aqa >> AQA_ASQS_SHIFT) & AQA_ASQS_MASK) > 4095 is always false regardless of the values of its operands. This occurs as the logical second operand of '||'. Signed-off-by: Gonglei Signed-off-by: Paolo Bonzini --- hw/block/nvme.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/hw/block/nvme.c b/hw/block/nvme.c index b6263dc..1327658 100644 --- a/hw/block/nvme.c +++ b/hw/block/nvme.c @@ -583,8 +583,7 @@ static int nvme_start_ctrl(NvmeCtrl *n) NVME_CC_IOCQES(n->bar.cc) > NVME_CTRL_CQES_MAX(n->id_ctrl.cqes) || NVME_CC_IOSQES(n->bar.cc) < NVME_CTRL_SQES_MIN(n->id_ctrl.sqes) || NVME_CC_IOSQES(n->bar.cc) > NVME_CTRL_SQES_MAX(n->id_ctrl.sqes) || - !NVME_AQA_ASQS(n->bar.aqa) || NVME_AQA_ASQS(n->bar.aqa) > 4095 || - !NVME_AQA_ACQS(n->bar.aqa) || NVME_AQA_ACQS(n->bar.aqa) > 4095) { + !NVME_AQA_ASQS(n->bar.aqa) || !NVME_AQA_ACQS(n->bar.aqa)) { return -1; } -- cgit v1.1 From 6cfcd864a468eb7bd3da20a5462b5af1791581d3 Mon Sep 17 00:00:00 2001 From: Gonglei Date: Sat, 15 Nov 2014 18:06:45 +0800 Subject: acl: fix memory leak If 'i != index' for all acl->entries, variable entry leaks the storage it points to. Signed-off-by: Gonglei Signed-off-by: Paolo Bonzini --- util/acl.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/util/acl.c b/util/acl.c index 938b7ae..571d686 100644 --- a/util/acl.c +++ b/util/acl.c @@ -132,7 +132,6 @@ int qemu_acl_insert(qemu_acl *acl, const char *match, int index) { - qemu_acl_entry *entry; qemu_acl_entry *tmp; int i = 0; @@ -142,13 +141,14 @@ int qemu_acl_insert(qemu_acl *acl, return qemu_acl_append(acl, deny, match); } - entry = g_malloc(sizeof(*entry)); - entry->match = g_strdup(match); - entry->deny = deny; - QTAILQ_FOREACH(tmp, &acl->entries, next) { i++; if (i == index) { + qemu_acl_entry *entry; + entry = g_malloc(sizeof(*entry)); + entry->match = g_strdup(match); + entry->deny = deny; + QTAILQ_INSERT_BEFORE(tmp, entry, next); acl->nentries++; break; -- cgit v1.1 From 86d10328a019b19838de3f7964a9da16af3a5073 Mon Sep 17 00:00:00 2001 From: Gonglei Date: Sat, 15 Nov 2014 18:06:46 +0800 Subject: qemu-char: fix MISSING_COMMA Signed-off-by: Gonglei Signed-off-by: Paolo Bonzini --- qemu-char.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qemu-char.c b/qemu-char.c index bd0709b..4a76f0f 100644 --- a/qemu-char.c +++ b/qemu-char.c @@ -464,7 +464,7 @@ static const char * const mux_help[] = { "% h print this help\n\r", "% x exit emulator\n\r", "% s save disk data back to file (if -snapshot)\n\r", - "% t toggle console timestamps\n\r" + "% t toggle console timestamps\n\r", "% b send break (magic sysrq)\n\r", "% c switch between console and monitor\n\r", "% % sends %\n\r", -- cgit v1.1 From 0e8b439ae57ee3c46fb95e1775ea038d34496346 Mon Sep 17 00:00:00 2001 From: Gonglei Date: Sat, 15 Nov 2014 18:06:47 +0800 Subject: shpc: fix error propaagation Signed-off-by: Gonglei Signed-off-by: Paolo Bonzini --- hw/pci/shpc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/hw/pci/shpc.c b/hw/pci/shpc.c index 65b2f51..9a39060 100644 --- a/hw/pci/shpc.c +++ b/hw/pci/shpc.c @@ -559,8 +559,9 @@ void shpc_device_hot_unplug_request_cb(HotplugHandler *hotplug_dev, uint8_t led; int slot; - shpc_device_hotplug_common(PCI_DEVICE(dev), &slot, shpc, errp); + shpc_device_hotplug_common(PCI_DEVICE(dev), &slot, shpc, &local_err); if (local_err) { + error_propagate(errp, local_err); return; } -- cgit v1.1 From fae38221e78fc9f847965f6d18b359b8044df348 Mon Sep 17 00:00:00 2001 From: zhanghailiang Date: Mon, 17 Nov 2014 13:57:34 +0800 Subject: target-cris/translate.c: fix out of bounds read In function t_gen_mov_TN_preg and t_gen_mov_preg_TN, The begin check about the validity of in-parameter 'r' is useless. We still access cpu_PR[r] in the follow code if it is invalid. Which will be an out-of-bounds read error. Fix it by using assert() to ensure it is valid before using it. Signed-off-by: zhanghailiang Signed-off-by: Paolo Bonzini --- target-cris/translate.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/target-cris/translate.c b/target-cris/translate.c index e37b04e..76406af 100644 --- a/target-cris/translate.c +++ b/target-cris/translate.c @@ -169,9 +169,7 @@ static int preg_sizes[] = { static inline void t_gen_mov_TN_preg(TCGv tn, int r) { - if (r < 0 || r > 15) { - fprintf(stderr, "wrong register read $p%d\n", r); - } + assert(r >= 0 && r <= 15); if (r == PR_BZ || r == PR_WZ || r == PR_DZ) { tcg_gen_mov_tl(tn, tcg_const_tl(0)); } else if (r == PR_VR) { @@ -182,9 +180,7 @@ static inline void t_gen_mov_TN_preg(TCGv tn, int r) } static inline void t_gen_mov_preg_TN(DisasContext *dc, int r, TCGv tn) { - if (r < 0 || r > 15) { - fprintf(stderr, "wrong register write $p%d\n", r); - } + assert(r >= 0 && r <= 15); if (r == PR_BZ || r == PR_WZ || r == PR_DZ) { return; } else if (r == PR_SRS) { -- cgit v1.1 From a9be76576e375a994bbcea0a5eb2a3852969de0e Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 17 Nov 2014 11:57:23 +0100 Subject: hcd-musb: fix dereference null return value usb_ep_get and usb_handle_packet can deal with a NULL device, but we have to avoid dereferencing NULL pointers when building the id. Thanks to Gonglei for an initial stab at fixing this. Signed-off-by: Paolo Bonzini --- hw/usb/hcd-musb.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/hw/usb/hcd-musb.c b/hw/usb/hcd-musb.c index 66bc61a..40809f6 100644 --- a/hw/usb/hcd-musb.c +++ b/hw/usb/hcd-musb.c @@ -608,6 +608,7 @@ static void musb_packet(MUSBState *s, MUSBEndPoint *ep, USBDevice *dev; USBEndpoint *uep; int idx = epnum && dir; + int id; int ttype; /* ep->type[0,1] contains: @@ -625,8 +626,11 @@ static void musb_packet(MUSBState *s, MUSBEndPoint *ep, /* A wild guess on the FADDR semantics... */ dev = usb_find_device(&s->port, ep->faddr[idx]); uep = usb_ep_get(dev, pid, ep->type[idx] & 0xf); - usb_packet_setup(&ep->packey[dir].p, pid, uep, 0, - (dev->addr << 16) | (uep->nr << 8) | pid, false, true); + id = pid; + if (uep) { + id |= (dev->addr << 16) | (uep->nr << 8); + } + usb_packet_setup(&ep->packey[dir].p, pid, uep, 0, id, false, true); usb_packet_addbuf(&ep->packey[dir].p, ep->buf[idx], len); ep->packey[dir].ep = ep; ep->packey[dir].dir = dir; -- cgit v1.1