Re: [PATCH 1/2] hw/net/rtl8139: Fix handling of VLAN tags on incoming short packets

On Fri, Jul 31, 2026 at 5:36 PM Peter Maydell <[email protected]> wrote:
>
> The rtl8139 receive code handles VLAN tags in incoming packets by
> copying the VLAN tag to a special field in the receive descriptor,
> and copying only the actual payload data to the receive buffer.  This
> code tries to ensure that it pads out the payload to at least
> MIN_BUF_SIZE bytes.
>
> In commit 63b901bfd30 we removed the main "pad short frames" code
> from this device because we switched to requiring net backends to do
> the padding.  However we didn't notice that this broke the VLAN tag
> handling, which relied on the old code making the buffer at least
> MIN_BUF_SIZE + VLAN_HLEN bytes so that it could copy MIN_BUF_SIZE
> bytes into the receive buffer even after removing the VLAN tag.  The
> result is that the guest can make us read 4 bytes off the end of a
> buffer by feeding itself a suitable short packet in loopback mode.
>
> The old behaviour is actually not correct, because the IEEE802.1Q
> standard says that the minimum ethernet frame size remains 64 bytes
> including the 4 checksum bytes, and so when a tag is present the
> payload data only needs to be 56 bytes.  (A bridge implementation can
> choose to pad tagged frames out to 68 bytes, but it doesn't have to,
> and so all devices have to correctly handle incoming tagged frames
> that are 64 bytes long.)
>
> The RTL8139 datasheet isn't very communicative on this topic, but
> there's nothing that suggests it adds extra padding on receive that
> didn't exist in the incoming packet.
>
> Drop the last remnants of the padding handling from this device;
> this avoids overcopying into the guest when we receive a short
> VLAN tagged packet.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3518
> Signed-off-by: Peter Maydell <[email protected]>
> ---
> I had been looking at this from the POV of making the generic
> net code add extra padding when a VLAN tag is present, but I
> went and read the 802.1Q spec and I now think this is just a
> place where the rtl8139 code got it wrong.
> ---
>  hw/net/rtl8139.c | 5 -----
>  1 file changed, 5 deletions(-)
>

Reviewed-by: Bin Meng <[email protected]>