On Thu, Nov 5, 2020 at 11:56 AM Mauro Matteo Cascella <[email protected]> wrote: > > The e1000e_write_packet_to_guest() function iterates over a set of > receive descriptors by advancing rx descriptor head register (RDH) from > its initial value to rx descriptor tail register (RDT). The check in > e1000e_ring_empty() is responsible for detecting whether RDH has reached > RDT, terminating the loop if that's the case. Additional checks have > been added in the past to deal with bogus values submitted by the guest > to prevent possible infinite loop. This is done by "wrapping around" RDH > at some point and detecting whether it assumes the original value during > the loop. > > However, when e1000e is configured to use the packet split feature, RDH is > incremented by two instead of one, as the packet split descriptors are > 32 bytes while regular descriptors are 16 bytes. A malicious or buggy > guest may set RDT to an odd value and transmit only null RX descriptors. > This corner case would prevent RDH from ever matching RDT, leading to an > infinite loop. This patch adds a check in e1000e_ring_advance() to make > sure RDH does never exceed RDT. > > This issue was independently reported by Gaoning Pan (Zhejiang University) > and Cheolwoo Myung. > > Signed-off-by: Mauro Matteo Cascella <[email protected]> > Reported-by: Gaoning Pan <[email protected]> > Reported-by: Cheolwoo Myung <[email protected]> > --- > References: > https://git.qemu.org/?p=qemu.git;a=commit;h=dd793a74882477ca38d49e191110c17dfe > https://git.qemu.org/?p=qemu.git;a=commit;h=4154c7e03fa55b4cf52509a83d50d6c09d743b7 > http://www.intel.com/content/dam/doc/datasheet/82574l-gbe-controller-datasheet.pdf > > hw/net/e1000e_core.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/hw/net/e1000e_core.c b/hw/net/e1000e_core.c > index bcd186cac5..4c4d14b6ed 100644 > --- a/hw/net/e1000e_core.c > +++ b/hw/net/e1000e_core.c > @@ -831,6 +831,10 @@ e1000e_ring_advance(E1000ECore *core, const > E1000E_RingInfo *r, uint32_t count) > { > core->mac[r->dh] += count; > > + if (core->mac[r->dh] > core->mac[r->dt]) { > + core->mac[r->dh] = core->mac[r->dt]; > + } > + > if (core->mac[r->dh] * E1000_RING_DESC_LEN >= core->mac[r->dlen]) { > core->mac[r->dh] = 0; > } > -- > 2.26.2 >
Here is a simple PoC to reproduce the aforementioned issue via qtest: cat << EOF | ./qemu-system-x86_64 -device e1000e -trace e1000e\* -display none -qtest stdio -accel qtest outl 0xcf8 0x80002010 outl 0xcfc 0xe0000000 outl 0xcf8 0x80002004 outw 0xcfc 0x7 write 0xe0003800 0x4 0x10001000 write 0xe0003808 0x4 0x00080000 write 0xe0000400 0x4 0xfa000400 write 0xe0002808 0x4 0x00080000 write 0xe0002818 0x4 0x7f000000 <== set RDT to 127 write 0xe0005008 0x4 0xfbffa3fa write 0xe0000100 0x4 0x5a840000 write 0x100018 0x2 0x6705 write 0x10001b 0x1 0x01 write 0xe0003818 0x4 0x04000000 EOF -- Mauro Matteo Cascella Red Hat Product Security PGP-Key ID: BB3410B0
