[PATCH] net: Correct padding check in qemu_receive_packet()

Peter Maydell posted 1 patch 3 weeks, 5 days ago
Failed in applying to current master (apply log)
Maintainers: Jason Wang <jasowang@redhat.com>
include/net/net.h | 23 +++++++++++++++++++++++
net/net.c         |  2 +-
2 files changed, 24 insertions(+), 1 deletion(-)
[PATCH] net: Correct padding check in qemu_receive_packet()
Posted by Peter Maydell 3 weeks, 5 days ago
In qemu_receive_packet() we check to see if we should pad a short
packet.  This is doing the wrong test: because this function is used
when the device adds a packet to its own incoming queue (i.e.  for
loopback), we should be checking the NetClientState's own do_not_pad
flag, not that for its peer.

We didn't notice this earlier, because at the moment all the real
peers of a network device (i.e.  the network backends) do not set
do_not_pad, so net_peer_needs_padding() always returns true except in
the corner case where the network device has no peer at all.

The effect of this is that if a network device has no peer (e.g.
because QEMU was started with -net none or with -nodefaults) then we
can still let through the kind of "guest misprograms the network
device to loopback-transmit a short packet and then we mishandle it
in the receive path" bug like #3043 which commit a01344d9d78 was
trying to fix.

Since the distinction between "we should check nc->do_not_pad"
and "we should check nc->peer->do_not_pad" is a bit subtle, add
enough documentation commentary to make it more obvious.

Cc: qemu-stable@nongnu.org
Fixes: a01344d9d78 ("net: pad packets to minimum length in qemu_receive_packet()")
Suggested-by: Bin Meng <bmeng.cn@gmail.com>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/net/net.h | 23 +++++++++++++++++++++++
 net/net.c         |  2 +-
 2 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/include/net/net.h b/include/net/net.h
index 45bc86fc86..9edfacf827 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -349,9 +349,32 @@ uint32_t net_crc32_le(const uint8_t *p, int len);
     .offset     = vmstate_offset_macaddr(_state, _field),            \
 }
 
+/**
+ * net_peer_needs_padding: Should we pad as we send out packets?
+ * @nc: NetClientState
+ *
+ * Return true if the peer of this NetClientState (i.e. the
+ * destination that qemu_send_packet() etc send to) requires us to pad
+ * out packets that are shorter than the minimum ethernet frame
+ * length.
+ */
 static inline bool net_peer_needs_padding(NetClientState *nc)
 {
   return nc->peer && !nc->peer->do_not_pad;
 }
 
+/**
+ * net_client_needs_padding: Should we pad as we queue packets to ourselves?
+ * @nc: NetClientState
+ *
+ * Return true if this NetClientState requires us to pad out packets
+ * that are shorter than the minimum ethernet frame length.  This is
+ * the check to make in qemu_receive_packet() when we are queuing a
+ * packet back into ourselves (i.e. loopback).
+ */
+static inline bool net_client_needs_padding(NetClientState *nc)
+{
+    return !nc->do_not_pad;
+}
+
 #endif
diff --git a/net/net.c b/net/net.c
index 2892f1730d..93fe2ed1c3 100644
--- a/net/net.c
+++ b/net/net.c
@@ -783,7 +783,7 @@ ssize_t qemu_receive_packet(NetClientState *nc, const uint8_t *buf, int size)
         return 0;
     }
 
-    if (net_peer_needs_padding(nc)) {
+    if (net_client_needs_padding(nc)) {
         if (eth_pad_short_frame(min_pkt, &min_pktsz, buf, size)) {
             buf = min_pkt;
             size = min_pktsz;
-- 
2.43.0
Re: [PATCH] net: Correct padding check in qemu_receive_packet()
Posted by Philippe Mathieu-Daudé 5 days ago
On 29/6/26 18:42, Peter Maydell wrote:
> In qemu_receive_packet() we check to see if we should pad a short
> packet.  This is doing the wrong test: because this function is used
> when the device adds a packet to its own incoming queue (i.e.  for
> loopback), we should be checking the NetClientState's own do_not_pad
> flag, not that for its peer.
> 
> We didn't notice this earlier, because at the moment all the real
> peers of a network device (i.e.  the network backends) do not set
> do_not_pad, so net_peer_needs_padding() always returns true except in
> the corner case where the network device has no peer at all.
> 
> The effect of this is that if a network device has no peer (e.g.
> because QEMU was started with -net none or with -nodefaults) then we
> can still let through the kind of "guest misprograms the network
> device to loopback-transmit a short packet and then we mishandle it
> in the receive path" bug like #3043 which commit a01344d9d78 was
> trying to fix.
> 
> Since the distinction between "we should check nc->do_not_pad"
> and "we should check nc->peer->do_not_pad" is a bit subtle, add
> enough documentation commentary to make it more obvious.
> 
> Cc: qemu-stable@nongnu.org
> Fixes: a01344d9d78 ("net: pad packets to minimum length in qemu_receive_packet()")
> Suggested-by: Bin Meng <bmeng.cn@gmail.com>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   include/net/net.h | 23 +++++++++++++++++++++++
>   net/net.c         |  2 +-
>   2 files changed, 24 insertions(+), 1 deletion(-)

Patch queued, thanks.
Re: [PATCH] net: Correct padding check in qemu_receive_packet()
Posted by Bin Meng 5 days ago
On Tue, Jun 30, 2026 at 12:42 AM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> In qemu_receive_packet() we check to see if we should pad a short
> packet.  This is doing the wrong test: because this function is used
> when the device adds a packet to its own incoming queue (i.e.  for
> loopback), we should be checking the NetClientState's own do_not_pad
> flag, not that for its peer.
>
> We didn't notice this earlier, because at the moment all the real
> peers of a network device (i.e.  the network backends) do not set
> do_not_pad, so net_peer_needs_padding() always returns true except in
> the corner case where the network device has no peer at all.
>
> The effect of this is that if a network device has no peer (e.g.
> because QEMU was started with -net none or with -nodefaults) then we
> can still let through the kind of "guest misprograms the network
> device to loopback-transmit a short packet and then we mishandle it
> in the receive path" bug like #3043 which commit a01344d9d78 was
> trying to fix.
>
> Since the distinction between "we should check nc->do_not_pad"
> and "we should check nc->peer->do_not_pad" is a bit subtle, add
> enough documentation commentary to make it more obvious.
>
> Cc: qemu-stable@nongnu.org
> Fixes: a01344d9d78 ("net: pad packets to minimum length in qemu_receive_packet()")
> Suggested-by: Bin Meng <bmeng.cn@gmail.com>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  include/net/net.h | 23 +++++++++++++++++++++++
>  net/net.c         |  2 +-
>  2 files changed, 24 insertions(+), 1 deletion(-)
>

Reviewed-by: Bin Meng <bin.meng@processmission.com>