[pve-devel] [PATCH kvm] Fix CVE-2016-1981

[Wolfgang Bumiller]

e1000: eliminate infinite loops on out-of-bounds transfer start
---
 ...E-2016-1981-e1000-eliminate-infinite-loop.patch | 98 ++++++++++++++++++++++
 debian/patches/series                              |  1 +
 2 files changed, 99 insertions(+)
 create mode 100644 debian/patches/extra/CVE-2016-1981-e1000-eliminate-infinite-loop.patch

diff --git a/debian/patches/extra/CVE-2016-1981-e1000-eliminate-infinite-loop.patch b/debian/patches/extra/CVE-2016-1981-e1000-eliminate-infinite-loop.patch
new file mode 100644
index 0000000..baf86bf
--- /dev/null
+++ b/debian/patches/extra/CVE-2016-1981-e1000-eliminate-infinite-loop.patch
@@ -0,0 +1,98 @@
+From e55bfae32b6e3ea1e9a8a318e1b9e76acbcdd50b Mon Sep 17 00:00:00 2001
+From: Laszlo Ersek <lersek at redhat.com>
+Date: Tue, 19 Jan 2016 14:17:20 +0100
+Subject: [PATCH] e1000: eliminate infinite loops on out-of-bounds transfer
+ start
+
+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" <mst at redhat.com>
+Cc: Petr Matousek <pmatouse at redhat.com>
+Cc: Stefano Stabellini <stefano.stabellini at eu.citrix.com>
+Cc: Prasad Pandit <ppandit at redhat.com>
+Cc: Michael Roth <mdroth at linux.vnet.ibm.com>
+Cc: Jason Wang <jasowang at redhat.com>
+Cc: qemu-stable at nongnu.org
+RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1296044
+Signed-off-by: Laszlo Ersek <lersek at redhat.com>
+Reviewed-by: Jason Wang <jasowang at redhat.com>
+Signed-off-by: Jason Wang <jasowang at redhat.com>
+---
+ hw/net/e1000.c | 6 ++++--
+ 1 file changed, 4 insertions(+), 2 deletions(-)
+
+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);
+-- 
+2.1.4
+
diff --git a/debian/patches/series b/debian/patches/series
index 969dd41..240f054 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -47,3 +47,4 @@ extra/vmxnet3-host-memory-leakage.patch
 extra/CVE-2015-8619-hmp-sendkey-oob-fix.patch
 extra/0001-vnc-clear-vs-tlscreds-after-unparenting-it.patch
 extra/CVE-2016-1922-i386-avoid-null-pointer-dereference.patch
+extra/CVE-2016-1981-e1000-eliminate-infinite-loop.patch
-- 
2.1.4