[PATCH] hw/net/fsl_etsec: validate FCB offsets in process_tx_fcb()

Feifan Qian posted 1 patch 2 weeks ago
Failed in applying to current master (apply log)
hw/net/fsl_etsec/rings.c | 25 ++++++++++++++++++++-----
1 file changed, 20 insertions(+), 5 deletions(-)
[PATCH] hw/net/fsl_etsec: validate FCB offsets in process_tx_fcb()
Posted by Feifan Qian 2 weeks ago
The TX Frame Control Block (FCB) is prepended to a TX frame when
BD_TX_TOEUN is set. It contains two guest-controlled u8 offset
fields that process_tx_fcb() uses to locate L3/L4 headers within
the frame buffer:

  l3_header_offset = FCB byte 3 (0..255)
  l4_header_offset = FCB byte 2 (0..255)

These offsets are applied without any bounds check. When the
UDP-no-CTU branch is taken, the function writes zero to
l4_header[6] and l4_header[7]. With both offsets set to 0xFF the
write target is:

  tx_buffer + 8 + 255 + 255 + 6/7 = tx_buffer + 525

A malicious guest can therefore corrupt up to 509 bytes of heap
memory beyond a minimally-sized (16 B) TX frame.

Fix: reject the frame and log a guest error when the minimum
required buffer length

  8 (FCB) + l3_header_offset + l4_header_offset + 8

exceeds tx_buffer_len. Move the l3_header and l4_header pointer
declarations past the new guard so that out-of-bounds pointers
are never materialised.

Cc: qemu-stable@nongnu.org
Signed-off-by: Feifan Qian <bea1e@proton.me>
---
 hw/net/fsl_etsec/rings.c | 25 ++++++++++++++++++++-----
 1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/hw/net/fsl_etsec/rings.c b/hw/net/fsl_etsec/rings.c
index 22660c32b8..e60303f52c 100644
--- a/hw/net/fsl_etsec/rings.c
+++ b/hw/net/fsl_etsec/rings.c
@@ -176,15 +176,30 @@ static void tx_padding_and_crc(eTSEC *etsec, uint32_t min_frame_len)
 static void process_tx_fcb(eTSEC *etsec)
 {
     uint8_t flags = (uint8_t)(*etsec->tx_buffer);
-    /* L3 header offset from start of frame */
+    /* L3 header offset from start of frame (FCB byte 3) */
     uint8_t l3_header_offset = (uint8_t)*(etsec->tx_buffer + 3);
-    /* L4 header offset from start of L3 header */
+    /* L4 header offset from start of L3 header (FCB byte 2) */
     uint8_t l4_header_offset = (uint8_t)*(etsec->tx_buffer + 2);
+    uint8_t *l3_header;
+    uint8_t *l4_header;
+    int csum = 0;
+
+    /*
+     * Validate FCB header offsets before pointer arithmetic. The highest
+     * byte accessed is l4_header[7], at offset
+     *   8 (FCB size) + l3_header_offset + l4_header_offset + 7
+     * from tx_buffer. Drop the frame if this exceeds the buffer length.
+     */
+    if (etsec->tx_buffer_len < 8u + l3_header_offset + l4_header_offset + 8u) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "eTSEC: FCB offsets exceed frame length, dropping\n");
+        return;
+    }
+
     /* L3 header */
-    uint8_t *l3_header = etsec->tx_buffer + 8 + l3_header_offset;
+    l3_header = etsec->tx_buffer + 8 + l3_header_offset;
     /* L4 header */
-    uint8_t *l4_header = l3_header + l4_header_offset;
-    int csum = 0;
+    l4_header = l3_header + l4_header_offset;

     /* if packet is IP4 and IP checksum is requested */
     if (flags & FCB_TX_IP && flags & FCB_TX_CIP) {
--
2.43.0