qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Jason Wang <jasowang@redhat.com>
To: peter.maydell@linaro.org, qemu-devel@nongnu.org
Cc: Alexey Dobriyan <adobriyan@yandex-team.ru>,
	Jason Wang <jasowang@redhat.com>
Subject: [PULL 19/20] virtio-net: drop too short packets early
Date: Tue,  4 Jun 2024 15:37:54 +0800	[thread overview]
Message-ID: <20240604073755.1859-20-jasowang@redhat.com> (raw)
In-Reply-To: <20240604073755.1859-1-jasowang@redhat.com>

From: Alexey Dobriyan <adobriyan@yandex-team.ru>

Reproducer from https://gitlab.com/qemu-project/qemu/-/issues/1451
creates small packet (1 segment, len = 10 == n->guest_hdr_len),
then destroys queue.

"if (n->host_hdr_len != n->guest_hdr_len)" is triggered, if body creates
zero length/zero segment packet as there is nothing after guest header.

qemu_sendv_packet_async() tries to send it.

slirp discards it because it is smaller than Ethernet header,
but returns 0 because tx hooks are supposed to return total length of data.

0 is propagated upwards and is interpreted as "packet has been sent"
which is terrible because queue is being destroyed, nobody is waiting for TX
to complete and assert it triggered.

Fix is discard such empty packets instead of sending them.

Length 1 packets will go via different codepath:

	virtqueue_push(q->tx_vq, elem, 0);
	virtio_notify(vdev, q->tx_vq);
	g_free(elem);

and aren't problematic.

Signed-off-by: Alexey Dobriyan <adobriyan@yandex-team.ru>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hw/net/virtio-net.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 666a4e2a03..9c7e85caea 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -2708,18 +2708,14 @@ static int32_t virtio_net_flush_tx(VirtIONetQueue *q)
         out_sg = elem->out_sg;
         if (out_num < 1) {
             virtio_error(vdev, "virtio-net header not in first element");
-            virtqueue_detach_element(q->tx_vq, elem, 0);
-            g_free(elem);
-            return -EINVAL;
+            goto detach;
         }
 
         if (n->needs_vnet_hdr_swap) {
             if (iov_to_buf(out_sg, out_num, 0, &vhdr, sizeof(vhdr)) <
                 sizeof(vhdr)) {
                 virtio_error(vdev, "virtio-net header incorrect");
-                virtqueue_detach_element(q->tx_vq, elem, 0);
-                g_free(elem);
-                return -EINVAL;
+                goto detach;
             }
             virtio_net_hdr_swap(vdev, &vhdr);
             sg2[0].iov_base = &vhdr;
@@ -2747,6 +2743,11 @@ static int32_t virtio_net_flush_tx(VirtIONetQueue *q)
                              n->guest_hdr_len, -1);
             out_num = sg_num;
             out_sg = sg;
+
+            if (out_num < 1) {
+                virtio_error(vdev, "virtio-net nothing to send");
+                goto detach;
+            }
         }
 
         ret = qemu_sendv_packet_async(qemu_get_subqueue(n->nic, queue_index),
@@ -2767,6 +2768,11 @@ drop:
         }
     }
     return num_packets;
+
+detach:
+    virtqueue_detach_element(q->tx_vq, elem, 0);
+    g_free(elem);
+    return -EINVAL;
 }
 
 static void virtio_net_tx_timer(void *opaque);
-- 
2.42.0



  parent reply	other threads:[~2024-06-04  7:40 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-06-04  7:37 [PULL 00/20] Net patches Jason Wang
2024-06-04  7:37 ` [PULL 01/20] tap: Remove tap_probe_vnet_hdr_len() Jason Wang
2024-06-04  7:37 ` [PULL 02/20] tap: Remove qemu_using_vnet_hdr() Jason Wang
2024-06-04  7:37 ` [PULL 03/20] net: Move virtio-net header length assertion Jason Wang
2024-06-04  7:37 ` [PULL 04/20] net: Remove receive_raw() Jason Wang
2024-06-04  7:37 ` [PULL 05/20] tap: Call tap_receive_iov() from tap_receive() Jason Wang
2024-06-04  7:37 ` [PULL 06/20] tap: Shrink zeroed virtio-net header Jason Wang
2024-06-04  7:37 ` [PULL 07/20] virtio-net: Do not propagate ebpf-rss-fds errors Jason Wang
2024-06-05 10:23   ` Daniel P. Berrangé
2024-06-05 20:14     ` Akihiko Odaki
2024-06-06  7:14       ` Daniel P. Berrangé
2024-06-06  7:19         ` Akihiko Odaki
2024-06-06  7:59           ` Daniel P. Berrangé
2024-06-07  6:04             ` Akihiko Odaki
2024-06-04  7:37 ` [PULL 08/20] virtio-net: Add only one queue pair when realizing Jason Wang
2024-10-14  8:30   ` Laurent Vivier
2024-10-14 15:16     ` Laurent Vivier
2024-10-17  6:59       ` Jason Wang
2024-10-17  7:32         ` Laurent Vivier
2024-10-17  9:07           ` Akihiko Odaki
2024-10-17  9:17             ` Laurent Vivier
2024-10-17  9:42               ` Akihiko Odaki
2024-10-18  4:50                 ` Jason Wang
2024-10-19 12:38                   ` Akihiko Odaki
2024-10-21  7:23                     ` Jason Wang
2024-10-21  8:40                       ` Akihiko Odaki
2024-06-04  7:37 ` [PULL 09/20] virtio-net: Copy header only when necessary Jason Wang
2024-06-04  7:37 ` [PULL 10/20] virtio-net: Shrink header byte swapping buffer Jason Wang
2024-06-04  7:37 ` [PULL 11/20] virtio-net: Disable RSS on reset Jason Wang
2024-06-04  7:37 ` [PULL 12/20] virtio-net: Unify the logic to update NIC state for RSS Jason Wang
2024-06-04  7:37 ` [PULL 13/20] virtio-net: Always set populate_hash Jason Wang
2024-06-04  7:37 ` [PULL 14/20] virtio-net: Do not write hashes to peer buffer Jason Wang
2024-06-04  7:37 ` [PULL 15/20] ebpf: Fix RSS error handling Jason Wang
2024-06-04  7:37 ` [PULL 16/20] ebpf: Return 0 when configuration fails Jason Wang
2024-06-04  7:37 ` [PULL 17/20] ebpf: Refactor tun_rss_steering_prog() Jason Wang
2024-06-04  7:37 ` [PULL 18/20] ebpf: Add a separate target for skeleton Jason Wang
2024-06-04  7:37 ` Jason Wang [this message]
2024-06-04  7:37 ` [PULL 20/20] ebpf: Added traces back. Changed source set for eBPF to 'system' Jason Wang
2024-06-04 19:52 ` [PULL 00/20] Net patches Richard Henderson
2024-06-05 10:14 ` Michael Tokarev
2024-06-05 20:18   ` Akihiko Odaki
2024-06-06  0:13   ` Jason Wang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240604073755.1859-20-jasowang@redhat.com \
    --to=jasowang@redhat.com \
    --cc=adobriyan@yandex-team.ru \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).