diff --git a/lib/bdev/bdev.c b/lib/bdev/bdev.c index b3e4d200f..2f7c491ac 100644 --- a/lib/bdev/bdev.c +++ b/lib/bdev/bdev.c @@ -1978,10 +1978,6 @@ spdk_bdev_free_io(struct spdk_bdev_io *bdev_io) bdev_io_put_buf(bdev_io); } - if (spdk_unlikely(bdev_io->internal.ext_opts == &bdev_io->internal.ext_opts_copy)) { - memset(&bdev_io->internal.ext_opts_copy, 0, sizeof(bdev_io->internal.ext_opts_copy)); - } - if (ch->per_thread_cache_count < ch->bdev_io_cache_size) { ch->per_thread_cache_count++; STAILQ_INSERT_HEAD(&ch->per_thread_cache, bdev_io, internal.buf_link); @@ -2926,8 +2922,11 @@ _bdev_io_copy_ext_opts(struct spdk_bdev_io *bdev_io, struct spdk_bdev_ext_io_opt { struct spdk_bdev_ext_io_opts *opts_copy = &bdev_io->internal.ext_opts_copy; + /* Zero part we don't copy */ + memset(((char *)opts_copy) + opts->size, 0, sizeof(*opts) - opts->size); memcpy(opts_copy, opts, opts->size); - bdev_io->internal.ext_opts_copy.metadata = bdev_io->u.bdev.md_buf; + opts_copy->size = sizeof(*opts_copy); + opts_copy->metadata = bdev_io->u.bdev.md_buf; /* Save pointer to the copied ext_opts which will be used by bdev modules */ bdev_io->u.bdev.ext_opts = opts_copy; } @@ -2959,7 +2958,7 @@ _bdev_io_submit_ext(struct spdk_bdev_desc *desc, struct spdk_bdev_io *bdev_io, * copy if size is smaller than opts struct to avoid having to check size * on every access to bdev_io->u.bdev.ext_opts */ - if (copy_opts || use_pull_push) { + if (copy_opts || use_pull_push || opts->size < sizeof(*opts)) { _bdev_io_copy_ext_opts(bdev_io, opts); if (use_pull_push) { _bdev_io_ext_use_bounce_buffer(bdev_io); @@ -4307,7 +4306,13 @@ spdk_bdev_readv_blocks_with_md(struct spdk_bdev_desc *desc, struct spdk_io_chann static inline bool _bdev_io_check_opts(struct spdk_bdev_ext_io_opts *opts, struct iovec *iov) { - return opts->size > 0 && + /* + * We check if opts size is at least of size when we first introduced + * spdk_bdev_ext_io_opts (ac6f2bdd8d) since access to those members + * are not checked internal. + */ + return opts->size >= offsetof(struct spdk_bdev_ext_io_opts, metadata) + + sizeof(opts->metadata) && opts->size <= sizeof(*opts) && /* When memory domain is used, the user must provide data buffers */ (!opts->memory_domain || (iov && iov[0].iov_base)); diff --git a/test/unit/lib/bdev/bdev.c/bdev_ut.c b/test/unit/lib/bdev/bdev.c/bdev_ut.c index e2cf897b8..8afbac6ae 100644 --- a/test/unit/lib/bdev/bdev.c/bdev_ut.c +++ b/test/unit/lib/bdev/bdev.c/bdev_ut.c @@ -5094,6 +5094,13 @@ bdev_writev_readv_ext(void) rc = spdk_bdev_writev_blocks_ext(desc, io_ch, &iov, 1, 32, 14, io_done, NULL, &ext_io_opts); CU_ASSERT(rc != 0); + ext_io_opts.size = offsetof(struct spdk_bdev_ext_io_opts, metadata) + + sizeof(ext_io_opts.metadata) - 1; + rc = spdk_bdev_readv_blocks_ext(desc, io_ch, &iov, 1, 32, 14, io_done, NULL, &ext_io_opts); + CU_ASSERT(rc != 0); + rc = spdk_bdev_writev_blocks_ext(desc, io_ch, &iov, 1, 32, 14, io_done, NULL, &ext_io_opts); + CU_ASSERT(rc != 0); + /* Test 3, Check that IO request with ext_opts and metadata is split correctly * Offset 14, length 8, payload 0xF000 * Child - Offset 14, length 2, payload 0xF000 @@ -5152,63 +5159,6 @@ bdev_writev_readv_ext(void) CU_ASSERT(g_io_done == true); CU_ASSERT(g_bdev_ut_channel->outstanding_io_count == 0); - /* ext_opts contains metadata but ext_opts::size doesn't cover metadata pointer. - * Check that IO request with ext_opts and metadata is split correctly - metadata pointer is not copied, so split - * requests do not have metadata - * Offset 14, length 8, payload 0xF000 - * Child - Offset 14, length 2, payload 0xF000 - * Child - Offset 16, length 6, payload 0xF000 + 2 * 512 - */ - iov.iov_base = (void *)0xF000; - iov.iov_len = 4096; - memset(&ext_io_opts, 0, sizeof(ext_io_opts)); - ext_io_opts.metadata = (void *)0xFF000000; - ext_io_opts.size = offsetof(struct spdk_bdev_ext_io_opts, metadata);; - g_io_done = false; - - /* read */ - expected_io = ut_alloc_expected_io(SPDK_BDEV_IO_TYPE_READ, 14, 2, 1); - /* Split request must not contain metadata pointer */ - expected_io->md_buf = NULL; - ut_expected_io_set_iov(expected_io, 0, (void *)0xF000, 2 * 512); - TAILQ_INSERT_TAIL(&g_bdev_ut_channel->expected_io, expected_io, link); - - expected_io = ut_alloc_expected_io(SPDK_BDEV_IO_TYPE_READ, 16, 6, 1); - expected_io->md_buf = NULL; - ut_expected_io_set_iov(expected_io, 0, (void *)(0xF000 + 2 * 512), 6 * 512); - TAILQ_INSERT_TAIL(&g_bdev_ut_channel->expected_io, expected_io, link); - - rc = spdk_bdev_readv_blocks_ext(desc, io_ch, &iov, 1, 14, 8, io_done, NULL, &ext_io_opts); - CU_ASSERT(rc == 0); - CU_ASSERT(g_io_done == false); - - CU_ASSERT(g_bdev_ut_channel->outstanding_io_count == 2); - stub_complete_io(2); - CU_ASSERT(g_io_done == true); - CU_ASSERT(g_bdev_ut_channel->outstanding_io_count == 0); - - /* write */ - g_io_done = false; - expected_io = ut_alloc_expected_io(SPDK_BDEV_IO_TYPE_WRITE, 14, 2, 1); - expected_io->md_buf = NULL; - ut_expected_io_set_iov(expected_io, 0, (void *)0xF000, 2 * 512); - TAILQ_INSERT_TAIL(&g_bdev_ut_channel->expected_io, expected_io, link); - - expected_io = ut_alloc_expected_io(SPDK_BDEV_IO_TYPE_WRITE, 16, 6, 1); - /* Split request must not contain metadata pointer */ - expected_io->md_buf = NULL; - ut_expected_io_set_iov(expected_io, 0, (void *)(0xF000 + 2 * 512), 6 * 512); - TAILQ_INSERT_TAIL(&g_bdev_ut_channel->expected_io, expected_io, link); - - rc = spdk_bdev_writev_blocks_ext(desc, io_ch, &iov, 1, 14, 8, io_done, NULL, &ext_io_opts); - CU_ASSERT(rc == 0); - CU_ASSERT(g_io_done == false); - - CU_ASSERT(g_bdev_ut_channel->outstanding_io_count == 2); - stub_complete_io(2); - CU_ASSERT(g_io_done == true); - CU_ASSERT(g_bdev_ut_channel->outstanding_io_count == 0); - /* Test 4, Verify data pull/push * bdev doens't support memory domains, so buffers from bdev memory pool will be used */ ext_io_opts.memory_domain = (struct spdk_memory_domain *)0xdeadbeef;