rx_desc_len is migrated as a raw uint8_t from the stream, but it
is a derived value that can be computed from the register state
in core.mac[RFCTL] and core.mac[RCTL]. A crafted migration stream
can set rx_desc_len to an invalid value (e.g. 64), causing a stack
buffer overflow in e1000e_write_packet_to_guest() which copies
rx_desc_len bytes into a 32-byte stack union.

Recalculate rx_desc_len and other derived values from the register
state in post_load, ignoring the untrusted values from the stream.

Cc: [email protected]
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3869
Signed-off-by: Laurent Vivier <[email protected]>
---
 hw/net/e1000e_core.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/hw/net/e1000e_core.c b/hw/net/e1000e_core.c
index 46e156a5ddc2..b87a9f167a71 100644
--- a/hw/net/e1000e_core.c
+++ b/hw/net/e1000e_core.c
@@ -1948,6 +1948,16 @@ e1000e_calc_rxdesclen(E1000ECore *core)
     trace_e1000e_rx_desc_len(core->rx_desc_len);
 }
 
+static void
+e1000e_calc_rxconf(E1000ECore *core)
+{
+    e1000e_parse_rxbufsize(core);
+    e1000e_calc_rxdesclen(core);
+    core->rxbuf_min_shift =
+        ((core->mac[RCTL] / E1000_RCTL_RDMTS_QUAT) & 3) + 1 +
+        E1000_RING_DESC_LEN_SHIFT;
+}
+
 static void
 e1000e_set_rx_control(E1000ECore *core, int index, uint32_t val)
 {
@@ -1955,11 +1965,7 @@ e1000e_set_rx_control(E1000ECore *core, int index, 
uint32_t val)
     trace_e1000e_rx_set_rctl(core->mac[RCTL]);
 
     if (val & E1000_RCTL_EN) {
-        e1000e_parse_rxbufsize(core);
-        e1000e_calc_rxdesclen(core);
-        core->rxbuf_min_shift = ((val / E1000_RCTL_RDMTS_QUAT) & 3) + 1 +
-                                E1000_RING_DESC_LEN_SHIFT;
-
+        e1000e_calc_rxconf(core);
         e1000e_start_recv(core);
     }
 }
@@ -3557,5 +3563,7 @@ e1000e_core_post_load(E1000ECore *core)
     e1000e_intrmgr_resume(core);
     e1000e_autoneg_resume(core);
 
+    e1000e_calc_rxconf(core);
+
     return 0;
 }
-- 
2.54.0


Reply via email to