On Wed, 24 Jun 2026 at 09:41, Peter Maydell <[email protected]> wrote:
>
> On Wed, 24 Jun 2026 at 07:57, Bin Meng <[email protected]> wrote:
> > * qemu_receive_packet() bypasses the send path for device loopback.
> > Its current net_peer_needs_padding(nc) check would become wrong once
> > backends or hub ports set do_not_pad: the receiver there is nc itself,
> > not nc->peer.
>
> Thanks -- I'll check this bit.

I ran into this from a different angle today, and I think
qemu_receive_packet()'s check is wrong currently, not just
with this change. Although net_peer_needs_padding() will
almost always return "true" today, if the network device
has no peer (e.g. run with -net none or -nodefaults and no
other network backend connected to it) then we won't pad
packets we feed in for loopback, and so the bugs which
commit a01344d9d78 is trying to fix still occur. I'll send
a patch to make that change once I've tested it a bit.

I'm also looking at a variant of this involving vlan headers:
https://gitlab.com/qemu-project/qemu/-/work_items/3518

In commit 63b901bfd30 you removed the code from rtl8139.c
which handled padding out short frames in the device's
receive path, because we now do the padding at the other end.
However, in this case the padding code in the device was
also handling a case where there's an 802.1Q vlan tag: it makes
the buffer at least MIN_BUF_SIZE + VLAN_HLEN == 64 bytes, with
the comment "Include some tailroom in case a vlan tag is
later removed". Then later we use this to avoid the copy
from the buffer reading off the end, but now the copy can
read 4 bytes of data off the end of the packet into guest
memory.

How should we handle this? We could:
 - reinstate the rtl8139 padding code
 - make the generic padding code allow enough extra space
   for the 802.1Q vlan tag (so devices can assume that the
   frame is at least its minimum size even if there's a
   vlan tag)
 - have rtl8139 special case this

thanks
-- PMM

Reply via email to