[Qemu-devel] [PATCH 1/2] Fix a race condition in E1000 device implementation:

[Dmitry Fleytman]

Device driver enables RX on shutdown after HW reset
in case device is configured for wake on LAN.
Although RX is enabled corresponding rings are not initialized
and descrptor addresses of RX ring are invalid.
If packet arrives with proper timing QEMU crashes due to invalid
guest memory access attempt or guest memory gets corrupted.

Reported-by: Chris Webb <address@hidden>
Reported-by: Richard Davies <address@hidden>
Signed-off-by: Dmitry Fleytman <address@hidden>
---
 hw/e1000.c | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/hw/e1000.c b/hw/e1000.c
index 63fee10..1e66ecf 100644
--- a/hw/e1000.c
+++ b/hw/e1000.c
@@ -93,6 +93,7 @@ typedef struct E1000State_st {
     uint32_t rxbuf_size;
     uint32_t rxbuf_min_shift;
     int check_rxov;
+    uint32_t rx_init_done;
     struct e1000_tx {
         unsigned char header[256];
         unsigned char vlan_header[4];
@@ -267,6 +268,7 @@ static void e1000_reset(void *opaque)
 {
     E1000State *d = opaque;
 
+    d->check_rxov = 1;
     qemu_del_timer(d->autoneg_timer);
     memset(d->phy_reg, 0, sizeof d->phy_reg);
     memmove(d->phy_reg, phy_reg_init, sizeof phy_reg_init);
@@ -291,6 +293,12 @@ static void
 set_rx_control(E1000State *s, int index, uint32_t val)
 {
     s->mac_reg[RCTL] = val;
+
+    if (!(s->mac_reg[RCTL] & E1000_RCTL_EN)) {
+        s->rx_init_done = 0;
+        s->check_rxov = 1;
+    }
+
     s->rxbuf_size = rxbufsize(val);
     s->rxbuf_min_shift = ((val / E1000_RCTL_RDMTS_QUAT) & 3) + 1;
     DBGOUT(RX, "RCTL: %d, mac_reg[RCTL] = 0x%x\n", s->mac_reg[RDT],
@@ -925,8 +933,18 @@ mac_writereg(E1000State *s, int index, uint32_t val)
 static void
 set_rdt(E1000State *s, int index, uint32_t val)
 {
-    s->check_rxov = 0;
     s->mac_reg[index] = val & 0xffff;
+
+    if (s->mac_reg[index] || s->rx_init_done) {
+        s->check_rxov = 0;
+        /* This is a fix for RX initialization race
+        *  present in e1000 driver on some kernels.
+        *  We consider RX enabled only after we've seen
+        *  at least one RX descriptor filled by guest.
+        */
+        s->rx_init_done = 1;
+    }
+
     if (e1000_has_rxbufs(s, 1)) {
         qemu_flush_queued_packets(&s->nic->nc);
     }
@@ -1102,6 +1120,7 @@ static const VMStateDescription vmstate_e1000 = {
         VMSTATE_UNUSED(4), /* Was mmio_base.  */
         VMSTATE_UINT32(rxbuf_size, E1000State),
         VMSTATE_UINT32(rxbuf_min_shift, E1000State),
+        VMSTATE_UINT32(rx_init_done, E1000State),
         VMSTATE_UINT32(eecd_state.val_in, E1000State),
         VMSTATE_UINT16(eecd_state.bitnum_in, E1000State),
         VMSTATE_UINT16(eecd_state.bitnum_out, E1000State),
@@ -1269,6 +1288,7 @@ static int pci_e1000_init(PCIDevice *pci_dev)
     add_boot_device_path(d->conf.bootindex, &pci_dev->qdev, "/address@hidden");
 
     d->autoneg_timer = qemu_new_timer_ms(vm_clock, e1000_autoneg_timer, d);
+    d->check_rxov = 1;
 
     return 0;
 }
-- 
1.7.11.4