[PATCH v2 09/12] packet: Distinguish severities of different packet_{add,git}_do() errors

[David Gibson <david@gibson.dropbear.id.au>]

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 <david@gibson.dropbear.id.au>
---
 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);
 }
 
 /**
-- 
@@ -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