dcavalca / rpms / qemu

Forked from rpms/qemu a year ago
Clone

Blame 0010-e1000-eliminate-infinite-loops-on-out-of-bounds-tran.patch

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