[Xen-changelog] [qemu-xen stable-4.4] e1000: eliminate infinite loops on out-of-bounds transfer start

[patchbot@xxxxxxx]

commit e331500ea4e69ec8376b012d39aef2bcba84cb81
Author:     Laszlo Ersek <lersek@xxxxxxxxxx>
AuthorDate: Tue Jan 19 14:17:20 2016 +0100
Commit:     Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
CommitDate: Fri Feb 5 14:53:53 2016 +0000

    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.
    
    upstream-commit-id: dd793a74882477ca38d49e191110c17dfee51dcc
    
    Cc: "Michael S. Tsirkin" <mst@xxxxxxxxxx>
    Cc: Petr Matousek <pmatouse@xxxxxxxxxx>
    Cc: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
    Cc: Prasad Pandit <ppandit@xxxxxxxxxx>
    Cc: Michael Roth <mdroth@xxxxxxxxxxxxxxxxxx>
    Cc: Jason Wang <jasowang@xxxxxxxxxx>
    Cc: qemu-stable@xxxxxxxxxx
    RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1296044
    Signed-off-by: Laszlo Ersek <lersek@xxxxxxxxxx>
    Reviewed-by: Jason Wang <jasowang@xxxxxxxxxx>
    Signed-off-by: Jason Wang <jasowang@xxxxxxxxxx>
    Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
---
 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 c31bc01..937fce5 100644
--- a/hw/net/e1000.c
+++ b/hw/net/e1000.c
@@ -712,7 +712,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;
@@ -918,7 +919,8 @@ e1000_receive(NetClientState *nc, const uint8_t *buf, 
size_t size)
         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);
--
generated by git-patchbot for /home/xen/git/qemu-xen.git#stable-4.4

_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxx
http://lists.xensource.com/xen-changelog