public inbox for passt-dev@passt.top
 help / color / mirror / code / Atom feed
From: David Gibson <david@gibson.dropbear.id.au>
To: passt-dev@passt.top, Stefano Brivio <sbrivio@redhat.com>
Cc: David Gibson <david@gibson.dropbear.id.au>
Subject: [PATCH 4/5] tap: Correctly handle frames of odd length
Date: Fri, 26 Jul 2024 17:20:30 +1000	[thread overview]
Message-ID: <20240726072031.3941305-5-david@gibson.dropbear.id.au> (raw)
In-Reply-To: <20240726072031.3941305-1-david@gibson.dropbear.id.au>

The Qemu socket protocol consists of a 32-bit frame length in network (BE)
order, followed by the Ethernet frame itself.  As far as I can tell,
frames can be any length, with no particular alignment requirement.  This
means that although pkt_buf itself is aligned, if we have a frame of odd
length, frames after it will have their frame length at an unaligned
address.

Currently we load the frame length by just casting a char pointer to
(uint32_t *) and loading.  Some platforms will generate a fatal trap on
such an unaligned load.  Even if they don't casting an incorrectly aligned
pointer to (uint32_t *) is undefined behaviour, strictly speaking.

Introduce a new helper to safely load a possibly unaligned value here.  We
assume that the compiler is smart enough to optimize this into nothing on
platforms that provide performant unaligned loads.  If that turns out not
to be the case, we can look at improvements then.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 tap.c  |  2 +-
 util.h | 16 ++++++++++++++++
 2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/tap.c b/tap.c
index df1287ad..aca27bb9 100644
--- a/tap.c
+++ b/tap.c
@@ -1011,7 +1011,7 @@ void tap_handler_passt(struct ctx *c, uint32_t events,
 	}
 
 	while (n > (ssize_t)sizeof(uint32_t)) {
-		uint32_t l2len = ntohl(*(uint32_t *)p);
+		uint32_t l2len = ntohl_unaligned(p);
 
 		if (l2len < sizeof(struct ethhdr) || l2len > ETH_MAX_MTU) {
 			err("Invalid frame size from QEMU (corrupt stream?)");
diff --git a/util.h b/util.h
index 826614cf..5a038e29 100644
--- a/util.h
+++ b/util.h
@@ -10,8 +10,10 @@
 #include <stdarg.h>
 #include <stdbool.h>
 #include <stddef.h>
+#include <stdint.h>
 #include <string.h>
 #include <signal.h>
+#include <arpa/inet.h>
 
 #include "log.h"
 
@@ -116,6 +118,20 @@
 #define	htonl_constant(x)	(__bswap_constant_32(x))
 #endif
 
+/**
+ * ntohl_unaligned() - Read 32-bit BE value from a possibly unaligned address
+ * @p:		Pointer to the BE value in memory
+ *
+ * Returns: Host-order value of 32-bit BE quantity at @p
+ */
+static inline uint32_t ntohl_unaligned(const void *p)
+{
+	uint32_t val;
+
+	memcpy(&val, p, sizeof(val));
+	return ntohl(val);
+}
+
 #define NS_FN_STACK_SIZE	(RLIMIT_STACK_VAL * 1024 / 8)
 int do_clone(int (*fn)(void *), char *stack_area, size_t stack_size, int flags,
 	     void *arg);
-- 
@@ -10,8 +10,10 @@
 #include <stdarg.h>
 #include <stdbool.h>
 #include <stddef.h>
+#include <stdint.h>
 #include <string.h>
 #include <signal.h>
+#include <arpa/inet.h>
 
 #include "log.h"
 
@@ -116,6 +118,20 @@
 #define	htonl_constant(x)	(__bswap_constant_32(x))
 #endif
 
+/**
+ * ntohl_unaligned() - Read 32-bit BE value from a possibly unaligned address
+ * @p:		Pointer to the BE value in memory
+ *
+ * Returns: Host-order value of 32-bit BE quantity at @p
+ */
+static inline uint32_t ntohl_unaligned(const void *p)
+{
+	uint32_t val;
+
+	memcpy(&val, p, sizeof(val));
+	return ntohl(val);
+}
+
 #define NS_FN_STACK_SIZE	(RLIMIT_STACK_VAL * 1024 / 8)
 int do_clone(int (*fn)(void *), char *stack_area, size_t stack_size, int flags,
 	     void *arg);
-- 
2.45.2


  parent reply	other threads:[~2024-07-26  7:20 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-07-26  7:20 [PATCH 0/5] Fix assorted errors in the Qemu socket tap receive handler David Gibson
2024-07-26  7:20 ` [PATCH 1/5] tap: Better report errors receiving from QEMU socket David Gibson
2024-07-26 11:25   ` Stefano Brivio
2024-07-26 11:50     ` Stefano Brivio
2024-07-26 12:02     ` David Gibson
2024-07-26  7:20 ` [PATCH 2/5] tap: Don't attempt to carry on if we get a bad frame length from qemu David Gibson
2024-07-26 11:26   ` Stefano Brivio
2024-07-26  7:20 ` [PATCH 3/5] tap: Don't use EPOLLET on Qemu sockets David Gibson
2024-07-26  8:00   ` Stefano Brivio
2024-07-26 10:44     ` Stefano Brivio
2024-07-26 12:12     ` David Gibson
2024-07-26 13:25       ` Stefano Brivio
2024-07-29  1:15         ` David Gibson
2024-07-26  7:20 ` David Gibson [this message]
2024-07-26  7:20 ` [PATCH 5/5] tap: Improve handling of partially received frames on qemu socket David Gibson
2024-07-26 11:39   ` Stefano Brivio
2024-07-26 12:33     ` David Gibson
2024-07-26 13:19 ` [PATCH 0/5] Fix assorted errors in the Qemu socket tap receive handler 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=20240726072031.3941305-5-david@gibson.dropbear.id.au \
    --to=david@gibson.dropbear.id.au \
    --cc=passt-dev@passt.top \
    --cc=sbrivio@redhat.com \
    /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).