From 7a8919dc29a9f46dcadd950c2aa1acf74f28974d Mon Sep 17 00:00:00 2001 From: Gonglei Date: Thu, 20 Nov 2014 19:35:00 +0800 Subject: net/slirp: fix memory leak commit b412eb61 introduce 'cmd:' target for guestfwd, and fwd don't be used in this scenario, and will leak memory in true branch with 'cmd:'. Let's allocate memory for fwd variable just in else statement. Cc: Alexander Graf Signed-off-by: Gonglei Reviewed-by: Jason Wang Signed-off-by: Stefan Hajnoczi --- net/slirp.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/net/slirp.c b/net/slirp.c index dc89e6b086..377d7ef8c0 100644 --- a/net/slirp.c +++ b/net/slirp.c @@ -643,17 +643,16 @@ static int slirp_guestfwd(SlirpState *s, const char *config_str, goto fail_syntax; } - fwd = g_malloc(sizeof(struct GuestFwd)); snprintf(buf, sizeof(buf), "guestfwd.tcp.%d", port); if ((strlen(p) > 4) && !strncmp(p, "cmd:", 4)) { if (slirp_add_exec(s->slirp, 0, &p[4], &server, port) < 0) { error_report("conflicting/invalid host:port in guest forwarding " "rule '%s'", config_str); - g_free(fwd); return -1; } } else { + fwd = g_malloc(sizeof(struct GuestFwd)); fwd->hd = qemu_chr_new(buf, p, NULL); if (!fwd->hd) { error_report("could not open guest forwarding device '%s'", buf); -- cgit v1.2.1 From 8db804ac412010fc96397c2d67ee6417eccd9d34 Mon Sep 17 00:00:00 2001 From: Gonglei Date: Thu, 20 Nov 2014 19:35:01 +0800 Subject: net/socket: fix Uninitialized scalar variable If is_connected parameter is false, the saddr variable will no initialize. Coverity report: uninit_use: Using uninitialized value saddr.sin_port. We don't need add saddr information to nc->info_str when is_connected is false. Signed-off-by: Gonglei Reviewed-by: Jason Wang Signed-off-by: Stefan Hajnoczi --- net/socket.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/net/socket.c b/net/socket.c index ca4b8ba2b3..68a93cd7e3 100644 --- a/net/socket.c +++ b/net/socket.c @@ -389,11 +389,6 @@ static NetSocketState *net_socket_fd_init_dgram(NetClientState *peer, nc = qemu_new_net_client(&net_dgram_socket_info, peer, model, name); - snprintf(nc->info_str, sizeof(nc->info_str), - "socket: fd=%d (%s mcast=%s:%d)", - fd, is_connected ? "cloned" : "", - inet_ntoa(saddr.sin_addr), ntohs(saddr.sin_port)); - s = DO_UPCAST(NetSocketState, nc, nc); s->fd = fd; @@ -404,6 +399,12 @@ static NetSocketState *net_socket_fd_init_dgram(NetClientState *peer, /* mcast: save bound address as dst */ if (is_connected) { s->dgram_dst = saddr; + snprintf(nc->info_str, sizeof(nc->info_str), + "socket: fd=%d (cloned mcast=%s:%d)", + fd, inet_ntoa(saddr.sin_addr), ntohs(saddr.sin_port)); + } else { + snprintf(nc->info_str, sizeof(nc->info_str), + "socket: fd=%d", fd); } return s; -- cgit v1.2.1 From 7b50d00911ddd6d56a766ac5671e47304c20a21b Mon Sep 17 00:00:00 2001 From: Gonglei Date: Thu, 20 Nov 2014 19:35:02 +0800 Subject: pcnet: fix Negative array index read s->xmit_pos maybe assigned to a negative value (-1), but in this branch variable s->xmit_pos as an index to array s->buffer. Let's add a check for s->xmit_pos. Signed-off-by: Gonglei Signed-off-by: Paolo Bonzini Reviewed-by: Jason Wang Reviewed-by: Jason Wang Signed-off-by: Stefan Hajnoczi --- hw/net/pcnet.c | 55 ++++++++++++++++++++++++++++++------------------------- 1 file changed, 30 insertions(+), 25 deletions(-) diff --git a/hw/net/pcnet.c b/hw/net/pcnet.c index d344c15da0..f409b9293d 100644 --- a/hw/net/pcnet.c +++ b/hw/net/pcnet.c @@ -1212,7 +1212,7 @@ static void pcnet_transmit(PCNetState *s) hwaddr xmit_cxda = 0; int count = CSR_XMTRL(s)-1; int add_crc = 0; - + int bcnt; s->xmit_pos = -1; if (!CSR_TXON(s)) { @@ -1247,35 +1247,40 @@ static void pcnet_transmit(PCNetState *s) s->xmit_pos = -1; goto txdone; } + + if (s->xmit_pos < 0) { + goto txdone; + } + + bcnt = 4096 - GET_FIELD(tmd.length, TMDL, BCNT); + s->phys_mem_read(s->dma_opaque, PHYSADDR(s, tmd.tbadr), + s->buffer + s->xmit_pos, bcnt, CSR_BSWP(s)); + s->xmit_pos += bcnt; + if (!GET_FIELD(tmd.status, TMDS, ENP)) { - int bcnt = 4096 - GET_FIELD(tmd.length, TMDL, BCNT); - s->phys_mem_read(s->dma_opaque, PHYSADDR(s, tmd.tbadr), - s->buffer + s->xmit_pos, bcnt, CSR_BSWP(s)); - s->xmit_pos += bcnt; - } else if (s->xmit_pos >= 0) { - int bcnt = 4096 - GET_FIELD(tmd.length, TMDL, BCNT); - s->phys_mem_read(s->dma_opaque, PHYSADDR(s, tmd.tbadr), - s->buffer + s->xmit_pos, bcnt, CSR_BSWP(s)); - s->xmit_pos += bcnt; + goto txdone; + } + #ifdef PCNET_DEBUG - printf("pcnet_transmit size=%d\n", s->xmit_pos); + printf("pcnet_transmit size=%d\n", s->xmit_pos); #endif - if (CSR_LOOP(s)) { - if (BCR_SWSTYLE(s) == 1) - add_crc = !GET_FIELD(tmd.status, TMDS, NOFCS); - s->looptest = add_crc ? PCNET_LOOPTEST_CRC : PCNET_LOOPTEST_NOCRC; - pcnet_receive(qemu_get_queue(s->nic), s->buffer, s->xmit_pos); - s->looptest = 0; - } else - if (s->nic) - qemu_send_packet(qemu_get_queue(s->nic), s->buffer, - s->xmit_pos); - - s->csr[0] &= ~0x0008; /* clear TDMD */ - s->csr[4] |= 0x0004; /* set TXSTRT */ - s->xmit_pos = -1; + if (CSR_LOOP(s)) { + if (BCR_SWSTYLE(s) == 1) + add_crc = !GET_FIELD(tmd.status, TMDS, NOFCS); + s->looptest = add_crc ? PCNET_LOOPTEST_CRC : PCNET_LOOPTEST_NOCRC; + pcnet_receive(qemu_get_queue(s->nic), s->buffer, s->xmit_pos); + s->looptest = 0; + } else { + if (s->nic) { + qemu_send_packet(qemu_get_queue(s->nic), s->buffer, + s->xmit_pos); + } } + s->csr[0] &= ~0x0008; /* clear TDMD */ + s->csr[4] |= 0x0004; /* set TXSTRT */ + s->xmit_pos = -1; + txdone: SET_FIELD(&tmd.status, TMDS, OWN, 0); TMDSTORE(&tmd, PHYSADDR(s,CSR_CXDA(s))); -- cgit v1.2.1 From b0af844007841609cc11fab58f838bd105cbe144 Mon Sep 17 00:00:00 2001 From: Gonglei Date: Thu, 20 Nov 2014 19:35:03 +0800 Subject: rtl8139: fix Pointer to local outside scope Coverity spot: Assigning: iov = struct iovec [3]({{buf, 12UL}, {(void *)dot1q_buf, 4UL}, {buf + 12, size - 12}}) (address of temporary variable of type struct iovec [3]). out_of_scope: Temporary variable of type struct iovec [3] goes out of scope. Pointer to local outside scope (RETURN_LOCAL) use_invalid: Using iov, which points to an out-of-scope temporary variable of type struct iovec [3]. Signed-off-by: Gonglei Signed-off-by: Paolo Bonzini Reviewed-by: Jason Wang Signed-off-by: Stefan Hajnoczi --- hw/net/rtl8139.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c index 8b8a1b18af..5f0197c9df 100644 --- a/hw/net/rtl8139.c +++ b/hw/net/rtl8139.c @@ -1775,6 +1775,7 @@ static void rtl8139_transfer_frame(RTL8139State *s, uint8_t *buf, int size, int do_interrupt, const uint8_t *dot1q_buf) { struct iovec *iov = NULL; + struct iovec vlan_iov[3]; if (!size) { @@ -1789,6 +1790,9 @@ static void rtl8139_transfer_frame(RTL8139State *s, uint8_t *buf, int size, { .iov_base = buf + ETHER_ADDR_LEN * 2, .iov_len = size - ETHER_ADDR_LEN * 2 }, }; + + memcpy(vlan_iov, iov, sizeof(vlan_iov)); + iov = vlan_iov; } if (TxLoopBack == (s->TxConfig & TxLoopBack)) -- cgit v1.2.1