public inbox for passt-dev@passt.top
 help / color / mirror / code / Atom feed
From: Stefano Brivio <sbrivio@redhat.com>
To: passt-dev@passt.top
Subject: [PATCH 1/2] tap: Keep stream consistent if qemu length descriptor spans two recv() calls
Date: Tue,  8 Nov 2022 09:54:18 +0100	[thread overview]
Message-ID: <20221108085419.3220900-2-sbrivio@redhat.com> (raw)
In-Reply-To: <20221108085419.3220900-1-sbrivio@redhat.com>

I got all paranoid after triggering a divide-by-zero general
protection fault in passt with a qemu version without the virtio_net
TX hang fix, while flooding UDP. I start thinking this was actually
coming from some random changes I was playing with, but before
reaching this conclusion I reviewed once more the relatively short
path in tap_handler_passt() before we start using packet_*()
functions, and found this.

Never observed in practice, but artificially reproduced with changes
in qemu's socket interface: if we don't receive from qemu a complete
length descriptor in one recv() call, or if we receive a partial one
at the end of one call, we currently disregard the rest, which would
make the stream inconsistent.

Nothing really bad happens, except that from that point on we would
disregard all the packets we get until, if ever, we get the stream
back in sync by chance.

Force reading a complete packet length descriptor with a blocking
recv(), if needed -- not just a complete packet later.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
 tap.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/tap.c b/tap.c
index f8314ef..11ac732 100644
--- a/tap.c
+++ b/tap.c
@@ -747,14 +747,26 @@ redo:
 		return -ECONNRESET;
 	}
 
-	while (n > (ssize_t)sizeof(uint32_t)) {
-		ssize_t len = ntohl(*(uint32_t *)p);
+	while (n > 0) {
+		ssize_t len;
+
+		/* Force receiving at least a complete length descriptor to
+		 * avoid an inconsistent stream.
+		 */
+		if (n < (ssize_t)sizeof(uint32_t)) {
+			rem = recv(c->fd_tap, p + n,
+				   (ssize_t)sizeof(uint32_t) - n, 0);
+			if ((n += rem) != (ssize_t)sizeof(uint32_t))
+				return 0;
+		}
+
+		len = ntohl(*(uint32_t *)p);
 
 		p += sizeof(uint32_t);
 		n -= sizeof(uint32_t);
 
 		/* At most one packet might not fit in a single read, and this
-		 * needs to be blocking.
+		 * also needs to be blocking.
 		 */
 		if (len > n) {
 			rem = recv(c->fd_tap, p + n, len - n, 0);
-- 
@@ -747,14 +747,26 @@ redo:
 		return -ECONNRESET;
 	}
 
-	while (n > (ssize_t)sizeof(uint32_t)) {
-		ssize_t len = ntohl(*(uint32_t *)p);
+	while (n > 0) {
+		ssize_t len;
+
+		/* Force receiving at least a complete length descriptor to
+		 * avoid an inconsistent stream.
+		 */
+		if (n < (ssize_t)sizeof(uint32_t)) {
+			rem = recv(c->fd_tap, p + n,
+				   (ssize_t)sizeof(uint32_t) - n, 0);
+			if ((n += rem) != (ssize_t)sizeof(uint32_t))
+				return 0;
+		}
+
+		len = ntohl(*(uint32_t *)p);
 
 		p += sizeof(uint32_t);
 		n -= sizeof(uint32_t);
 
 		/* At most one packet might not fit in a single read, and this
-		 * needs to be blocking.
+		 * also needs to be blocking.
 		 */
 		if (len > n) {
 			rem = recv(c->fd_tap, p + n, len - n, 0);
-- 
2.35.1


  reply	other threads:[~2022-11-08  8:54 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-08  8:54 [PATCH 0/2] Try harder to avoid inconsistent qemu packet stream Stefano Brivio
2022-11-08  8:54 ` Stefano Brivio [this message]
2022-11-09 10:15   ` [PATCH 1/2] tap: Keep stream consistent if qemu length descriptor spans two recv() calls David Gibson
2022-11-09 10:29     ` Stefano Brivio
2022-11-10 12:59       ` Stefano Brivio
2022-11-08  8:54 ` [PATCH 2/2] tap: Return -EIO from tap_handler_passt() on inconsistent packet stream Stefano Brivio

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=20221108085419.3220900-2-sbrivio@redhat.com \
    --to=sbrivio@redhat.com \
    --cc=passt-dev@passt.top \
    /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.
Code repositories for project(s) associated with this public inbox

	https://passt.top/passt

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 IMAP folder(s).