- bsc#963783 - VUL-1: CVE-2016-1981: xen: net: e1000 infinite loop
in start_xmit and e1000_receive_iov routines CVE-2016-1981-qemuu-e1000-eliminate-infinite-loops-on-out-of-bounds-transfer.patch CVE-2016-1981-qemut-e1000-eliminate-infinite-loops-on-out-of-bounds-transfer.patch OBS-URL: https://build.opensuse.org/package/show/Virtualization/xen?expand=0&rev=396
This commit is contained in:
Charles Arnold
Git OBS Bridge
parent
39134eb9d2
commit
fbfd58d3a2
@ -0,0 +1,94 @@
|
|||||||
|
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.
|
||||||
|
|
||||||
|
Cc: "Michael S. Tsirkin" <address@hidden>
|
||||||
|
Cc: Petr Matousek <address@hidden>
|
||||||
|
Cc: Stefano Stabellini <address@hidden>
|
||||||
|
Cc: Prasad Pandit <address@hidden>
|
||||||
|
Cc: Michael Roth <address@hidden>
|
||||||
|
Cc: Jason Wang <address@hidden>
|
||||||
|
RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1296044
|
||||||
|
Signed-off-by: Laszlo Ersek <address@hidden>
|
||||||
|
Reviewed-by: Jason Wang <address@hidden>
|
||||||
|
---
|
||||||
|
|
||||||
|
Notes:
|
||||||
|
Regarding the public posting: we made an honest effort to vet this
|
||||||
|
vulnerability, and the impact seems low -- no host side reads/writes,
|
||||||
|
"just" a DoS (infinite loop). We decided the patch could be posted
|
||||||
|
publicly, for the usual review process. Jason and Prasad checked the
|
||||||
|
patch in the internal discussion already, but comments, improvements
|
||||||
|
etc. are clearly welcome. The CVE request is underway. Thanks.
|
||||||
|
|
||||||
|
hw/net/e1000.c | 6 ++++--
|
||||||
|
1 file changed, 4 insertions(+), 2 deletions(-)
|
||||||
|
|
||||||
|
Index: xen-4.6.0-testing/tools/qemu-xen-traditional-dir-remote/hw/e1000.c
|
||||||
|
===================================================================
|
||||||
|
--- xen-4.6.0-testing.orig/tools/qemu-xen-traditional-dir-remote/hw/e1000.c
|
||||||
|
+++ xen-4.6.0-testing/tools/qemu-xen-traditional-dir-remote/hw/e1000.c
|
||||||
|
@@ -537,7 +537,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;
|
||||||
|
@@ -727,7 +728,8 @@ e1000_receive(void *opaque, const uint8_
|
||||||
|
s->mac_reg[RDH] = 0;
|
||||||
|
s->check_rxov = 1;
|
||||||
|
/* 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);
|
||||||
@ -0,0 +1,94 @@
|
|||||||
|
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.
|
||||||
|
|
||||||
|
Cc: "Michael S. Tsirkin" <address@hidden>
|
||||||
|
Cc: Petr Matousek <address@hidden>
|
||||||
|
Cc: Stefano Stabellini <address@hidden>
|
||||||
|
Cc: Prasad Pandit <address@hidden>
|
||||||
|
Cc: Michael Roth <address@hidden>
|
||||||
|
Cc: Jason Wang <address@hidden>
|
||||||
|
RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1296044
|
||||||
|
Signed-off-by: Laszlo Ersek <address@hidden>
|
||||||
|
Reviewed-by: Jason Wang <address@hidden>
|
||||||
|
---
|
||||||
|
|
||||||
|
Notes:
|
||||||
|
Regarding the public posting: we made an honest effort to vet this
|
||||||
|
vulnerability, and the impact seems low -- no host side reads/writes,
|
||||||
|
"just" a DoS (infinite loop). We decided the patch could be posted
|
||||||
|
publicly, for the usual review process. Jason and Prasad checked the
|
||||||
|
patch in the internal discussion already, but comments, improvements
|
||||||
|
etc. are clearly welcome. The CVE request is underway. Thanks.
|
||||||
|
|
||||||
|
hw/net/e1000.c | 6 ++++--
|
||||||
|
1 file changed, 4 insertions(+), 2 deletions(-)
|
||||||
|
|
||||||
|
Index: xen-4.6.0-testing/tools/qemu-xen-dir-remote/hw/net/e1000.c
|
||||||
|
===================================================================
|
||||||
|
--- xen-4.6.0-testing.orig/tools/qemu-xen-dir-remote/hw/net/e1000.c
|
||||||
|
+++ xen-4.6.0-testing/tools/qemu-xen-dir-remote/hw/net/e1000.c
|
||||||
|
@@ -815,7 +815,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;
|
||||||
|
@@ -1059,7 +1060,8 @@ e1000_receive_iov(NetClientState *nc, co
|
||||||
|
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);
|
||||||
@ -1,3 +1,11 @@
|
|||||||
|
-------------------------------------------------------------------
|
||||||
|
Wed Jan 27 08:23:26 MST 2016 - carnold@suse.com
|
||||||
|
|
||||||
|
- bsc#963783 - VUL-1: CVE-2016-1981: xen: net: e1000 infinite loop
|
||||||
|
in start_xmit and e1000_receive_iov routines
|
||||||
|
CVE-2016-1981-qemuu-e1000-eliminate-infinite-loops-on-out-of-bounds-transfer.patch
|
||||||
|
CVE-2016-1981-qemut-e1000-eliminate-infinite-loops-on-out-of-bounds-transfer.patch
|
||||||
|
|
||||||
-------------------------------------------------------------------
|
-------------------------------------------------------------------
|
||||||
Wed Jan 20 08:21:42 MST 2016 - carnold@suse.com
|
Wed Jan 20 08:21:42 MST 2016 - carnold@suse.com
|
||||||
|
|
||||||
|
|||||||
7
xen.spec
7
xen.spec
@ -1,7 +1,7 @@
|
|||||||
#
|
#
|
||||||
# spec file for package xen
|
# spec file for package xen
|
||||||
#
|
#
|
||||||
# Copyright (c) 2016 SUSE LINUX GmbH, Nuernberg, Germany.
|
# Copyright (c) 2016 SUSE LINUX Products GmbH, Nuernberg, Germany.
|
||||||
#
|
#
|
||||||
# All modifications and additions to the file contributed by third parties
|
# All modifications and additions to the file contributed by third parties
|
||||||
# remain the property of their copyright owners, unless otherwise agreed
|
# remain the property of their copyright owners, unless otherwise agreed
|
||||||
@ -15,7 +15,6 @@
|
|||||||
# Please submit bugfixes or comments via http://bugs.opensuse.org/
|
# Please submit bugfixes or comments via http://bugs.opensuse.org/
|
||||||
#
|
#
|
||||||
|
|
||||||
|
|
||||||
# needssslcertforbuild
|
# needssslcertforbuild
|
||||||
|
|
||||||
Name: xen
|
Name: xen
|
||||||
@ -277,6 +276,8 @@ Patch281: CVE-2013-4537-qemut-ssi-sd-fix-buffer-overrun-on-invalid-state-l
|
|||||||
Patch282: CVE-2015-1779-qemuu-incrementally-decode-websocket-frames.patch
|
Patch282: CVE-2015-1779-qemuu-incrementally-decode-websocket-frames.patch
|
||||||
Patch283: CVE-2015-1779-qemuu-limit-size-of-HTTP-headers-from-websockets-clients.patch
|
Patch283: CVE-2015-1779-qemuu-limit-size-of-HTTP-headers-from-websockets-clients.patch
|
||||||
Patch284: CVE-2013-4539-qemut-tsc210x-fix-buffer-overrun-on-invalid-state-load.patch
|
Patch284: CVE-2013-4539-qemut-tsc210x-fix-buffer-overrun-on-invalid-state-load.patch
|
||||||
|
Patch285: CVE-2016-1981-qemuu-e1000-eliminate-infinite-loops-on-out-of-bounds-transfer.patch
|
||||||
|
Patch286: CVE-2016-1981-qemut-e1000-eliminate-infinite-loops-on-out-of-bounds-transfer.patch
|
||||||
# Our platform specific patches
|
# Our platform specific patches
|
||||||
Patch321: xen-destdir.patch
|
Patch321: xen-destdir.patch
|
||||||
Patch322: vif-bridge-no-iptables.patch
|
Patch322: vif-bridge-no-iptables.patch
|
||||||
@ -620,6 +621,8 @@ Authors:
|
|||||||
%patch282 -p1
|
%patch282 -p1
|
||||||
%patch283 -p1
|
%patch283 -p1
|
||||||
%patch284 -p1
|
%patch284 -p1
|
||||||
|
%patch285 -p1
|
||||||
|
%patch286 -p1
|
||||||
# Our platform specific patches
|
# Our platform specific patches
|
||||||
%patch321 -p1
|
%patch321 -p1
|
||||||
%patch322 -p1
|
%patch322 -p1
|
||||||
|
|||||||
Loading…
Reference in New Issue
Block a user