net: pad packets to minimum length in qemu_receive_packet()

[PATCH] net: pad packets to minimum length in qemu_receive_packet()

Posted by Peter Maydell 2 weeks, 3 days ago

In commits like 969e50b61a28 ("net: Pad short frames to minimum size
before sending from SLiRP/TAP") we switched away from requiring
network devices to handle short frames to instead having the net core
code do the padding of short frames out to the ETH_ZLEN minimum size.
We then dropped the code for handling short frames from the network
devices in a series of commits like 140eae9c8f7 ("hw/net: e1000:
Remove the logic of padding short frames in the receive path").

This missed one route where the device's receive code can still see a
short frame: if the device is in loopback mode and it transmits a
short frame via the qemu_receive_packet() function, this will be fed
back into its own receive code without being padded.

Add the padding logic to qemu_receive_packet().

This fixes a buffer overrun which can be triggered in the
e1000_receive_iov() logic via the loopback code path.

Other devices that use qemu_receive_packet() to implement loopback
are cadence_gem, dp8393x, lan9118, msf2-emac, pcnet, rtl8139
and sungem.

Cc: qemu-stable@nongnu.org
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3043
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
I think this is the right fix, but I'm not very familiar
with the net internals...
---
 net/net.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/net/net.c b/net/net.c
index 27e0d278071..8aefdb3424f 100644
--- a/net/net.c
+++ b/net/net.c
@@ -775,10 +775,20 @@ ssize_t qemu_send_packet(NetClientState *nc, const uint8_t *buf, int size)
 
 ssize_t qemu_receive_packet(NetClientState *nc, const uint8_t *buf, int size)
 {
+    uint8_t min_pkt[ETH_ZLEN];
+    size_t min_pktsz = sizeof(min_pkt);
+
     if (!qemu_can_receive_packet(nc)) {
         return 0;
     }
 
+    if (net_peer_needs_padding(nc)) {
+        if (eth_pad_short_frame(min_pkt, &min_pktsz, buf, size)) {
+            buf = min_pkt;
+            size = min_pktsz;
+        }
+    }
+
     return qemu_net_queue_receive(nc->incoming_queue, buf, size);
 }
 
-- 
2.43.0

Re: [PATCH] net: pad packets to minimum length in qemu_receive_packet()

Posted by Philippe Mathieu-Daudé 1 week, 4 days ago

(Cc'ing Akihiko)

On 28/10/25 17:00, Peter Maydell wrote:
> In commits like 969e50b61a28 ("net: Pad short frames to minimum size
> before sending from SLiRP/TAP") we switched away from requiring
> network devices to handle short frames to instead having the net core
> code do the padding of short frames out to the ETH_ZLEN minimum size.
> We then dropped the code for handling short frames from the network
> devices in a series of commits like 140eae9c8f7 ("hw/net: e1000:
> Remove the logic of padding short frames in the receive path").
> 
> This missed one route where the device's receive code can still see a
> short frame: if the device is in loopback mode and it transmits a
> short frame via the qemu_receive_packet() function, this will be fed
> back into its own receive code without being padded.
> 
> Add the padding logic to qemu_receive_packet().
> 
> This fixes a buffer overrun which can be triggered in the
> e1000_receive_iov() logic via the loopback code path.
> 
> Other devices that use qemu_receive_packet() to implement loopback
> are cadence_gem, dp8393x, lan9118, msf2-emac, pcnet, rtl8139
> and sungem.
> 
> Cc: qemu-stable@nongnu.org
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3043
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> I think this is the right fix, but I'm not very familiar
> with the net internals...
> ---
>   net/net.c | 10 ++++++++++
>   1 file changed, 10 insertions(+)
> 
> diff --git a/net/net.c b/net/net.c
> index 27e0d278071..8aefdb3424f 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -775,10 +775,20 @@ ssize_t qemu_send_packet(NetClientState *nc, const uint8_t *buf, int size)
>   
>   ssize_t qemu_receive_packet(NetClientState *nc, const uint8_t *buf, int size)
>   {
> +    uint8_t min_pkt[ETH_ZLEN];
> +    size_t min_pktsz = sizeof(min_pkt);
> +
>       if (!qemu_can_receive_packet(nc)) {
>           return 0;
>       }
>   
> +    if (net_peer_needs_padding(nc)) {
> +        if (eth_pad_short_frame(min_pkt, &min_pktsz, buf, size)) {
> +            buf = min_pkt;
> +            size = min_pktsz;
> +        }
> +    }
> +
>       return qemu_net_queue_receive(nc->incoming_queue, buf, size);
>   }
>

Re: [PATCH] net: pad packets to minimum length in qemu_receive_packet()

Posted by Akihiko Odaki 1 week, 4 days ago

On 2025/11/03 20:36, Philippe Mathieu-Daudé wrote:
> (Cc'ing Akihiko)
> 
> On 28/10/25 17:00, Peter Maydell wrote:
>> In commits like 969e50b61a28 ("net: Pad short frames to minimum size
>> before sending from SLiRP/TAP") we switched away from requiring
>> network devices to handle short frames to instead having the net core
>> code do the padding of short frames out to the ETH_ZLEN minimum size.
>> We then dropped the code for handling short frames from the network
>> devices in a series of commits like 140eae9c8f7 ("hw/net: e1000:
>> Remove the logic of padding short frames in the receive path").
>>
>> This missed one route where the device's receive code can still see a
>> short frame: if the device is in loopback mode and it transmits a
>> short frame via the qemu_receive_packet() function, this will be fed
>> back into its own receive code without being padded.
>>
>> Add the padding logic to qemu_receive_packet().
>>
>> This fixes a buffer overrun which can be triggered in the
>> e1000_receive_iov() logic via the loopback code path.
>>
>> Other devices that use qemu_receive_packet() to implement loopback
>> are cadence_gem, dp8393x, lan9118, msf2-emac, pcnet, rtl8139
>> and sungem.
>>
>> Cc: qemu-stable@nongnu.org
>> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3043
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>

Re: [PATCH] net: pad packets to minimum length in qemu_receive_packet()

Posted by Philippe Mathieu-Daudé 1 week, 4 days ago

On 28/10/25 17:00, Peter Maydell wrote:
> In commits like 969e50b61a28 ("net: Pad short frames to minimum size
> before sending from SLiRP/TAP") we switched away from requiring
> network devices to handle short frames to instead having the net core
> code do the padding of short frames out to the ETH_ZLEN minimum size.
> We then dropped the code for handling short frames from the network
> devices in a series of commits like 140eae9c8f7 ("hw/net: e1000:
> Remove the logic of padding short frames in the receive path").
> 
> This missed one route where the device's receive code can still see a
> short frame: if the device is in loopback mode and it transmits a
> short frame via the qemu_receive_packet() function, this will be fed
> back into its own receive code without being padded.
> 
> Add the padding logic to qemu_receive_packet().
> 
> This fixes a buffer overrun which can be triggered in the
> e1000_receive_iov() logic via the loopback code path.
> 
> Other devices that use qemu_receive_packet() to implement loopback
> are cadence_gem, dp8393x, lan9118, msf2-emac, pcnet, rtl8139
> and sungem.
> 
> Cc: qemu-stable@nongnu.org
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3043
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> I think this is the right fix, but I'm not very familiar
> with the net internals...
> ---
>   net/net.c | 10 ++++++++++
>   1 file changed, 10 insertions(+)
> 
> diff --git a/net/net.c b/net/net.c
> index 27e0d278071..8aefdb3424f 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -775,10 +775,20 @@ ssize_t qemu_send_packet(NetClientState *nc, const uint8_t *buf, int size)
>   
>   ssize_t qemu_receive_packet(NetClientState *nc, const uint8_t *buf, int size)
>   {
> +    uint8_t min_pkt[ETH_ZLEN];
> +    size_t min_pktsz = sizeof(min_pkt);
> +
>       if (!qemu_can_receive_packet(nc)) {
>           return 0;
>       }
>   
> +    if (net_peer_needs_padding(nc)) {
> +        if (eth_pad_short_frame(min_pkt, &min_pktsz, buf, size)) {
> +            buf = min_pkt;
> +            size = min_pktsz;
> +        }
> +    }
> +
>       return qemu_net_queue_receive(nc->incoming_queue, buf, size);
>   }

Nitpicking, variables scope can be reduced:

-- >8 --
@@ -777,5 +777,2 @@ ssize_t qemu_receive_packet(NetClientState *nc, 
const uint8_t *buf, int size)
  {
-    uint8_t min_pkt[ETH_ZLEN];
-    size_t min_pktsz = sizeof(min_pkt);
-
      if (!qemu_can_receive_packet(nc)) {
@@ -785,2 +782,5 @@ ssize_t qemu_receive_packet(NetClientState *nc, 
const uint8_t *buf, int size)
      if (net_peer_needs_padding(nc)) {
+        uint8_t min_pkt[ETH_ZLEN];
+        size_t min_pktsz = sizeof(min_pkt);
+
          if (eth_pad_short_frame(min_pkt, &min_pktsz, buf, size)) {
---

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH] net: pad packets to minimum length in qemu_receive_packet()

Posted by Peter Maydell 1 week, 4 days ago

On Mon, 3 Nov 2025 at 11:35, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> On 28/10/25 17:00, Peter Maydell wrote:
> > In commits like 969e50b61a28 ("net: Pad short frames to minimum size
> > before sending from SLiRP/TAP") we switched away from requiring
> > network devices to handle short frames to instead having the net core
> > code do the padding of short frames out to the ETH_ZLEN minimum size.
> > We then dropped the code for handling short frames from the network
> > devices in a series of commits like 140eae9c8f7 ("hw/net: e1000:
> > Remove the logic of padding short frames in the receive path").
> >
> > This missed one route where the device's receive code can still see a
> > short frame: if the device is in loopback mode and it transmits a
> > short frame via the qemu_receive_packet() function, this will be fed
> > back into its own receive code without being padded.
> >
> > Add the padding logic to qemu_receive_packet().
> >
> > This fixes a buffer overrun which can be triggered in the
> > e1000_receive_iov() logic via the loopback code path.
> >
> > Other devices that use qemu_receive_packet() to implement loopback
> > are cadence_gem, dp8393x, lan9118, msf2-emac, pcnet, rtl8139
> > and sungem.
> >
> > Cc: qemu-stable@nongnu.org
> > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3043
> > Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> > ---
> > I think this is the right fix, but I'm not very familiar
> > with the net internals...
> > ---
> >   net/net.c | 10 ++++++++++
> >   1 file changed, 10 insertions(+)
> >
> > diff --git a/net/net.c b/net/net.c
> > index 27e0d278071..8aefdb3424f 100644
> > --- a/net/net.c
> > +++ b/net/net.c
> > @@ -775,10 +775,20 @@ ssize_t qemu_send_packet(NetClientState *nc, const uint8_t *buf, int size)
> >
> >   ssize_t qemu_receive_packet(NetClientState *nc, const uint8_t *buf, int size)
> >   {
> > +    uint8_t min_pkt[ETH_ZLEN];
> > +    size_t min_pktsz = sizeof(min_pkt);
> > +
> >       if (!qemu_can_receive_packet(nc)) {
> >           return 0;
> >       }
> >
> > +    if (net_peer_needs_padding(nc)) {
> > +        if (eth_pad_short_frame(min_pkt, &min_pktsz, buf, size)) {
> > +            buf = min_pkt;
> > +            size = min_pktsz;
> > +        }
> > +    }
> > +
> >       return qemu_net_queue_receive(nc->incoming_queue, buf, size);
> >   }
>
> Nitpicking, variables scope can be reduced:

We can't move min_pkt to inside the if() because that
array has to stay in scope until the call to
qemu_net_queue_receive() returns (we have a live
pointer into it).

thanks
-- PMM

Re: [PATCH] net: pad packets to minimum length in qemu_receive_packet()

Posted by Daniel P. Berrangé 1 week, 4 days ago

On Mon, Nov 03, 2025 at 12:35:48PM +0100, Philippe Mathieu-Daudé wrote:
> On 28/10/25 17:00, Peter Maydell wrote:
> > In commits like 969e50b61a28 ("net: Pad short frames to minimum size
> > before sending from SLiRP/TAP") we switched away from requiring
> > network devices to handle short frames to instead having the net core
> > code do the padding of short frames out to the ETH_ZLEN minimum size.
> > We then dropped the code for handling short frames from the network
> > devices in a series of commits like 140eae9c8f7 ("hw/net: e1000:
> > Remove the logic of padding short frames in the receive path").
> > 
> > This missed one route where the device's receive code can still see a
> > short frame: if the device is in loopback mode and it transmits a
> > short frame via the qemu_receive_packet() function, this will be fed
> > back into its own receive code without being padded.
> > 
> > Add the padding logic to qemu_receive_packet().
> > 
> > This fixes a buffer overrun which can be triggered in the
> > e1000_receive_iov() logic via the loopback code path.
> > 
> > Other devices that use qemu_receive_packet() to implement loopback
> > are cadence_gem, dp8393x, lan9118, msf2-emac, pcnet, rtl8139
> > and sungem.
> > 
> > Cc: qemu-stable@nongnu.org
> > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3043
> > Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> > ---
> > I think this is the right fix, but I'm not very familiar
> > with the net internals...
> > ---
> >   net/net.c | 10 ++++++++++
> >   1 file changed, 10 insertions(+)
> > 
> > diff --git a/net/net.c b/net/net.c
> > index 27e0d278071..8aefdb3424f 100644
> > --- a/net/net.c
> > +++ b/net/net.c
> > @@ -775,10 +775,20 @@ ssize_t qemu_send_packet(NetClientState *nc, const uint8_t *buf, int size)
> >   ssize_t qemu_receive_packet(NetClientState *nc, const uint8_t *buf, int size)
> >   {
> > +    uint8_t min_pkt[ETH_ZLEN];
> > +    size_t min_pktsz = sizeof(min_pkt);
> > +
> >       if (!qemu_can_receive_packet(nc)) {
> >           return 0;
> >       }
> > +    if (net_peer_needs_padding(nc)) {
> > +        if (eth_pad_short_frame(min_pkt, &min_pktsz, buf, size)) {
> > +            buf = min_pkt;
> > +            size = min_pktsz;
> > +        }
> > +    }
> > +
> >       return qemu_net_queue_receive(nc->incoming_queue, buf, size);
> >   }
> 
> Nitpicking, variables scope can be reduced:
> 
> -- >8 --
> @@ -777,5 +777,2 @@ ssize_t qemu_receive_packet(NetClientState *nc, const
> uint8_t *buf, int size)
>  {
> -    uint8_t min_pkt[ETH_ZLEN];
> -    size_t min_pktsz = sizeof(min_pkt);
> -
>      if (!qemu_can_receive_packet(nc)) {
> @@ -785,2 +782,5 @@ ssize_t qemu_receive_packet(NetClientState *nc, const
> uint8_t *buf, int size)
>      if (net_peer_needs_padding(nc)) {
> +        uint8_t min_pkt[ETH_ZLEN];
> +        size_t min_pktsz = sizeof(min_pkt);
> +
>          if (eth_pad_short_frame(min_pkt, &min_pktsz, buf, size)) {
> ---

That isn't desirable, as then 'buf' would be holding a stack pointer
that has gone out of scope when qemu_net_queue_receive is called.

> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> 
> 

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH] net: pad packets to minimum length in qemu_receive_packet()

Posted by Daniel P. Berrangé 2 weeks, 2 days ago

On Tue, Oct 28, 2025 at 04:00:42PM +0000, Peter Maydell wrote:
> In commits like 969e50b61a28 ("net: Pad short frames to minimum size
> before sending from SLiRP/TAP") we switched away from requiring
> network devices to handle short frames to instead having the net core
> code do the padding of short frames out to the ETH_ZLEN minimum size.
> We then dropped the code for handling short frames from the network
> devices in a series of commits like 140eae9c8f7 ("hw/net: e1000:
> Remove the logic of padding short frames in the receive path").
> 
> This missed one route where the device's receive code can still see a
> short frame: if the device is in loopback mode and it transmits a
> short frame via the qemu_receive_packet() function, this will be fed
> back into its own receive code without being padded.
> 
> Add the padding logic to qemu_receive_packet().
> 
> This fixes a buffer overrun which can be triggered in the
> e1000_receive_iov() logic via the loopback code path.
> 
> Other devices that use qemu_receive_packet() to implement loopback
> are cadence_gem, dp8393x, lan9118, msf2-emac, pcnet, rtl8139
> and sungem.
> 
> Cc: qemu-stable@nongnu.org
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3043
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Since it is guest triggerable, this bug has been assigned
CVE-2025-12464

> ---
> I think this is the right fix, but I'm not very familiar
> with the net internals...
> ---
>  net/net.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|