From patchwork Thu Mar 12 20:16:38 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Peter Maydell X-Patchwork-Id: 184545 Delivered-To: patch@linaro.org Received: by 2002:a92:1f12:0:0:0:0:0 with SMTP id i18csp984694ile; Thu, 12 Mar 2020 13:16:56 -0700 (PDT) X-Google-Smtp-Source: ADFU+vvi7mFwOojP9eCihWDZPBu4xvvQXUQ0wDsSgFR5iWUU/O7rhtIfkBa932SBuV+CKf006UQz X-Received: by 2002:a37:7183:: with SMTP id m125mr9723047qkc.442.1584044216466; Thu, 12 Mar 2020 13:16:56 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1584044216; cv=none; d=google.com; s=arc-20160816; b=pe6spRjcqp6vmImHb+TSGVe55fwZiavO1juRdr/tIW/gqVVIB4j4sDNueDXlPj3vOZ osGPtGTm0nEkGndthCeME8Cr787MRy8WpnLaGblaLwbGAH4nyQUyD2wi6kUCC/Gitdxm CLZHxrzlkTWu97lZ2etHeowg5bQiJBNB22lkZrIKw3Rri4R0TNKtbqWfT4F7/6/ka5mV E36JXENFxkjkP1HvO6BQMYOKCXCWCPcFxiKcZP69TnfnRi1kRBOWp8loqsEWru4sk7DY O0F9jVONfqrdTmuK587F2V2af3JwEo2KBZWr6mDBFMx7C8/4xyWgCuUDyRdbempr4p4I Cjlg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:cc:list-subscribe:list-help:list-post:list-archive :list-unsubscribe:list-id:precedence:content-transfer-encoding :mime-version:message-id:date:subject:to:from:dkim-signature; bh=8Tro1z7uTIp12iQuDnwssQFro2Zzq8+IX8DXdBO8SLQ=; b=laEsl5cEkSAz4BN6a6S7jxmVkLJiKOQszwiR5zhS5zKC1JuNVjV1OzhgUfqgLVFy2s 2mKTTl0pjBJeyiGLqI/clL15nhZI+mSgAP6zSH61F1+4gT5wWsNJJttEgsIYTbnO4+zw jl5KoTDTP3MdP4+cJDl9JFcga3ovqfWoh1qAFG3u0tATw/ebaAqJSq5QAWHqDzQkKh1F p3rtT0E3CbY/TFTiu17TjiTuqohghz965wBBxZ3OtT0Y4/0eFwUDi20kWq+TWOdFohvW zpesxPC7uw2b7AjShkf2QwbHZyXeD1/B4EIJxYWJFpdOj/JMSKxnif3VLlP4XBRLc2bX Rrrg== ARC-Authentication-Results: i=1; mx.google.com; dkim=fail header.i=@linaro.org header.s=google header.b=AI8YIRvI; spf=pass (google.com: domain of qemu-devel-bounces+patch=linaro.org@nongnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom="qemu-devel-bounces+patch=linaro.org@nongnu.org"; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=linaro.org Return-Path: Received: from lists.gnu.org (lists.gnu.org. [209.51.188.17]) by mx.google.com with ESMTPS id e16si4032577qtr.370.2020.03.12.13.16.56 for (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Thu, 12 Mar 2020 13:16:56 -0700 (PDT) Received-SPF: pass (google.com: domain of qemu-devel-bounces+patch=linaro.org@nongnu.org designates 209.51.188.17 as permitted sender) client-ip=209.51.188.17; Authentication-Results: mx.google.com; dkim=fail header.i=@linaro.org header.s=google header.b=AI8YIRvI; spf=pass (google.com: domain of qemu-devel-bounces+patch=linaro.org@nongnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom="qemu-devel-bounces+patch=linaro.org@nongnu.org"; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=linaro.org Received: from localhost ([::1]:50140 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1jCUGF-0005cS-W3 for patch@linaro.org; Thu, 12 Mar 2020 16:16:56 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:53495) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1jCUG5-0005cK-4J for qemu-devel@nongnu.org; Thu, 12 Mar 2020 16:16:46 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1jCUG3-0008VR-Ls for qemu-devel@nongnu.org; Thu, 12 Mar 2020 16:16:44 -0400 Received: from mail-wr1-x442.google.com ([2a00:1450:4864:20::442]:33603) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1jCUG3-0008V3-F9 for qemu-devel@nongnu.org; Thu, 12 Mar 2020 16:16:43 -0400 Received: by mail-wr1-x442.google.com with SMTP id a25so9206224wrd.0 for ; Thu, 12 Mar 2020 13:16:43 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=8Tro1z7uTIp12iQuDnwssQFro2Zzq8+IX8DXdBO8SLQ=; b=AI8YIRvIjbVTY7oDAMagQPOfSx5jEK2SnxmcDtTszUGceWT9+scIBaU5dOugoZIirE uPuscEaLPslrVICKx9MIImg6roh8kF0QrSzt4ixjxBWjBZjgjv0x0MgNesXYP20+EN0P P1zZlTGFKD1Uj4LFCBKBwXV8hLEA/0xgq+B6QUxiTePfgyhVDfxAhQO+sL3H7nhuR0nO WB022es1Ukuq7LBGaiB7SvcC1M2s936fy4Wx2pIXtbD+D7A6so3OTvTFUEPvh7YeRKkR PWmgjXWX3WaPe+yk1QEQhIB67Q6L0kwCSkLepo0a/MSZE+cATR6Ws4E6pERbPd8HOGoq DxDw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=8Tro1z7uTIp12iQuDnwssQFro2Zzq8+IX8DXdBO8SLQ=; b=ixcSFJAH/GjdU0Qs+Dsf3giluUFkwlQN/SUDZO66STY0OoimQaUPP9ucLBgIdCHmHn rotvE+Yhx5+KvFKUju8uYtyqokffgkxKAQwIvCjH82rWhfT9DtBnPAx9hlvUVKemS3GX RHZk85St7AvxzB1D4GlPWh5cKvTfW5MgK5UB4zfOPGagTW29UlCBggqyjhDlKANGUTTt Ir6xPeRSRWkCn8dsW7eKEt4hW4CyFH1mOm0bhq8w6bFIh/tddi2+K04x59y2vri6vHkz YqU2rcVZoeGwofYyF6jCpUgl1PqEvBngMHRZfNyj40lh8k1a8qcoBjdJ2nP9pTN8W4eR ggZg== X-Gm-Message-State: ANhLgQ22MlJ+TPLNJdkpCmKzuLOxnedEKj7M983QhBagxALCiWKsbcj2 yICX3swVOL7Krp3FcJVImSWIIff28qjoWA== X-Received: by 2002:a5d:6150:: with SMTP id y16mr12915859wrt.352.1584044201685; Thu, 12 Mar 2020 13:16:41 -0700 (PDT) Received: from orth.archaic.org.uk (orth.archaic.org.uk. [81.2.115.148]) by smtp.gmail.com with ESMTPSA id p16sm4401554wmg.22.2020.03.12.13.16.40 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 12 Mar 2020 13:16:40 -0700 (PDT) From: Peter Maydell To: qemu-devel@nongnu.org Subject: [PATCH] hw/net/i82596.c: Avoid reading off end of buffer in i82596_receive() Date: Thu, 12 Mar 2020 20:16:38 +0000 Message-Id: <20200312201638.6375-1-peter.maydell@linaro.org> X-Mailer: git-send-email 2.20.1 MIME-Version: 1.0 X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 2a00:1450:4864:20::442 X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Helge Deller , Jason Wang , Richard Henderson Errors-To: qemu-devel-bounces+patch=linaro.org@nongnu.org Sender: "Qemu-devel" The i82596_receive() function attempts to pass the guest a buffer which is effectively the concatenation of the data it is passed and a 4 byte CRC value. However, rather than implementing this as "write the data; then write the CRC" it instead bumps the length value of the data by 4, and writes 4 extra bytes from beyond the end of the buffer, which it then overwrites with the CRC. It also assumed that we could always fit all four bytes of the CRC into the final receive buffer, which might not be true if the CRC needs to be split over two receive buffers. Calculate separately how many bytes we need to transfer into the guest's receive buffer from the source buffer, and how many we need to transfer from the CRC work. We add a count 'bufsz' of the number of bytes left in the source buffer, which we use purely to assert() that we don't overrun. Spotted by Coverity (CID 1419396) for the specific case when we end up using a local array as the source buffer. Signed-off-by: Peter Maydell --- I know Helge has some significant rework of this device planned, but for 5.0 we need to fix the buffer overrun. Tested with 'make check' only. --- hw/net/i82596.c | 44 +++++++++++++++++++++++++++++++++++--------- 1 file changed, 35 insertions(+), 9 deletions(-) -- 2.20.1 diff --git a/hw/net/i82596.c b/hw/net/i82596.c index fe9f2390a94..2bd5d310367 100644 --- a/hw/net/i82596.c +++ b/hw/net/i82596.c @@ -501,7 +501,8 @@ ssize_t i82596_receive(NetClientState *nc, const uint8_t *buf, size_t sz) uint32_t rfd_p; uint32_t rbd; uint16_t is_broadcast = 0; - size_t len = sz; + size_t len = sz; /* length of data for guest (including CRC) */ + size_t bufsz = sz; /* length of data in buf */ uint32_t crc; uint8_t *crc_ptr; uint8_t buf1[MIN_BUF_SIZE + VLAN_HLEN]; @@ -595,6 +596,7 @@ ssize_t i82596_receive(NetClientState *nc, const uint8_t *buf, size_t sz) if (len < MIN_BUF_SIZE) { len = MIN_BUF_SIZE; } + bufsz = len; } /* Calculate the ethernet checksum (4 bytes) */ @@ -627,6 +629,7 @@ ssize_t i82596_receive(NetClientState *nc, const uint8_t *buf, size_t sz) while (len) { uint16_t buffer_size, num; uint32_t rba; + size_t bufcount, crccount; /* printf("Receive: rbd is %08x\n", rbd); */ buffer_size = get_uint16(rbd + 12); @@ -639,14 +642,37 @@ ssize_t i82596_receive(NetClientState *nc, const uint8_t *buf, size_t sz) } rba = get_uint32(rbd + 8); /* printf("rba is 0x%x\n", rba); */ - address_space_write(&address_space_memory, rba, - MEMTXATTRS_UNSPECIFIED, buf, num); - rba += num; - buf += num; - len -= num; - if (len == 0) { /* copy crc */ - address_space_write(&address_space_memory, rba - 4, - MEMTXATTRS_UNSPECIFIED, crc_ptr, 4); + /* + * Calculate how many bytes we want from buf[] and how many + * from the CRC. + */ + if ((len - num) >= 4) { + /* The whole guest buffer, we haven't hit the CRC yet */ + bufcount = num; + } else { + /* All that's left of buf[] */ + bufcount = len - 4; + } + crccount = num - bufcount; + + if (bufcount > 0) { + /* Still some of the actual data buffer to transfer */ + bufsz -= bufcount; + assert(bufsz >= 0); + address_space_write(&address_space_memory, rba, + MEMTXATTRS_UNSPECIFIED, buf, bufcount); + rba += bufcount; + buf += bufcount; + len -= bufcount; + } + + /* Write as much of the CRC as fits */ + if (crccount > 0) { + address_space_write(&address_space_memory, rba, + MEMTXATTRS_UNSPECIFIED, crc_ptr, crccount); + rba += crccount; + crc_ptr += crccount; + len -= crccount; } num |= 0x4000; /* set F BIT */