]>
Commit | Line | Data |
---|---|---|
64c66641 WB |
1 | From e55bfae32b6e3ea1e9a8a318e1b9e76acbcdd50b Mon Sep 17 00:00:00 2001 |
2 | From: Laszlo Ersek <lersek@redhat.com> | |
3 | Date: Tue, 19 Jan 2016 14:17:20 +0100 | |
4 | Subject: [PATCH] e1000: eliminate infinite loops on out-of-bounds transfer | |
5 | start | |
6 | ||
7 | The start_xmit() and e1000_receive_iov() functions implement DMA transfers | |
8 | iterating over a set of descriptors that the guest's e1000 driver | |
9 | prepares: | |
10 | ||
11 | - the TDLEN and RDLEN registers store the total size of the descriptor | |
12 | area, | |
13 | ||
14 | - while the TDH and RDH registers store the offset (in whole tx / rx | |
15 | descriptors) into the area where the transfer is supposed to start. | |
16 | ||
17 | Each time a descriptor is processed, the TDH and RDH register is bumped | |
18 | (as appropriate for the transfer direction). | |
19 | ||
20 | QEMU already contains logic to deal with bogus transfers submitted by the | |
21 | guest: | |
22 | ||
23 | - Normally, the transmit case wants to increase TDH from its initial value | |
24 | to TDT. (TDT is allowed to be numerically smaller than the initial TDH | |
25 | value; wrapping at or above TDLEN bytes to zero is normal.) The failsafe | |
26 | that QEMU currently has here is a check against reaching the original | |
27 | TDH value again -- a complete wraparound, which should never happen. | |
28 | ||
29 | - In the receive case RDH is increased from its initial value until | |
30 | "total_size" bytes have been received; preferably in a single step, or | |
31 | in "s->rxbuf_size" byte steps, if the latter is smaller. However, null | |
32 | RX descriptors are skipped without receiving data, while RDH is | |
33 | incremented just the same. QEMU tries to prevent an infinite loop | |
34 | (processing only null RX descriptors) by detecting whether RDH assumes | |
35 | its original value during the loop. (Again, wrapping from RDLEN to 0 is | |
36 | normal.) | |
37 | ||
38 | What both directions miss is that the guest could program TDLEN and RDLEN | |
39 | so low, and the initial TDH and RDH so high, that these registers will | |
40 | immediately be truncated to zero, and then never reassume their initial | |
41 | values in the loop -- a full wraparound will never occur. | |
42 | ||
43 | The condition that expresses this is: | |
44 | ||
45 | xdh_start >= s->mac_reg[XDLEN] / sizeof(desc) | |
46 | ||
47 | i.e., TDH or RDH start out after the last whole rx or tx descriptor that | |
48 | fits into the TDLEN or RDLEN sized area. | |
49 | ||
50 | This condition could be checked before we enter the loops, but | |
51 | pci_dma_read() / pci_dma_write() knows how to fill in buffers safely for | |
52 | bogus DMA addresses, so we just extend the existing failsafes with the | |
53 | above condition. | |
54 | ||
55 | This is CVE-2016-1981. | |
56 | ||
57 | Cc: "Michael S. Tsirkin" <mst@redhat.com> | |
58 | Cc: Petr Matousek <pmatouse@redhat.com> | |
59 | Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com> | |
60 | Cc: Prasad Pandit <ppandit@redhat.com> | |
61 | Cc: Michael Roth <mdroth@linux.vnet.ibm.com> | |
62 | Cc: Jason Wang <jasowang@redhat.com> | |
63 | Cc: qemu-stable@nongnu.org | |
64 | RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1296044 | |
65 | Signed-off-by: Laszlo Ersek <lersek@redhat.com> | |
66 | Reviewed-by: Jason Wang <jasowang@redhat.com> | |
67 | Signed-off-by: Jason Wang <jasowang@redhat.com> | |
68 | --- | |
69 | hw/net/e1000.c | 6 ++++-- | |
70 | 1 file changed, 4 insertions(+), 2 deletions(-) | |
71 | ||
72 | diff --git a/hw/net/e1000.c b/hw/net/e1000.c | |
73 | index bec06e9..34d0823 100644 | |
74 | --- a/hw/net/e1000.c | |
75 | +++ b/hw/net/e1000.c | |
76 | @@ -908,7 +908,8 @@ start_xmit(E1000State *s) | |
77 | * bogus values to TDT/TDLEN. | |
78 | * there's nothing too intelligent we could do about this. | |
79 | */ | |
80 | - if (s->mac_reg[TDH] == tdh_start) { | |
81 | + if (s->mac_reg[TDH] == tdh_start || | |
82 | + tdh_start >= s->mac_reg[TDLEN] / sizeof(desc)) { | |
83 | DBGOUT(TXERR, "TDH wraparound @%x, TDT %x, TDLEN %x\n", | |
84 | tdh_start, s->mac_reg[TDT], s->mac_reg[TDLEN]); | |
85 | break; | |
86 | @@ -1165,7 +1166,8 @@ e1000_receive_iov(NetClientState *nc, const struct iovec *iov, int iovcnt) | |
87 | if (++s->mac_reg[RDH] * sizeof(desc) >= s->mac_reg[RDLEN]) | |
88 | s->mac_reg[RDH] = 0; | |
89 | /* see comment in start_xmit; same here */ | |
90 | - if (s->mac_reg[RDH] == rdh_start) { | |
91 | + if (s->mac_reg[RDH] == rdh_start || | |
92 | + rdh_start >= s->mac_reg[RDLEN] / sizeof(desc)) { | |
93 | DBGOUT(RXERR, "RDH wraparound @%x, RDT %x, RDLEN %x\n", | |
94 | rdh_start, s->mac_reg[RDT], s->mac_reg[RDLEN]); | |
95 | set_ics(s, 0, E1000_ICS_RXO); | |
96 | -- | |
97 | 2.1.4 | |
98 |