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=202412 header.b=bX06RiZf; dkim-atps=neutral Received: from mail.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id 5A7ED5A061B for ; Fri, 20 Dec 2024 09:58:34 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202412; t=1734685098; bh=Np2+QipvvHWOZbBij+usfpi01CG/65VwuIwnUXSjFU0=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=bX06RiZfvvqOEXURMk9H8mPoSoTjnHgr8EPaY2fNi1pcgn7c7iG/YSr76D+D5nm1m uwvHcdD3yxNaLJRehC9Qv7IzWfW5GxndhxZ8LG8GLx/XbvM2N+rGlQAde4XH2ofOr5 YNGLx8XM1FH2wlDLPOfy/bm7S2JdUz+Fcn0vTCkYn/Uk9MZTatOgKZzEjqj9Nl/nDi OBXDgIga5my363+n8UX0LxAnaoerDX7BMICksJIAe6G5hAWxx3RNTCkRLJcRvZDoOX /LyjKpaUUga2bvZ3qdY3MMeLflEcz4FJi3Vv0dmRC2QtQvzc4JFJ4msbRyZerkTen3 uyMlmNoUlQkmw== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4YF1Xp1LpWz4xdV; Fri, 20 Dec 2024 19:58:18 +1100 (AEDT) From: David Gibson To: Stefano Brivio , passt-dev@passt.top Subject: [PATCH v2 09/12] packet: Distinguish severities of different packet_{add,git}_do() errors Date: Fri, 20 Dec 2024 19:35:32 +1100 Message-ID: <20241220083535.1372523-10-david@gibson.dropbear.id.au> X-Mailer: git-send-email 2.47.1 In-Reply-To: <20241220083535.1372523-1-david@gibson.dropbear.id.au> References: <20241220083535.1372523-1-david@gibson.dropbear.id.au> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Message-ID-Hash: A2PIIXTQJVIGC75YDCX2W2AU4XPRBPDP X-Message-ID-Hash: A2PIIXTQJVIGC75YDCX2W2AU4XPRBPDP 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_add() and packet_get() can fail for a number of reasons, and we currently treat them all basically the same: we log a trace() level message and for packet_get() we return NULL. However the different causes of error are quite different and suggest different handling: 1) If we run out of space in the pool on add, that's (rightly) non-fatal, but is an unusual situation which we might want to know about. Promote it's message to debug() level. 2) All packet_check_range() errors and the checks on packet length indicate a serious problem. Due to either a bug in calling code, or some sort of memory-clobbering, we've been given addresses or sizes of packets that are nonsensical. If things are this bad, we shouldn't try to carry on replace these with asserts. 3) Requesting a packet index that doesn't exist in the pool in packet_get() indicates a bug in the caller - it should always check the index first. Replace this with an assert. 4) On packet_get() requesting a section of the packet beyond its bounds is correct already. This happens routinely from some callers when they probe to see if certain parts of the packet are present. At worst it indicates that the guest has generate a malformed packet, which we should quietly drop as we already do. Signed-off-by: David Gibson --- packet.c | 71 ++++++++++++++++++++--------------------------------- packet.h | 3 ++- vu_common.c | 13 +++++++--- 3 files changed, 38 insertions(+), 49 deletions(-) diff --git a/packet.c b/packet.c index c921aa15..24f12448 100644 --- a/packet.c +++ b/packet.c @@ -30,37 +30,27 @@ * @func: For tracing: name of calling function * @line: For tracing: caller line of function call * - * Return: 0 if the range is valid, -1 otherwise + * ASSERT()s if the given range isn't valid (within the expected buffer(s) for + * the pool). */ -static int packet_check_range(const struct pool *p, const char *ptr, size_t len, - const char *func, int line) +static void packet_check_range(const struct pool *p, + const char *ptr, size_t len, + const char *func, int line) { if (p->buf_size == 0) { - int ret; - - ret = vu_packet_check_range((void *)p->buf, ptr, len); - - if (ret == -1) - trace("cannot find region, %s:%i", func, line); - - return ret; - } - - if (ptr < p->buf) { - trace("packet range start %p before buffer start %p, %s:%i", - (void *)ptr, (void *)p->buf, func, line); - 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), - func, line); - return -1; + vu_packet_check_range((void *)p->buf, ptr, len, func, line); + return; } - return 0; + ASSERT_WITH_MSG(ptr >= p->buf, + "packet range start %p before buffer start %p, %s:%i", + (void *)ptr, (void *)p->buf, func, line); + ASSERT_WITH_MSG(ptr + len <= p->buf + p->buf_size, + "packet range end %p after buffer end %p, %s:%i", + (void *)(ptr + len), (void *)(p->buf + p->buf_size), + func, line); } + /** * packet_add_do() - Add data as packet descriptor to given pool * @p: Existing pool @@ -75,18 +65,16 @@ void packet_add_do(struct pool *p, size_t len, const char *start, size_t idx = p->count; if (idx >= p->size) { - trace("add packet index %zu to pool with size %zu, %s:%i", + debug("add packet index %zu to pool with size %zu, %s:%i", idx, p->size, func, line); return; } - if (packet_check_range(p, start, len, func, line)) - return; + packet_check_range(p, start, len, func, line); - if (len > PACKET_MAX_LEN) { - trace("add packet length %zu, %s:%i", len, func, line); - return; - } + ASSERT_WITH_MSG(len <= PACKET_MAX_LEN, + "add packet length %zu (max %zu), %s:%i", + len, PACKET_MAX_LEN, func, line); p->pkt[idx].iov_base = (void *)start; p->pkt[idx].iov_len = len; @@ -111,16 +99,12 @@ void *packet_get_do(const struct pool *p, size_t idx, size_t offset, { char *ptr; - if (idx >= p->size || idx >= p->count) { - trace("packet %zu from pool size: %zu, count: %zu, %s:%i", - idx, p->size, p->count, func, line); - return NULL; - } - - if (len > PACKET_MAX_LEN) { - trace("packet data length %zu, %s:%i", len, func, line); - return NULL; - } + ASSERT_WITH_MSG(idx < p->size && idx < p->count, + "packet %zu from pool size: %zu, count: %zu, %s:%i", + idx, p->size, p->count, func, line); + ASSERT_WITH_MSG(len <= PACKET_MAX_LEN, + "packet range length %zu (max %zu), %s:%i", + len, PACKET_MAX_LEN, func, line); if (len + offset > p->pkt[idx].iov_len) { trace("data length %zu, offset %zu from length %zu, %s:%i", @@ -130,8 +114,7 @@ void *packet_get_do(const struct pool *p, size_t idx, size_t offset, ptr = (char *)p->pkt[idx].iov_base + offset; - if (packet_check_range(p, ptr, len, func, line)) - return NULL; + packet_check_range(p, ptr, len, func, line); if (left) *left = p->pkt[idx].iov_len - offset - len; diff --git a/packet.h b/packet.h index f95cda08..b164f77e 100644 --- a/packet.h +++ b/packet.h @@ -31,7 +31,8 @@ struct pool { struct iovec pkt[]; }; -int vu_packet_check_range(void *buf, const char *ptr, size_t len); +void vu_packet_check_range(void *buf, const char *ptr, size_t len, + const char *func, int line); void packet_add_do(struct pool *p, size_t len, const char *start, const char *func, int line); void *packet_get_do(const struct pool *p, const size_t idx, diff --git a/vu_common.c b/vu_common.c index 531f8786..528b9b08 100644 --- a/vu_common.c +++ b/vu_common.c @@ -24,10 +24,13 @@ * @buf: Array of the available memory regions * @ptr: Start of desired data range * @size: Length of desired data range + * @func: For tracing: name of calling function + * @line: For tracing: caller line of function call * - * Return: 0 if the zone is in a mapped memory region, -1 otherwise + * Aborts if the zone isn't in any of the device regions */ -int vu_packet_check_range(void *buf, const char *ptr, size_t len) +void vu_packet_check_range(void *buf, const char *ptr, size_t len, + const char *func, int line) { struct vu_dev_region *dev_region; @@ -37,10 +40,12 @@ int vu_packet_check_range(void *buf, const char *ptr, size_t len) if (m <= ptr && ptr + len <= m + dev_region->mmap_offset + dev_region->size) - return 0; + return; } - return -1; + abort_with_msg( + "package range at %p, length %zd not within dev region %s:%i", + (void *)ptr, len, func, line); } /** -- 2.47.1