From 4c88744f84d2eedef65c540044cc426162bb13bc Mon Sep 17 00:00:00 2001 From: Wolfgang Bumiller Date: Fri, 22 Jan 2016 08:56:49 +0100 Subject: [PATCH] Fix CVE-2016-1981 e1000: eliminate infinite loops on out-of-bounds transfer start --- ...6-1981-e1000-eliminate-infinite-loop.patch | 98 +++++++++++++++++++ debian/patches/series | 1 + 2 files changed, 99 insertions(+) create mode 100644 debian/patches/CVE-2016-1981-e1000-eliminate-infinite-loop.patch diff --git a/debian/patches/CVE-2016-1981-e1000-eliminate-infinite-loop.patch b/debian/patches/CVE-2016-1981-e1000-eliminate-infinite-loop.patch new file mode 100644 index 0000000..baf86bf --- /dev/null +++ b/debian/patches/CVE-2016-1981-e1000-eliminate-infinite-loop.patch @@ -0,0 +1,98 @@ +From e55bfae32b6e3ea1e9a8a318e1b9e76acbcdd50b Mon Sep 17 00:00:00 2001 +From: Laszlo Ersek +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" +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 +--- + 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 41629b3..63b656a 100644 --- a/debian/patches/series +++ b/debian/patches/series @@ -53,3 +53,4 @@ CVE-2015-8745-vmxnet3-support-reading-imr-registers.patch CVE-2015-8619-hmp-sendkey-oob-fix.patch CVE-2016-1714-fw_cfg-add-check-to-validate-current-entry.patch CVE-2016-1922-i386-avoid-null-pointer-dereference.patch +CVE-2016-1981-e1000-eliminate-infinite-loop.patch -- 2.39.2