From 3602ad8f6ebb786adf1e09b4f39e9e9465d2fffc Mon Sep 17 00:00:00 2001 From: John Levon Date: Wed, 24 Nov 2021 23:15:33 +0000 Subject: is_valid_region_access(): account for wrap-around (#624) AFL++ found this, though we already knew about it, so fix it by comparing against a saturating addition. This was the only instance of client-controlled potential overflow I noticed. Signed-off-by: John Levon Reviewed-by: Swapnil Ingle Reviewed-by: Thanos Makatos --- lib/common.h | 8 ++++++++ lib/libvfio-user.c | 3 +-- test/py/libvfio_user.py | 2 ++ test/py/test_setup_region.py | 19 +++++++++++++++++++ 4 files changed, 30 insertions(+), 2 deletions(-) diff --git a/lib/common.h b/lib/common.h index 599759f..4d60199 100644 --- a/lib/common.h +++ b/lib/common.h @@ -58,6 +58,14 @@ #define ROUND_DOWN(x, a) ((x) & ~((a)-1)) #define ROUND_UP(x,a) ROUND_DOWN((x)+(a)-1, a) +/* Saturating uint64_t addition. */ +static inline uint64_t +satadd_u64(uint64_t a, uint64_t b) +{ + uint64_t res = a + b; + return (res < a) ? UINT64_MAX : res; +} + /* * The size, in bytes, of the bitmap that represents the given range with the * given page size. diff --git a/lib/libvfio-user.c b/lib/libvfio-user.c index 94a3419..9014c40 100644 --- a/lib/libvfio-user.c +++ b/lib/libvfio-user.c @@ -275,8 +275,7 @@ is_valid_region_access(vfu_ctx_t *vfu_ctx, size_t size, uint16_t cmd, return false; } - // FIXME: need to audit later for wraparound - if (ra->offset + ra->count > vfu_ctx->reg_info[index].size) { + if (satadd_u64(ra->offset, ra->count) > vfu_ctx->reg_info[index].size) { vfu_log(vfu_ctx, LOG_ERR, "out of bounds region access %#lx-%#lx " "(size %u)", ra->offset, ra->offset + ra->count, vfu_ctx->reg_info[index].size); diff --git a/test/py/libvfio_user.py b/test/py/libvfio_user.py index 77b3b67..cbe6156 100644 --- a/test/py/libvfio_user.py +++ b/test/py/libvfio_user.py @@ -42,6 +42,8 @@ import socket import struct import syslog +UINT64_MAX = 18446744073709551615 + # from linux/pci_regs.h and linux/pci_defs.h PCI_HEADER_TYPE_NORMAL = 0 diff --git a/test/py/test_setup_region.py b/test/py/test_setup_region.py index ac6dc03..76cd1d9 100644 --- a/test/py/test_setup_region.py +++ b/test/py/test_setup_region.py @@ -174,5 +174,24 @@ def test_setup_region_cfg_always_cb(): disconnect_client(ctx, sock) +def test_region_offset_overflow(): + global ctx + + ret = vfu_setup_region(ctx, index=VFU_PCI_DEV_CFG_REGION_IDX, + size=PCI_CFG_SPACE_EXP_SIZE, cb=pci_cfg_region_cb, + flags=(VFU_REGION_FLAG_RW)) + assert ret == 0 + + ret = vfu_realize_ctx(ctx) + assert ret == 0 + + sock = connect_client(ctx) + + read_region(ctx, sock, VFU_PCI_DEV_CFG_REGION_IDX, + offset=UINT64_MAX, count=256, expect=errno.EINVAL) + + disconnect_client(ctx, sock) + + def test_setup_region_cleanup(): vfu_destroy_ctx(ctx) -- cgit v1.1