From 4328a67ea7553add730d514c9abc1f7b67a5a3b9 Mon Sep 17 00:00:00 2001 From: Seth Howell Date: Mon, 16 Dec 2019 16:07:34 -0700 Subject: [PATCH] lib/reduce: add array bounds checking for iovs. We also need to make sure that the deconm_iov is large enough to handle all of the iovs we claim to support plus 2, one for offset into the chunk when doing writes and one for the remainder. Plus a unittest to demonstrate the possible out of bounds error in the library. Change-Id: I7747ad39f76e50f25ecf5168b01e046f71fa0ea8 Signed-off-by: Seth Howell Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/478125 Reviewed-by: Jim Harris Reviewed-by: Shuhei Matsumoto Tested-by: SPDK CI Jenkins --- lib/reduce/reduce.c | 6 +++- test/unit/lib/reduce/reduce.c/reduce_ut.c | 40 +++++++++++++++++++++++ 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/lib/reduce/reduce.c b/lib/reduce/reduce.c index 03ea2ca81..0f77e7357 100644 --- a/lib/reduce/reduce.c +++ b/lib/reduce/reduce.c @@ -103,7 +103,7 @@ struct spdk_reduce_vol_request { * the decomp engine, they point to a mix of the scratch buffer * and user buffer */ - struct iovec decomp_iov[REDUCE_MAX_IOVECS]; + struct iovec decomp_iov[REDUCE_MAX_IOVECS + 2]; int decomp_iovcnt; /** @@ -1376,6 +1376,10 @@ _iov_array_is_valid(struct spdk_reduce_vol *vol, struct iovec *iov, int iovcnt, uint64_t size = 0; int i; + if (iovcnt > REDUCE_MAX_IOVECS) { + return false; + } + for (i = 0; i < iovcnt; i++) { size += iov[i].iov_len; } diff --git a/test/unit/lib/reduce/reduce.c/reduce_ut.c b/test/unit/lib/reduce/reduce.c/reduce_ut.c index 4cf1810ee..2a3ee2386 100644 --- a/test/unit/lib/reduce/reduce.c/reduce_ut.c +++ b/test/unit/lib/reduce/reduce.c/reduce_ut.c @@ -985,6 +985,45 @@ read_write(void) _read_write(4096); } +static void +_readv_writev(uint32_t backing_blocklen) +{ + struct spdk_reduce_vol_params params = {}; + struct spdk_reduce_backing_dev backing_dev = {}; + struct iovec iov[REDUCE_MAX_IOVECS + 1]; + + params.chunk_size = 16 * 1024; + params.backing_io_unit_size = 4096; + params.logical_block_size = 512; + spdk_uuid_generate(¶ms.uuid); + + backing_dev_init(&backing_dev, ¶ms, backing_blocklen); + + g_vol = NULL; + g_reduce_errno = -1; + spdk_reduce_vol_init(¶ms, &backing_dev, TEST_MD_PATH, init_cb, NULL); + CU_ASSERT(g_reduce_errno == 0); + SPDK_CU_ASSERT_FATAL(g_vol != NULL); + + g_reduce_errno = -1; + spdk_reduce_vol_writev(g_vol, iov, REDUCE_MAX_IOVECS + 1, 2, REDUCE_MAX_IOVECS + 1, write_cb, NULL); + CU_ASSERT(g_reduce_errno == -EINVAL); + + g_reduce_errno = -1; + spdk_reduce_vol_unload(g_vol, unload_cb, NULL); + CU_ASSERT(g_reduce_errno == 0); + + persistent_pm_buf_destroy(); + backing_dev_destroy(&backing_dev); +} + +static void +readv_writev(void) +{ + _readv_writev(512); + _readv_writev(4096); +} + static void destroy_cb(void *ctx, int reduce_errno) { @@ -1257,6 +1296,7 @@ main(int argc, char **argv) CU_add_test(suite, "load", load) == NULL || CU_add_test(suite, "write_maps", write_maps) == NULL || CU_add_test(suite, "read_write", read_write) == NULL || + CU_add_test(suite, "readv_writev", readv_writev) == NULL || CU_add_test(suite, "destroy", destroy) == NULL || CU_add_test(suite, "defer_bdev_io", defer_bdev_io) == NULL || CU_add_test(suite, "overlapped", overlapped) == NULL ||