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, at least if we're treating 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.
While addressing this, correct a flaw in vu_packet_check_range() where it
only checked that the given packet piece was above the region's mmap
address, not the base of the specific section of the mmap we expect to find
it in (dev_region->mmap_addr + dev_region->mmap_offset).
Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au>
---
packet.c | 9 ++++++---
vu_common.c | 15 +++++++++++----
2 files changed, 17 insertions(+), 7 deletions(-)
diff --git a/packet.c b/packet.c
index c3b80924..04a7a829 100644
--- a/packet.c
+++ b/packet.c
@@ -49,9 +49,12 @@ static void packet_check_range(const struct pool *p,
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),
+ ASSERT_WITH_MSG(len <= p->buf_size,
+ "packet range length %zu larger than buffer %zu, %s:%i",
+ len, p->buf_size, func, line);
+ ASSERT_WITH_MSG((size_t)(ptr - p->buf) <= p->buf_size - len,
+"packet range %p, len %zu extends beyond buffer %p, len %zu, %s:%i",
+ (void *)ptr, len, (void *)p->buf, p->buf_size,
func, line);
}
diff --git a/vu_common.c b/vu_common.c
index 6f49f873..3f7fb29d 100644
--- a/vu_common.c
+++ b/vu_common.c
@@ -35,16 +35,23 @@ void 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++) {
+ uintptr_t base_addr = dev_region->mmap_addr +
+ dev_region->mmap_offset;
/* NOLINTNEXTLINE(performance-no-int-to-ptr) */
- char *m = (char *)(uintptr_t)dev_region->mmap_addr;
+ const char *base = (const char *)base_addr;
- if (m <= ptr &&
- ptr + len <= m + dev_region->mmap_offset + dev_region->size)
+ ASSERT(base_addr >= dev_region->mmap_addr);
+
+ if (len > dev_region->size)
+ continue;
+
+ if (base <= ptr &&
+ (size_t)(ptr - base) <= dev_region->size - len)
return;
}
abort_with_msg(
- "package range at %p, length %zd not within dev region %s:%i",
+ "packet range at %p, length %zd not within dev region, %s:%i",
(void *)ptr, len, func, line);
}
--
2.47.1