From patchwork Fri Mar 27 11:13:56 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jason Wang X-Patchwork-Id: 185005 Delivered-To: patch@linaro.org Received: by 2002:a92:de47:0:0:0:0:0 with SMTP id e7csp1174153ilr; Fri, 27 Mar 2020 04:14:36 -0700 (PDT) X-Google-Smtp-Source: ADFU+vtmhj7sJjgXSSQK3hOq1y3OrOIdrysWTGBRKgqDtJzoSRFrxXBC+WJDSoA9QYd/ntlbUY85 X-Received: by 2002:a37:a755:: with SMTP id q82mr13840891qke.390.1585307675983; Fri, 27 Mar 2020 04:14:35 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1585307675; cv=none; d=google.com; s=arc-20160816; b=o4/10VP2NAIp0Sa4Y3h3rbWhMLugDh5XNcJfd6X5HHl01QUtkzVIa2jGKwNRd9Vn7U G64WpSRvFdB9wlmZEc/vPHSyseLKspFXrriwncJz+7VDPZZyGnlHSPq/WkjQdPQ0vZ7E kPZys/7WTqTHw9l60TvIHk9NNv2unfFIbrF+wUt/hSqNqYov6h7ZAxOLoQ+petv2T4vt owbRzlbcoPEHsGAAp4lB2tac4R52AxM7z014m9CKFh59NONZdeex4z2rDg5J8Odt43iK UZktpRwLJHy3wjM4Cz+eTPc6X8X+UVTcToKyg4aJV+iErtbX2anyVb0fyFhq88XIGc1d Tjwg== 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 :references:in-reply-to:message-id:date:subject:to:from :dkim-signature; bh=qtWw28fp9uAXP1+QUNc+crdXF5Chv0nlMJAFt9zgSUY=; b=QiabKeECM9Fq5A12E3FHWU75jE5MbUIfE5xevGKAc9ZNBlYL4wACAzCUgFjBUvZzWi 57GVk+0PKaa3T/UrPy0QvqJl5vIyyfzO4H4fH4xzc+YalVeQP19xn6kYXWusgbJ2t+y6 8y0fPX4wjsmd/5WKSd2qhidAVwpCI9i7Dnajd526JiyPL458Y5Antww34rrv7uoHHjQp DAN7rnfejiybggg5d1nJcewGTmjW7CnDaYgPHxMoKsVBi0clrMIq0G9V3ZPAxb4Ll6Mf emD4EKE0bgr7LEn0U504T1ksSQXCcbhB61NlVXNhLrR65Zw6+P7vLiODN7UFXK6J0dIc Qwxg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=g5JGauPm; 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=pass (p=NONE sp=NONE dis=NONE) header.from=redhat.com Return-Path: Received: from lists.gnu.org (lists.gnu.org. [209.51.188.17]) by mx.google.com with ESMTPS id g13si3184356qke.105.2020.03.27.04.14.35 for (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Fri, 27 Mar 2020 04:14:35 -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=pass header.i=@redhat.com header.s=mimecast20190719 header.b=g5JGauPm; 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=pass (p=NONE sp=NONE dis=NONE) header.from=redhat.com Received: from localhost ([::1]:40276 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1jHmwd-0000hQ-Ez for patch@linaro.org; Fri, 27 Mar 2020 07:14:35 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:37735) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1jHmwQ-0000dr-2I for qemu-devel@nongnu.org; Fri, 27 Mar 2020 07:14:23 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1jHmwO-0001Qy-PM for qemu-devel@nongnu.org; Fri, 27 Mar 2020 07:14:21 -0400 Received: from us-smtp-delivery-74.mimecast.com ([216.205.24.74]:40931) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1jHmwO-0001QE-Lj for qemu-devel@nongnu.org; Fri, 27 Mar 2020 07:14:20 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1585307660; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=qtWw28fp9uAXP1+QUNc+crdXF5Chv0nlMJAFt9zgSUY=; b=g5JGauPmuWLyPT+vqcqJ0CFXP8Nj1dIP9LSJWv7Ufhd1taycD2AUN/ZJyIXc+YMRZ9on1m Z/B7ml1l+KTUnIJS0uj4Pin9X4HRqO1TmpJXOH7X/OSLRXS/aE3y15q3o672J8ubKxyHKX kHgPk0UoM6UNKcajGY2jEJIerH/4hYE= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-177-G9Ugy_tQM5Kam8E336TqgQ-1; Fri, 27 Mar 2020 07:14:16 -0400 X-MC-Unique: G9Ugy_tQM5Kam8E336TqgQ-1 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 7CF1D800D4E; Fri, 27 Mar 2020 11:14:15 +0000 (UTC) Received: from jason-ThinkPad-T430s.redhat.com (ovpn-12-77.pek2.redhat.com [10.72.12.77]) by smtp.corp.redhat.com (Postfix) with ESMTP id 1C60F5E01D; Fri, 27 Mar 2020 11:14:13 +0000 (UTC) From: Jason Wang To: qemu-devel@nongnu.org, peter.maydell@linaro.org Subject: [PULL 02/13] hw/net/i82596.c: Avoid reading off end of buffer in i82596_receive() Date: Fri, 27 Mar 2020 19:13:56 +0800 Message-Id: <1585307647-24456-3-git-send-email-jasowang@redhat.com> In-Reply-To: <1585307647-24456-1-git-send-email-jasowang@redhat.com> References: <1585307647-24456-1-git-send-email-jasowang@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 216.205.24.74 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: Jason Wang Errors-To: qemu-devel-bounces+patch=linaro.org@nongnu.org Sender: "Qemu-devel" From: Peter Maydell 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 Signed-off-by: Jason Wang --- hw/net/i82596.c | 44 +++++++++++++++++++++++++++++++++++--------- 1 file changed, 35 insertions(+), 9 deletions(-) -- 2.5.0 diff --git a/hw/net/i82596.c b/hw/net/i82596.c index ecdb9aa..f9607ad 100644 --- a/hw/net/i82596.c +++ b/hw/net/i82596.c @@ -497,7 +497,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]; @@ -591,6 +592,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) */ @@ -623,6 +625,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); @@ -635,14 +638,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 */