From patchwork Fri Jan 20 03:07:48 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Jason Wang X-Patchwork-Id: 92024 Delivered-To: patch@linaro.org Received: by 10.140.20.99 with SMTP id 90csp588068qgi; Thu, 19 Jan 2017 19:13:30 -0800 (PST) X-Received: by 10.55.221.5 with SMTP id n5mr10170814qki.58.1484882010700; Thu, 19 Jan 2017 19:13:30 -0800 (PST) Return-Path: Received: from lists.gnu.org (lists.gnu.org. [2001:4830:134:3::11]) by mx.google.com with ESMTPS id a2si3960632qkh.324.2017.01.19.19.13.30 for (version=TLS1 cipher=AES128-SHA bits=128/128); Thu, 19 Jan 2017 19:13:30 -0800 (PST) Received-SPF: pass (google.com: domain of qemu-devel-bounces+patch=linaro.org@nongnu.org designates 2001:4830:134:3::11 as permitted sender) client-ip=2001:4830:134:3::11; Authentication-Results: mx.google.com; spf=pass (google.com: domain of qemu-devel-bounces+patch=linaro.org@nongnu.org designates 2001:4830:134:3::11 as permitted sender) smtp.mailfrom=qemu-devel-bounces+patch=linaro.org@nongnu.org Received: from localhost ([::1]:52240 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cUPdo-0004E8-1h for patch@linaro.org; Thu, 19 Jan 2017 22:13:28 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39579) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cUPYf-0000W3-4c for qemu-devel@nongnu.org; Thu, 19 Jan 2017 22:08:12 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1cUPYc-0002sY-DM for qemu-devel@nongnu.org; Thu, 19 Jan 2017 22:08:09 -0500 Received: from mx1.redhat.com ([209.132.183.28]:57178) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1cUPYc-0002ry-3r for qemu-devel@nongnu.org; Thu, 19 Jan 2017 22:08:06 -0500 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 5BA6161B8E; Fri, 20 Jan 2017 03:08:06 +0000 (UTC) Received: from jason-ThinkPad-T450s.redhat.com (vpn1-6-106.pek2.redhat.com [10.72.6.106]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id v0K37r7d014507; Thu, 19 Jan 2017 22:08:04 -0500 From: Jason Wang To: peter.maydell@linaro.org, qemu-devel@nongnu.org Date: Fri, 20 Jan 2017 11:07:48 +0800 Message-Id: <1484881670-24237-5-git-send-email-jasowang@redhat.com> In-Reply-To: <1484881670-24237-1-git-send-email-jasowang@redhat.com> References: <1484881670-24237-1-git-send-email-jasowang@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.68 on 10.5.11.24 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.39]); Fri, 20 Jan 2017 03:08:06 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PULL 4/6] hw/net/dp8393x: Avoid unintentional sign extensions on addresses X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Jason Wang Errors-To: qemu-devel-bounces+patch=linaro.org@nongnu.org Sender: "Qemu-devel" From: Peter Maydell The dp8393x has several 32-bit values which are formed by concatenating two 16 bit device register values. Attempting to do these inline with ((s->reg[HI] << 16) | s->reg[LO]) can result in an unintended sign extension because "x << 16" is of type 'int' even though s->reg is unsigned, and so if the expression is used in a context where it is cast to uint64_t the value is incorrectly sign-extended. Fix this by using accessor functions with a uint32_t return type; this also makes the code a bit easier to read. This should fix Coverity issues 1307765, 1307766, 1307767, 1307768. (To avoid having a ctda read function only used in a DPRINTF, we move the DPRINTF down slightly so it can use the ttda function.) Reviewed-by: Laurent Vivier Tested-by: Laurent Vivier Reviewed-by: Hervé Poussineau Signed-off-by: Peter Maydell Signed-off-by: Jason Wang --- hw/net/dp8393x.c | 95 ++++++++++++++++++++++++++++++++++++++++---------------- 1 file changed, 68 insertions(+), 27 deletions(-) -- 2.7.4 diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c index 17f0338..efa33ad 100644 --- a/hw/net/dp8393x.c +++ b/hw/net/dp8393x.c @@ -174,6 +174,52 @@ typedef struct dp8393xState { AddressSpace as; } dp8393xState; +/* Accessor functions for values which are formed by + * concatenating two 16 bit device registers. By putting these + * in their own functions with a uint32_t return type we avoid the + * pitfall of implicit sign extension where ((x << 16) | y) is a + * signed 32 bit integer that might get sign-extended to a 64 bit integer. + */ +static uint32_t dp8393x_cdp(dp8393xState *s) +{ + return (s->regs[SONIC_URRA] << 16) | s->regs[SONIC_CDP]; +} + +static uint32_t dp8393x_crba(dp8393xState *s) +{ + return (s->regs[SONIC_CRBA1] << 16) | s->regs[SONIC_CRBA0]; +} + +static uint32_t dp8393x_crda(dp8393xState *s) +{ + return (s->regs[SONIC_URDA] << 16) | s->regs[SONIC_CRDA]; +} + +static uint32_t dp8393x_rbwc(dp8393xState *s) +{ + return (s->regs[SONIC_RBWC1] << 16) | s->regs[SONIC_RBWC0]; +} + +static uint32_t dp8393x_rrp(dp8393xState *s) +{ + return (s->regs[SONIC_URRA] << 16) | s->regs[SONIC_RRP]; +} + +static uint32_t dp8393x_tsa(dp8393xState *s) +{ + return (s->regs[SONIC_TSA1] << 16) | s->regs[SONIC_TSA0]; +} + +static uint32_t dp8393x_ttda(dp8393xState *s) +{ + return (s->regs[SONIC_UTDA] << 16) | s->regs[SONIC_TTDA]; +} + +static uint32_t dp8393x_wt(dp8393xState *s) +{ + return s->regs[SONIC_WT1] << 16 | s->regs[SONIC_WT0]; +} + static void dp8393x_update_irq(dp8393xState *s) { int level = (s->regs[SONIC_IMR] & s->regs[SONIC_ISR]) ? 1 : 0; @@ -203,8 +249,7 @@ static void dp8393x_do_load_cam(dp8393xState *s) while (s->regs[SONIC_CDC] & 0x1f) { /* Fill current entry */ - address_space_rw(&s->as, - (s->regs[SONIC_URRA] << 16) | s->regs[SONIC_CDP], + address_space_rw(&s->as, dp8393x_cdp(s), MEMTXATTRS_UNSPECIFIED, (uint8_t *)data, size, 0); s->cam[index][0] = data[1 * width] & 0xff; s->cam[index][1] = data[1 * width] >> 8; @@ -222,8 +267,7 @@ static void dp8393x_do_load_cam(dp8393xState *s) } /* Read CAM enable */ - address_space_rw(&s->as, - (s->regs[SONIC_URRA] << 16) | s->regs[SONIC_CDP], + address_space_rw(&s->as, dp8393x_cdp(s), MEMTXATTRS_UNSPECIFIED, (uint8_t *)data, size, 0); s->regs[SONIC_CE] = data[0 * width]; DPRINTF("load cam done. cam enable mask 0x%04x\n", s->regs[SONIC_CE]); @@ -242,8 +286,7 @@ static void dp8393x_do_read_rra(dp8393xState *s) /* Read memory */ width = (s->regs[SONIC_DCR] & SONIC_DCR_DW) ? 2 : 1; size = sizeof(uint16_t) * 4 * width; - address_space_rw(&s->as, - (s->regs[SONIC_URRA] << 16) | s->regs[SONIC_RRP], + address_space_rw(&s->as, dp8393x_rrp(s), MEMTXATTRS_UNSPECIFIED, (uint8_t *)data, size, 0); /* Update SONIC registers */ @@ -292,7 +335,7 @@ static void dp8393x_set_next_tick(dp8393xState *s) return; } - ticks = s->regs[SONIC_WT1] << 16 | s->regs[SONIC_WT0]; + ticks = dp8393x_wt(s); s->wt_last_update = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); delay = NANOSECONDS_PER_SECOND * ticks / 5000000; timer_mod(s->watchdog, s->wt_last_update + delay); @@ -309,7 +352,7 @@ static void dp8393x_update_wt_regs(dp8393xState *s) } elapsed = s->wt_last_update - qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); - val = s->regs[SONIC_WT1] << 16 | s->regs[SONIC_WT0]; + val = dp8393x_wt(s); val -= elapsed / 5000000; s->regs[SONIC_WT1] = (val >> 16) & 0xffff; s->regs[SONIC_WT0] = (val >> 0) & 0xffff; @@ -356,12 +399,11 @@ static void dp8393x_do_transmit_packets(dp8393xState *s) while (1) { /* Read memory */ - DPRINTF("Transmit packet at %08x\n", - (s->regs[SONIC_UTDA] << 16) | s->regs[SONIC_CTDA]); size = sizeof(uint16_t) * 6 * width; s->regs[SONIC_TTDA] = s->regs[SONIC_CTDA]; + DPRINTF("Transmit packet at %08x\n", dp8393x_ttda(s)); address_space_rw(&s->as, - ((s->regs[SONIC_UTDA] << 16) | s->regs[SONIC_TTDA]) + sizeof(uint16_t) * width, + dp8393x_ttda(s) + sizeof(uint16_t) * width, MEMTXATTRS_UNSPECIFIED, (uint8_t *)data, size, 0); tx_len = 0; @@ -386,8 +428,7 @@ static void dp8393x_do_transmit_packets(dp8393xState *s) if (tx_len + len > sizeof(s->tx_buffer)) { len = sizeof(s->tx_buffer) - tx_len; } - address_space_rw(&s->as, - (s->regs[SONIC_TSA1] << 16) | s->regs[SONIC_TSA0], + address_space_rw(&s->as, dp8393x_tsa(s), MEMTXATTRS_UNSPECIFIED, &s->tx_buffer[tx_len], len, 0); tx_len += len; @@ -396,7 +437,7 @@ static void dp8393x_do_transmit_packets(dp8393xState *s) /* Read next fragment details */ size = sizeof(uint16_t) * 3 * width; address_space_rw(&s->as, - ((s->regs[SONIC_UTDA] << 16) | s->regs[SONIC_TTDA]) + sizeof(uint16_t) * (4 + 3 * i) * width, + dp8393x_ttda(s) + sizeof(uint16_t) * (4 + 3 * i) * width, MEMTXATTRS_UNSPECIFIED, (uint8_t *)data, size, 0); s->regs[SONIC_TSA0] = data[0 * width]; s->regs[SONIC_TSA1] = data[1 * width]; @@ -430,14 +471,16 @@ static void dp8393x_do_transmit_packets(dp8393xState *s) data[0 * width] = s->regs[SONIC_TCR] & 0x0fff; /* status */ size = sizeof(uint16_t) * width; address_space_rw(&s->as, - (s->regs[SONIC_UTDA] << 16) | s->regs[SONIC_TTDA], + dp8393x_ttda(s), MEMTXATTRS_UNSPECIFIED, (uint8_t *)data, size, 1); if (!(s->regs[SONIC_CR] & SONIC_CR_HTX)) { /* Read footer of packet */ size = sizeof(uint16_t) * width; address_space_rw(&s->as, - ((s->regs[SONIC_UTDA] << 16) | s->regs[SONIC_TTDA]) + sizeof(uint16_t) * (4 + 3 * s->regs[SONIC_TFC]) * width, + dp8393x_ttda(s) + + sizeof(uint16_t) * + (4 + 3 * s->regs[SONIC_TFC]) * width, MEMTXATTRS_UNSPECIFIED, (uint8_t *)data, size, 0); s->regs[SONIC_CTDA] = data[0 * width] & ~0x1; if (data[0 * width] & 0x1) { @@ -700,7 +743,7 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf, if (s->regs[SONIC_LLFA] & 0x1) { /* Are we still in resource exhaustion? */ size = sizeof(uint16_t) * 1 * width; - address = ((s->regs[SONIC_URDA] << 16) | s->regs[SONIC_CRDA]) + sizeof(uint16_t) * 5 * width; + address = dp8393x_crda(s) + sizeof(uint16_t) * 5 * width; address_space_rw(&s->as, address, MEMTXATTRS_UNSPECIFIED, (uint8_t *)data, size, 0); if (data[0 * width] & 0x1) { @@ -719,8 +762,8 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf, checksum = cpu_to_le32(crc32(0, buf, rx_len)); /* Put packet into RBA */ - DPRINTF("Receive packet at %08x\n", (s->regs[SONIC_CRBA1] << 16) | s->regs[SONIC_CRBA0]); - address = (s->regs[SONIC_CRBA1] << 16) | s->regs[SONIC_CRBA0]; + DPRINTF("Receive packet at %08x\n", dp8393x_crba(s)); + address = dp8393x_crba(s); address_space_rw(&s->as, address, MEMTXATTRS_UNSPECIFIED, (uint8_t *)buf, rx_len, 1); address += rx_len; @@ -729,13 +772,13 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf, rx_len += 4; s->regs[SONIC_CRBA1] = address >> 16; s->regs[SONIC_CRBA0] = address & 0xffff; - available = (s->regs[SONIC_RBWC1] << 16) | s->regs[SONIC_RBWC0]; + available = dp8393x_rbwc(s); available -= rx_len / 2; s->regs[SONIC_RBWC1] = available >> 16; s->regs[SONIC_RBWC0] = available & 0xffff; /* Update status */ - if (((s->regs[SONIC_RBWC1] << 16) | s->regs[SONIC_RBWC0]) < s->regs[SONIC_EOBC]) { + if (dp8393x_rbwc(s) < s->regs[SONIC_EOBC]) { s->regs[SONIC_RCR] |= SONIC_RCR_LPKT; } s->regs[SONIC_RCR] |= packet_type; @@ -746,20 +789,19 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf, } /* Write status to memory */ - DPRINTF("Write status at %08x\n", (s->regs[SONIC_URDA] << 16) | s->regs[SONIC_CRDA]); + DPRINTF("Write status at %08x\n", dp8393x_crda(s)); data[0 * width] = s->regs[SONIC_RCR]; /* status */ data[1 * width] = rx_len; /* byte count */ data[2 * width] = s->regs[SONIC_TRBA0]; /* pkt_ptr0 */ data[3 * width] = s->regs[SONIC_TRBA1]; /* pkt_ptr1 */ data[4 * width] = s->regs[SONIC_RSC]; /* seq_no */ size = sizeof(uint16_t) * 5 * width; - address_space_rw(&s->as, (s->regs[SONIC_URDA] << 16) | s->regs[SONIC_CRDA], + address_space_rw(&s->as, dp8393x_crda(s), MEMTXATTRS_UNSPECIFIED, (uint8_t *)data, size, 1); /* Move to next descriptor */ size = sizeof(uint16_t) * width; - address_space_rw(&s->as, - ((s->regs[SONIC_URDA] << 16) | s->regs[SONIC_CRDA]) + sizeof(uint16_t) * 5 * width, + address_space_rw(&s->as, dp8393x_crda(s) + sizeof(uint16_t) * 5 * width, MEMTXATTRS_UNSPECIFIED, (uint8_t *)data, size, 0); s->regs[SONIC_LLFA] = data[0 * width]; if (s->regs[SONIC_LLFA] & 0x1) { @@ -767,8 +809,7 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf, s->regs[SONIC_ISR] |= SONIC_ISR_RDE; } else { data[0 * width] = 0; /* in_use */ - address_space_rw(&s->as, - ((s->regs[SONIC_URDA] << 16) | s->regs[SONIC_CRDA]) + sizeof(uint16_t) * 6 * width, + address_space_rw(&s->as, dp8393x_crda(s) + sizeof(uint16_t) * 6 * width, MEMTXATTRS_UNSPECIFIED, (uint8_t *)data, sizeof(uint16_t), 1); s->regs[SONIC_CRDA] = s->regs[SONIC_LLFA]; s->regs[SONIC_ISR] |= SONIC_ISR_PKTRX; From patchwork Fri Jan 20 03:07:50 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jason Wang X-Patchwork-Id: 92025 Delivered-To: patch@linaro.org Received: by 10.140.20.99 with SMTP id 90csp589191qgi; Thu, 19 Jan 2017 19:18:10 -0800 (PST) X-Received: by 10.200.56.211 with SMTP id g19mr10289013qtc.177.1484882290775; Thu, 19 Jan 2017 19:18:10 -0800 (PST) Return-Path: Received: from lists.gnu.org (lists.gnu.org. [2001:4830:134:3::11]) by mx.google.com with ESMTPS id 42si3978201qtv.293.2017.01.19.19.18.10 for (version=TLS1 cipher=AES128-SHA bits=128/128); Thu, 19 Jan 2017 19:18:10 -0800 (PST) Received-SPF: pass (google.com: domain of qemu-devel-bounces+patch=linaro.org@nongnu.org designates 2001:4830:134:3::11 as permitted sender) client-ip=2001:4830:134:3::11; Authentication-Results: mx.google.com; spf=pass (google.com: domain of qemu-devel-bounces+patch=linaro.org@nongnu.org designates 2001:4830:134:3::11 as permitted sender) smtp.mailfrom=qemu-devel-bounces+patch=linaro.org@nongnu.org Received: from localhost ([::1]:52266 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cUPiK-0007H8-7i for patch@linaro.org; Thu, 19 Jan 2017 22:18:08 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39609) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cUPYh-0000WG-UN for qemu-devel@nongnu.org; Thu, 19 Jan 2017 22:08:12 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1cUPYg-0002uV-WD for qemu-devel@nongnu.org; Thu, 19 Jan 2017 22:08:11 -0500 Received: from mx1.redhat.com ([209.132.183.28]:54332) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1cUPYg-0002uG-QA for qemu-devel@nongnu.org; Thu, 19 Jan 2017 22:08:10 -0500 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 0CB143B721; Fri, 20 Jan 2017 03:08:11 +0000 (UTC) Received: from jason-ThinkPad-T450s.redhat.com (vpn1-6-106.pek2.redhat.com [10.72.6.106]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id v0K37r7f014507; Thu, 19 Jan 2017 22:08:09 -0500 From: Jason Wang To: peter.maydell@linaro.org, qemu-devel@nongnu.org Date: Fri, 20 Jan 2017 11:07:50 +0800 Message-Id: <1484881670-24237-7-git-send-email-jasowang@redhat.com> In-Reply-To: <1484881670-24237-1-git-send-email-jasowang@redhat.com> References: <1484881670-24237-1-git-send-email-jasowang@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.24 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.30]); Fri, 20 Jan 2017 03:08:11 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PULL 6/6] tap: fix memory leak on failure in net_init_tap() X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Jason Wang Errors-To: qemu-devel-bounces+patch=linaro.org@nongnu.org Sender: "Qemu-devel" From: Peter Maydell Commit 091a6b2ac fixed most of the memory leaks in failure paths in net_init_tap() reported by Coverity (CID 1356216), but missed one. Fix it by deferring the allocation of fds and vhost_fds until after the error check. Signed-off-by: Peter Maydell Signed-off-by: Jason Wang --- net/tap.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) -- 2.7.4 diff --git a/net/tap.c b/net/tap.c index b6896a7..6248e85 100644 --- a/net/tap.c +++ b/net/tap.c @@ -788,8 +788,8 @@ int net_init_tap(const Netdev *netdev, const char *name, return -1; } } else if (tap->has_fds) { - char **fds = g_new0(char *, MAX_TAP_QUEUES); - char **vhost_fds = g_new0(char *, MAX_TAP_QUEUES); + char **fds; + char **vhost_fds; int nfds, nvhosts; if (tap->has_ifname || tap->has_script || tap->has_downscript || @@ -801,6 +801,9 @@ int net_init_tap(const Netdev *netdev, const char *name, return -1; } + fds = g_new0(char *, MAX_TAP_QUEUES); + vhost_fds = g_new0(char *, MAX_TAP_QUEUES); + nfds = get_fds(tap->fds, fds, MAX_TAP_QUEUES); if (tap->has_vhostfds) { nvhosts = get_fds(tap->vhostfds, vhost_fds, MAX_TAP_QUEUES);