On Mon, 29 Jun 2026 at 17:31, Peter Maydell <[email protected]> wrote:
> 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

Ping? How do you think we should handle the "short packet
with a vlan header" case? I think I lean towards making
the generic code deal with it.

I'm also still very unsure whether our network layer is
intended to distinguish "this is a short packet that we
padded" from "this is a packet that the guest padded".
Is a network driver/card model supposed to be able to
opt in to getting unpadded packets and then identify and
diagnose-as-error incoming short packets if it wants to?
That would mean we need to distinguish "we padded this
because the guest sent a short packet and we don't want
to deal with that in loopback or otherwise" from "we
padded this because it comes from a source that hands
us legitimate short packets that would have been padded
if they'd ever gone through a real network device".

thanks
-- PMM

Reply via email to