Re: [PATCH 1/2] hw/net/rtl8139: Fix handling of VLAN tags on incoming short packets
Peter Maydell <[email protected]> Fri, 31 Jul 2026 14:24:36 +0100
| Newsgroups | org.nongnu.qemu-devel |
|---|---|
| Message-ID | <CAFEAcA9Vj79K0-CDnamk-uvFD3e2DNQvKSp-SoTUoPGLZ-5EMg@mail.gmail.com> |
On Fri, 31 Jul 2026 at 10:36, 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. > -- I forgot the Cc: [email protected] on this one. -- PMM