From patchwork Mon Mar 21 17:28:25 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Michael Roth X-Patchwork-Id: 64120 Delivered-To: patch@linaro.org Received: by 10.112.199.169 with SMTP id jl9csp1544157lbc; Mon, 21 Mar 2016 10:39:11 -0700 (PDT) X-Received: by 10.140.230.207 with SMTP id a198mr44569409qhc.94.1458581951237; Mon, 21 Mar 2016 10:39:11 -0700 (PDT) Return-Path: Received: from lists.gnu.org (lists.gnu.org. [2001:4830:134:3::11]) by mx.google.com with ESMTPS id w206si11960192qhc.124.2016.03.21.10.39.11 for (version=TLS1 cipher=AES128-SHA bits=128/128); Mon, 21 Mar 2016 10:39:11 -0700 (PDT) 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]:59296 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ai3nK-0001aG-MJ for patch@linaro.org; Mon, 21 Mar 2016 13:39:10 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51856) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ai3e3-0002Dg-N3 for qemu-devel@nongnu.org; Mon, 21 Mar 2016 13:29:39 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ai3e2-00017u-83 for qemu-devel@nongnu.org; Mon, 21 Mar 2016 13:29:35 -0400 Received: from e32.co.us.ibm.com ([32.97.110.150]:60016) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ai3e2-00017a-2I for qemu-devel@nongnu.org; Mon, 21 Mar 2016 13:29:34 -0400 Received: from localhost by e32.co.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 21 Mar 2016 11:29:33 -0600 Received: from d03dlp02.boulder.ibm.com (9.17.202.178) by e32.co.us.ibm.com (192.168.1.132) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Mon, 21 Mar 2016 11:29:32 -0600 X-IBM-Helo: d03dlp02.boulder.ibm.com X-IBM-MailFrom: mdroth@linux.vnet.ibm.com X-IBM-RcptTo: qemu-devel@nongnu.org;qemu-stable@nongnu.org Received: from b01cxnp22034.gho.pok.ibm.com (b01cxnp22034.gho.pok.ibm.com [9.57.198.24]) by d03dlp02.boulder.ibm.com (Postfix) with ESMTP id 7D3C73E40030; Mon, 21 Mar 2016 11:29:30 -0600 (MDT) Received: from d01av04.pok.ibm.com (d01av04.pok.ibm.com [9.56.224.64]) by b01cxnp22034.gho.pok.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id u2LHTTUK19464340; Mon, 21 Mar 2016 17:29:29 GMT Received: from d01av04.pok.ibm.com (localhost [127.0.0.1]) by d01av04.pok.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id u2LHTTpj013057; Mon, 21 Mar 2016 13:29:29 -0400 Received: from localhost ([9.80.111.41]) by d01av04.pok.ibm.com (8.14.4/8.14.4/NCO v10.0 AVin) with ESMTP id u2LHTSaG013018; Mon, 21 Mar 2016 13:29:28 -0400 From: Michael Roth To: qemu-devel@nongnu.org Date: Mon, 21 Mar 2016 12:28:25 -0500 Message-Id: <1458581313-19045-28-git-send-email-mdroth@linux.vnet.ibm.com> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1458581313-19045-1-git-send-email-mdroth@linux.vnet.ibm.com> References: <1458581313-19045-1-git-send-email-mdroth@linux.vnet.ibm.com> X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 16032117-0005-0000-0000-00001F2D40A6 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 32.97.110.150 Cc: Petr Matousek , Michael Roth , Stefano Stabellini , Jason Wang , "Michael S. Tsirkin" , qemu-stable@nongnu.org, Prasad Pandit , Laszlo Ersek Subject: [Qemu-devel] [PATCH 27/35] e1000: eliminate infinite loops on out-of-bounds transfer start X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+patch=linaro.org@nongnu.org Sender: qemu-devel-bounces+patch=linaro.org@nongnu.org From: Laszlo Ersek The start_xmit() and e1000_receive_iov() functions implement DMA transfers iterating over a set of descriptors that the guest's e1000 driver prepares: - the TDLEN and RDLEN registers store the total size of the descriptor area, - while the TDH and RDH registers store the offset (in whole tx / rx descriptors) into the area where the transfer is supposed to start. Each time a descriptor is processed, the TDH and RDH register is bumped (as appropriate for the transfer direction). QEMU already contains logic to deal with bogus transfers submitted by the guest: - Normally, the transmit case wants to increase TDH from its initial value to TDT. (TDT is allowed to be numerically smaller than the initial TDH value; wrapping at or above TDLEN bytes to zero is normal.) The failsafe that QEMU currently has here is a check against reaching the original TDH value again -- a complete wraparound, which should never happen. - In the receive case RDH is increased from its initial value until "total_size" bytes have been received; preferably in a single step, or in "s->rxbuf_size" byte steps, if the latter is smaller. However, null RX descriptors are skipped without receiving data, while RDH is incremented just the same. QEMU tries to prevent an infinite loop (processing only null RX descriptors) by detecting whether RDH assumes its original value during the loop. (Again, wrapping from RDLEN to 0 is normal.) What both directions miss is that the guest could program TDLEN and RDLEN so low, and the initial TDH and RDH so high, that these registers will immediately be truncated to zero, and then never reassume their initial values in the loop -- a full wraparound will never occur. The condition that expresses this is: xdh_start >= s->mac_reg[XDLEN] / sizeof(desc) i.e., TDH or RDH start out after the last whole rx or tx descriptor that fits into the TDLEN or RDLEN sized area. This condition could be checked before we enter the loops, but pci_dma_read() / pci_dma_write() knows how to fill in buffers safely for bogus DMA addresses, so we just extend the existing failsafes with the above condition. This is CVE-2016-1981. Cc: "Michael S. Tsirkin" Cc: Petr Matousek Cc: Stefano Stabellini Cc: Prasad Pandit Cc: Michael Roth Cc: Jason Wang Cc: qemu-stable@nongnu.org RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1296044 Signed-off-by: Laszlo Ersek Reviewed-by: Jason Wang Signed-off-by: Jason Wang (cherry picked from commit dd793a74882477ca38d49e191110c17dfee51dcc) Signed-off-by: Michael Roth --- hw/net/e1000.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) -- 1.9.1 diff --git a/hw/net/e1000.c b/hw/net/e1000.c index bec06e9..34d0823 100644 --- a/hw/net/e1000.c +++ b/hw/net/e1000.c @@ -908,7 +908,8 @@ start_xmit(E1000State *s) * bogus values to TDT/TDLEN. * there's nothing too intelligent we could do about this. */ - if (s->mac_reg[TDH] == tdh_start) { + if (s->mac_reg[TDH] == tdh_start || + tdh_start >= s->mac_reg[TDLEN] / sizeof(desc)) { DBGOUT(TXERR, "TDH wraparound @%x, TDT %x, TDLEN %x\n", tdh_start, s->mac_reg[TDT], s->mac_reg[TDLEN]); break; @@ -1165,7 +1166,8 @@ e1000_receive_iov(NetClientState *nc, const struct iovec *iov, int iovcnt) if (++s->mac_reg[RDH] * sizeof(desc) >= s->mac_reg[RDLEN]) s->mac_reg[RDH] = 0; /* see comment in start_xmit; same here */ - if (s->mac_reg[RDH] == rdh_start) { + if (s->mac_reg[RDH] == rdh_start || + rdh_start >= s->mac_reg[RDLEN] / sizeof(desc)) { DBGOUT(RXERR, "RDH wraparound @%x, RDT %x, RDLEN %x\n", rdh_start, s->mac_reg[RDT], s->mac_reg[RDLEN]); set_ics(s, 0, E1000_ICS_RXO);