yeahuh / rpms / qemu-kvm

Forked from rpms/qemu-kvm 2 years ago
Clone

Blame SOURCES/kvm-e1000-eliminate-infinite-loops-on-out-of-bounds-tran.patch

9ae3a8
From 4fef3479339001ef3ea529fb0552533fae422240 Mon Sep 17 00:00:00 2001
9ae3a8
From: Laszlo Ersek <lersek@redhat.com>
9ae3a8
Date: Fri, 5 Feb 2016 14:26:18 +0100
9ae3a8
Subject: [PATCH 1/5] e1000: eliminate infinite loops on out-of-bounds transfer
9ae3a8
 start
9ae3a8
9ae3a8
RH-Author: Laszlo Ersek <lersek@redhat.com>
9ae3a8
Message-id: <1454682378-29144-2-git-send-email-lersek@redhat.com>
9ae3a8
Patchwork-id: 69116
9ae3a8
O-Subject: [RHEL-7.3 qemu-kvm PATCH 1/1] e1000: eliminate infinite loops on out-of-bounds transfer start
9ae3a8
Bugzilla: 1296044
9ae3a8
RH-Acked-by: Xiao Wang <jasowang@redhat.com>
9ae3a8
RH-Acked-by: P J P <ppandit@redhat.com>
9ae3a8
RH-Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
9ae3a8
9ae3a8
The start_xmit() and e1000_receive_iov() functions implement DMA transfers
9ae3a8
iterating over a set of descriptors that the guest's e1000 driver
9ae3a8
prepares:
9ae3a8
9ae3a8
- the TDLEN and RDLEN registers store the total size of the descriptor
9ae3a8
  area,
9ae3a8
9ae3a8
- while the TDH and RDH registers store the offset (in whole tx / rx
9ae3a8
  descriptors) into the area where the transfer is supposed to start.
9ae3a8
9ae3a8
Each time a descriptor is processed, the TDH and RDH register is bumped
9ae3a8
(as appropriate for the transfer direction).
9ae3a8
9ae3a8
QEMU already contains logic to deal with bogus transfers submitted by the
9ae3a8
guest:
9ae3a8
9ae3a8
- Normally, the transmit case wants to increase TDH from its initial value
9ae3a8
  to TDT. (TDT is allowed to be numerically smaller than the initial TDH
9ae3a8
  value; wrapping at or above TDLEN bytes to zero is normal.) The failsafe
9ae3a8
  that QEMU currently has here is a check against reaching the original
9ae3a8
  TDH value again -- a complete wraparound, which should never happen.
9ae3a8
9ae3a8
- In the receive case RDH is increased from its initial value until
9ae3a8
  "total_size" bytes have been received; preferably in a single step, or
9ae3a8
  in "s->rxbuf_size" byte steps, if the latter is smaller. However, null
9ae3a8
  RX descriptors are skipped without receiving data, while RDH is
9ae3a8
  incremented just the same. QEMU tries to prevent an infinite loop
9ae3a8
  (processing only null RX descriptors) by detecting whether RDH assumes
9ae3a8
  its original value during the loop. (Again, wrapping from RDLEN to 0 is
9ae3a8
  normal.)
9ae3a8
9ae3a8
What both directions miss is that the guest could program TDLEN and RDLEN
9ae3a8
so low, and the initial TDH and RDH so high, that these registers will
9ae3a8
immediately be truncated to zero, and then never reassume their initial
9ae3a8
values in the loop -- a full wraparound will never occur.
9ae3a8
9ae3a8
The condition that expresses this is:
9ae3a8
9ae3a8
  xdh_start >= s->mac_reg[XDLEN] / sizeof(desc)
9ae3a8
9ae3a8
i.e., TDH or RDH start out after the last whole rx or tx descriptor that
9ae3a8
fits into the TDLEN or RDLEN sized area.
9ae3a8
9ae3a8
This condition could be checked before we enter the loops, but
9ae3a8
pci_dma_read() / pci_dma_write() knows how to fill in buffers safely for
9ae3a8
bogus DMA addresses, so we just extend the existing failsafes with the
9ae3a8
above condition.
9ae3a8
9ae3a8
This is CVE-2016-1981.
9ae3a8
9ae3a8
Cc: "Michael S. Tsirkin" <mst@redhat.com>
9ae3a8
Cc: Petr Matousek <pmatouse@redhat.com>
9ae3a8
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
9ae3a8
Cc: Prasad Pandit <ppandit@redhat.com>
9ae3a8
Cc: Michael Roth <mdroth@linux.vnet.ibm.com>
9ae3a8
Cc: Jason Wang <jasowang@redhat.com>
9ae3a8
Cc: qemu-stable@nongnu.org
9ae3a8
RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1296044
9ae3a8
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
9ae3a8
Reviewed-by: Jason Wang <jasowang@redhat.com>
9ae3a8
Signed-off-by: Jason Wang <jasowang@redhat.com>
9ae3a8
(cherry picked from commit dd793a74882477ca38d49e191110c17dfee51dcc)
9ae3a8
Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
9ae3a8
---
9ae3a8
 hw/net/e1000.c | 6 ++++--
9ae3a8
 1 file changed, 4 insertions(+), 2 deletions(-)
9ae3a8
9ae3a8
diff --git a/hw/net/e1000.c b/hw/net/e1000.c
9ae3a8
index 87a84a7..2cd38bc 100644
9ae3a8
--- a/hw/net/e1000.c
9ae3a8
+++ b/hw/net/e1000.c
9ae3a8
@@ -697,7 +697,8 @@ start_xmit(E1000State *s)
9ae3a8
          * bogus values to TDT/TDLEN.
9ae3a8
          * there's nothing too intelligent we could do about this.
9ae3a8
          */
9ae3a8
-        if (s->mac_reg[TDH] == tdh_start) {
9ae3a8
+        if (s->mac_reg[TDH] == tdh_start ||
9ae3a8
+            tdh_start >= s->mac_reg[TDLEN] / sizeof(desc)) {
9ae3a8
             DBGOUT(TXERR, "TDH wraparound @%x, TDT %x, TDLEN %x\n",
9ae3a8
                    tdh_start, s->mac_reg[TDT], s->mac_reg[TDLEN]);
9ae3a8
             break;
9ae3a8
@@ -902,7 +903,8 @@ e1000_receive(NetClientState *nc, const uint8_t *buf, size_t size)
9ae3a8
         if (++s->mac_reg[RDH] * sizeof(desc) >= s->mac_reg[RDLEN])
9ae3a8
             s->mac_reg[RDH] = 0;
9ae3a8
         /* see comment in start_xmit; same here */
9ae3a8
-        if (s->mac_reg[RDH] == rdh_start) {
9ae3a8
+        if (s->mac_reg[RDH] == rdh_start ||
9ae3a8
+            rdh_start >= s->mac_reg[RDLEN] / sizeof(desc)) {
9ae3a8
             DBGOUT(RXERR, "RDH wraparound @%x, RDT %x, RDLEN %x\n",
9ae3a8
                    rdh_start, s->mac_reg[RDT], s->mac_reg[RDLEN]);
9ae3a8
             set_ics(s, 0, E1000_ICS_RXO);
9ae3a8
-- 
9ae3a8
1.8.3.1
9ae3a8