From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:45788) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TNo2P-00024K-Oc for qemu-devel@nongnu.org; Mon, 15 Oct 2012 13:01:10 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TNo2I-0006Tw-3c for qemu-devel@nongnu.org; Mon, 15 Oct 2012 13:01:09 -0400 Received: from mail-wg0-f53.google.com ([74.125.82.53]:55849) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TNo2H-0006Tk-Tu for qemu-devel@nongnu.org; Mon, 15 Oct 2012 13:01:02 -0400 Received: by mail-wg0-f53.google.com with SMTP id dr1so3584022wgb.10 for ; Mon, 15 Oct 2012 10:01:01 -0700 (PDT) From: Dmitry Fleytman Date: Mon, 15 Oct 2012 18:48:52 +0200 Message-Id: <1350319733-7306-2-git-send-email-dmitry@daynix.com> In-Reply-To: <1350319733-7306-1-git-send-email-dmitry@daynix.com> References: <1350319733-7306-1-git-send-email-dmitry@daynix.com> Subject: [Qemu-devel] [PATCH 1/2] Fix a race condition in E1000 device implementation: List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: Yan Vugenfirer , Dmitry Fleytman , Chris Webb , Richard Davies 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 Reported-by: Richard Davies Signed-off-by: Dmitry Fleytman --- 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, "/ethernet-phy@0"); d->autoneg_timer = qemu_new_timer_ms(vm_clock, e1000_autoneg_timer, d); + d->check_rxov = 1; return 0; } -- 1.7.11.4