From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id 62C5A5A004F for ; Fri, 26 Jul 2024 09:20:37 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202312; t=1721978433; bh=5cnKOA/bvPnv3K4/53ObVByWyRwSuYDbRutOciTNaZs=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=aY9gV+fB0uXrIg6qzs1FhYP96127Yexzw3kaZgRoDBQ1G93Hie3fCgveXvlD/wXm4 WIBDMHvUWQfCHHrKtWVhY/cn9+uFc7TnFSNLnQOLfe4kJrz60HQ9AnPMeHJjRSK5Bo bhLeHpV0eCZHrIR8uEeogtPF0PU/ZR60kH5iiYA+uLD4FQpRl2g/tmOtoTVGXM+gAf H7R6AWmlCin1AeGtkQIn3X5d1Nz14gjgQAuagaK8liS0ZNBluW9SaafQO4gGa+zlvn E9XmcoNarc9LVmwycjtViVlnC3Dbb3i0vAVA3X/GMgXKkfZCvFMtpXl/SCuxRp6uLg FRWl3q+iU/4qA== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4WVfKs0yCKz4x4c; Fri, 26 Jul 2024 17:20:33 +1000 (AEST) From: David Gibson To: passt-dev@passt.top, Stefano Brivio Subject: [PATCH 4/5] tap: Correctly handle frames of odd length Date: Fri, 26 Jul 2024 17:20:30 +1000 Message-ID: <20240726072031.3941305-5-david@gibson.dropbear.id.au> X-Mailer: git-send-email 2.45.2 In-Reply-To: <20240726072031.3941305-1-david@gibson.dropbear.id.au> References: <20240726072031.3941305-1-david@gibson.dropbear.id.au> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Message-ID-Hash: FAXVHGLW7WUH4FJ4LHS5QDCTXVKQHVVS X-Message-ID-Hash: FAXVHGLW7WUH4FJ4LHS5QDCTXVKQHVVS X-MailFrom: dgibson@gandalf.ozlabs.org X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: David Gibson X-Mailman-Version: 3.3.8 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: 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 --- 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 #include #include +#include #include #include +#include #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