Alexander Bulekov reported a UAF bug related e1000e packets send. -->https://bugs.launchpad.net/qemu/+bug/1886362
This is because the guest trigger a e1000e packet send and set the data's address to e1000e's MMIO address. So when the e1000e do DMA it will write the MMIO again and trigger re-entrancy and finally causes this UAF. Paolo suggested to use a bottom half whenever MMIO is doing complicate things in here: -->https://lists.nongnu.org/archive/html/qemu-devel/2020-07/msg03342.html Reference here: 'The easiest solution is to delay processing of descriptors to a bottom half whenever MMIO is doing something complicated. This is also better for latency because it will free the vCPU thread more quickly and leave the work to the I/O thread.' This patch fixes this UAF. Signed-off-by: Li Qiang <liq...@163.com> --- hw/net/e1000e_core.c | 25 +++++++++++++++++-------- hw/net/e1000e_core.h | 2 ++ 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/hw/net/e1000e_core.c b/hw/net/e1000e_core.c index bcd186cac5..6165b04b68 100644 --- a/hw/net/e1000e_core.c +++ b/hw/net/e1000e_core.c @@ -2423,32 +2423,27 @@ e1000e_set_dbal(E1000ECore *core, int index, uint32_t val) static void e1000e_set_tctl(E1000ECore *core, int index, uint32_t val) { - E1000E_TxRing txr; core->mac[index] = val; if (core->mac[TARC0] & E1000_TARC_ENABLE) { - e1000e_tx_ring_init(core, &txr, 0); - e1000e_start_xmit(core, &txr); + qemu_bh_schedule(core->tx[0].tx_bh); } if (core->mac[TARC1] & E1000_TARC_ENABLE) { - e1000e_tx_ring_init(core, &txr, 1); - e1000e_start_xmit(core, &txr); + qemu_bh_schedule(core->tx[1].tx_bh); } } static void e1000e_set_tdt(E1000ECore *core, int index, uint32_t val) { - E1000E_TxRing txr; int qidx = e1000e_mq_queue_idx(TDT, index); uint32_t tarc_reg = (qidx == 0) ? TARC0 : TARC1; core->mac[index] = val & 0xffff; if (core->mac[tarc_reg] & E1000_TARC_ENABLE) { - e1000e_tx_ring_init(core, &txr, qidx); - e1000e_start_xmit(core, &txr); + qemu_bh_schedule(core->tx[qidx].tx_bh); } } @@ -3322,6 +3317,16 @@ e1000e_vm_state_change(void *opaque, int running, RunState state) } } +static void e1000e_core_tx_bh(void *opaque) +{ + struct e1000e_tx *tx = opaque; + E1000ECore *core = tx->core; + E1000E_TxRing txr; + + e1000e_tx_ring_init(core, &txr, tx - &core->tx[0]); + e1000e_start_xmit(core, &txr); +} + void e1000e_core_pci_realize(E1000ECore *core, const uint16_t *eeprom_templ, @@ -3340,6 +3345,8 @@ e1000e_core_pci_realize(E1000ECore *core, for (i = 0; i < E1000E_NUM_QUEUES; i++) { net_tx_pkt_init(&core->tx[i].tx_pkt, core->owner, E1000E_MAX_TX_FRAGS, core->has_vnet); + core->tx[i].core = core; + core->tx[i].tx_bh = qemu_bh_new(e1000e_core_tx_bh, &core->tx[i]); } net_rx_pkt_init(&core->rx_pkt, core->has_vnet); @@ -3367,6 +3374,8 @@ e1000e_core_pci_uninit(E1000ECore *core) for (i = 0; i < E1000E_NUM_QUEUES; i++) { net_tx_pkt_reset(core->tx[i].tx_pkt); net_tx_pkt_uninit(core->tx[i].tx_pkt); + qemu_bh_delete(core->tx[i].tx_bh); + core->tx[i].tx_bh = NULL; } net_rx_pkt_uninit(core->rx_pkt); diff --git a/hw/net/e1000e_core.h b/hw/net/e1000e_core.h index aee32f7e48..94ddc6afc2 100644 --- a/hw/net/e1000e_core.h +++ b/hw/net/e1000e_core.h @@ -77,6 +77,8 @@ struct E1000Core { unsigned char sum_needed; bool cptse; struct NetTxPkt *tx_pkt; + QEMUBH *tx_bh; + E1000ECore *core; } tx[E1000E_NUM_QUEUES]; struct NetRxPkt *rx_pkt; -- 2.17.1