When inserting the VLAN tag in packets, memmove() can generate an
integer overflow for packets whose length is less than 12 bytes.
Check length against the size of the ethernet header (14 bytes) to
avoid the crash and return FTGMAC100_INT_XPKT_LOST status. This seems
like a good modeling choice even if Aspeed does not specify anything
in that case.
Cc: Frederic Konrad <konrad.frede...@yahoo.fr>
Cc: Mauro Matteo Cascella <mcasc...@redhat.com>
Reported-by: Ziming Zhang <ezrak...@gmail.com>
Signed-off-by: Cédric Le Goater <c...@kaod.org>
---
hw/net/ftgmac100.c | 19 +++++++++++++++----
1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/hw/net/ftgmac100.c b/hw/net/ftgmac100.c
index 280aa3d3a1e2..987b843fabc4 100644
--- a/hw/net/ftgmac100.c
+++ b/hw/net/ftgmac100.c
@@ -540,10 +540,21 @@ static void ftgmac100_do_tx(FTGMAC100State *s, uint32_t
tx_ring,
s->isr |= FTGMAC100_INT_XPKT_LOST;
len = sizeof(s->frame) - frame_size - 4;
}
- memmove(ptr + 16, ptr + 12, len - 12);
- stw_be_p(ptr + 12, ETH_P_VLAN);
- stw_be_p(ptr + 14, bd.des1);
- len += 4;
+
+ if (len < sizeof(struct eth_header)) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: frame too small for VLAN insertion : %d bytes\n",
+ __func__, len);
+ s->isr |= FTGMAC100_INT_XPKT_LOST;
+ } else {
+ uint8_t *vlan_hdr = ptr + (ETH_ALEN * 2);
+ uint8_t *payload = vlan_hdr + sizeof(struct vlan_header);
+
+ memmove(payload, vlan_hdr, len - (ETH_ALEN * 2));
+ stw_be_p(vlan_hdr, ETH_P_VLAN);
+ stw_be_p(vlan_hdr + 2, FTGMAC100_TXDES1_VLANTAG_CI(bd.des1));
+ len += sizeof(struct vlan_header);
+ }
}
ptr += len;
--
2.25.4
