From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=none (p=none dis=none) header.from=gibson.dropbear.id.au Authentication-Results: passt.top; dkim=pass (2048-bit key; secure) header.d=gibson.dropbear.id.au header.i=@gibson.dropbear.id.au header.a=rsa-sha256 header.s=202502 header.b=T+8oASaX; dkim-atps=neutral Received: from mail.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id 0EC0F5A061A for ; Thu, 13 Mar 2025 06:40:57 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202502; t=1741844452; bh=14tY+03CKcutb2M72Q6yy34LQnBJxaGZ7HWFY412HWk=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=T+8oASaXn5ZJphTMRLZSIrJxS7tHRqpzTVyA8AE2++fhxbVAD0MT+z+5BkERYs1tU lYkDdXShUMfADRSvO653WA3ZIHAHXKmbMmuYfhat8g4n6iktcKAKXaj4O36Gd53z9p AoTs+COIgdtGAFyZWPxl/2lDLxxjvLg9U41oi5EUwjkoF+DX2Fki4d/KoZEBUilpER TttZNU/cpQXbEjGU/zVUfN0keikgPNXBwCaEcZT8OmWu0EBF/0S5JH4M10cAMLMxm8 6tQMJ8eGtVOexFDB3SrqJ7l535jyPG8n97Eq67XKrMJ2mL/BbOw5+d+v970HYrf/fC 3lZMpnXvn4b9w== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4ZCxDh4W25z4x5k; Thu, 13 Mar 2025 16:40:52 +1100 (AEDT) From: David Gibson To: passt-dev@passt.top, Stefano Brivio Subject: [PATCH 2/4] packet: More cautious checks to avoid pointer arithmetic UB Date: Thu, 13 Mar 2025 16:40:48 +1100 Message-ID: <20250313054050.642978-3-david@gibson.dropbear.id.au> X-Mailer: git-send-email 2.48.1 In-Reply-To: <20250313054050.642978-1-david@gibson.dropbear.id.au> References: <20250313054050.642978-1-david@gibson.dropbear.id.au> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Message-ID-Hash: 5ZJSQDF2IB6Z5XJTG5FA5Q6OQ2SQH3J3 X-Message-ID-Hash: 5ZJSQDF2IB6Z5XJTG5FA5Q6OQ2SQH3J3 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: packet_check_range and vu_packet_check_range() verify that the packet or section of packet we're interested in lies in the packet buffer pool we expect it to. However, in doing so it doesn't avoid the possibility of an integer overflow while performing pointer arithmetic, with is UB. In fact, AFAICT it's UB even to use arbitrary pointer arithmetic to construct a pointer outside of a known valid buffer. To do this safely, we can't calculate the end of a memory region with pointer addition when then the length as untrusted. Instead we must work out the offset of one memory region within another using pointer subtraction, then do integer checks against the length of the outer region. We then need to be careful about the order of checks so that those integer checks can't themselves overflow. Signed-off-by: David Gibson --- packet.c | 12 +++++++++--- vu_common.c | 10 +++++++--- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/packet.c b/packet.c index bcac0375..d1a51a5b 100644 --- a/packet.c +++ b/packet.c @@ -52,9 +52,15 @@ static int packet_check_range(const struct pool *p, const char *ptr, size_t len, return -1; } - if (ptr + len > p->buf + p->buf_size) { - trace("packet range end %p after buffer end %p, %s:%i", - (void *)(ptr + len), (void *)(p->buf + p->buf_size), + if (len > p->buf_size) { + trace("packet range length %zu larger than buffer %zu, %s:%i", + len, p->buf_size, func, line); + return -1; + } + + if ((size_t)(ptr - p->buf) > p->buf_size - len) { + trace("packet range %p, len %zu after buffer end %p, %s:%i", + (void *)ptr, len, (void *)(p->buf + p->buf_size), func, line); return -1; } diff --git a/vu_common.c b/vu_common.c index 9eea4f2f..cefe5e20 100644 --- a/vu_common.c +++ b/vu_common.c @@ -36,11 +36,15 @@ int vu_packet_check_range(void *buf, const char *ptr, size_t len) struct vu_dev_region *dev_region; for (dev_region = buf; dev_region->mmap_addr; dev_region++) { - /* NOLINTNEXTLINE(performance-no-int-to-ptr) */ - char *m = (char *)(uintptr_t)dev_region->mmap_addr + + uintptr_t base_addr = dev_region->mmap_addr + dev_region->mmap_offset; + /* NOLINTNEXTLINE(performance-no-int-to-ptr) */ + const char *base = (const char *)base_addr; + + ASSERT(base_addr >= dev_region->mmap_addr); - if (m <= ptr && ptr + len <= m + dev_region->size) + if (len <= dev_region->size && base <= ptr && + (size_t)(ptr - base) <= dev_region->size - len) return 0; } -- 2.48.1