From 2be196c6097e1f68b9f0e01056f9df79850df62a Mon Sep 17 00:00:00 2001 From: Jim Harris Date: Tue, 22 Nov 2022 18:10:50 +0000 Subject: [PATCH] nvme/pcie: validate that mptr is iova contiguous Also add unit tests that explicitly test this condition. They fail without the nvme driver changes in this patch. Signed-off-by: Jim Harris Change-Id: Iaa369be341eb4eba394f248990e56dce001d3940 Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/15579 Reviewed-by: Mariusz Barczak Reviewed-by: Wojciech Malikowski Reviewed-by: Changpeng Liu Reviewed-by: Aleksey Marchuk Tested-by: SPDK CI Jenkins --- lib/nvme/nvme_pcie_common.c | 11 +++++--- test/unit/lib/nvme/nvme_pcie.c/nvme_pcie_ut.c | 26 +++++++++++++++---- 2 files changed, 28 insertions(+), 9 deletions(-) diff --git a/lib/nvme/nvme_pcie_common.c b/lib/nvme/nvme_pcie_common.c index 6ff59193b..c75155c2c 100644 --- a/lib/nvme/nvme_pcie_common.c +++ b/lib/nvme/nvme_pcie_common.c @@ -1585,6 +1585,7 @@ nvme_pcie_qpair_build_metadata(struct spdk_nvme_qpair *qpair, struct nvme_tracke { void *md_payload; struct nvme_request *req = tr->req; + uint64_t mapping_length; if (req->payload.md) { md_payload = req->payload.md + req->md_offset; @@ -1593,11 +1594,13 @@ nvme_pcie_qpair_build_metadata(struct spdk_nvme_qpair *qpair, struct nvme_tracke goto exit; } + mapping_length = req->md_size; if (sgl_supported && dword_aligned) { assert(req->cmd.psdt == SPDK_NVME_PSDT_SGL_MPTR_CONTIG); req->cmd.psdt = SPDK_NVME_PSDT_SGL_MPTR_SGL; - tr->meta_sgl.address = nvme_pcie_vtophys(qpair->ctrlr, md_payload, NULL); - if (tr->meta_sgl.address == SPDK_VTOPHYS_ERROR) { + + tr->meta_sgl.address = nvme_pcie_vtophys(qpair->ctrlr, md_payload, &mapping_length); + if (tr->meta_sgl.address == SPDK_VTOPHYS_ERROR || mapping_length != req->md_size) { goto exit; } tr->meta_sgl.unkeyed.type = SPDK_NVME_SGL_TYPE_DATA_BLOCK; @@ -1605,8 +1608,8 @@ nvme_pcie_qpair_build_metadata(struct spdk_nvme_qpair *qpair, struct nvme_tracke tr->meta_sgl.unkeyed.subtype = 0; req->cmd.mptr = tr->prp_sgl_bus_addr - sizeof(struct spdk_nvme_sgl_descriptor); } else { - req->cmd.mptr = nvme_pcie_vtophys(qpair->ctrlr, md_payload, NULL); - if (req->cmd.mptr == SPDK_VTOPHYS_ERROR) { + req->cmd.mptr = nvme_pcie_vtophys(qpair->ctrlr, md_payload, &mapping_length); + if (req->cmd.mptr == SPDK_VTOPHYS_ERROR || mapping_length != req->md_size) { goto exit; } } diff --git a/test/unit/lib/nvme/nvme_pcie.c/nvme_pcie_ut.c b/test/unit/lib/nvme/nvme_pcie.c/nvme_pcie_ut.c index 8c22dfbe6..347991266 100644 --- a/test/unit/lib/nvme/nvme_pcie.c/nvme_pcie_ut.c +++ b/test/unit/lib/nvme/nvme_pcie.c/nvme_pcie_ut.c @@ -98,7 +98,7 @@ DEFINE_RETURN_MOCK(spdk_vtophys, uint64_t); uint64_t spdk_vtophys(const void *buf, uint64_t *size) { - if (size) { + if (size && g_vtophys_size > 0) { *size = g_vtophys_size; } @@ -491,7 +491,8 @@ test_build_contig_hw_sgl_request(void) static void test_nvme_pcie_qpair_build_metadata(void) { - struct spdk_nvme_qpair qpair = {}; + struct nvme_pcie_qpair pqpair = {}; + struct spdk_nvme_qpair *qpair = &pqpair.qpair; struct nvme_tracker tr = {}; struct nvme_request req = {}; struct spdk_nvme_ctrlr ctrlr = {}; @@ -499,7 +500,7 @@ test_nvme_pcie_qpair_build_metadata(void) ctrlr.trid.trtype = SPDK_NVME_TRANSPORT_PCIE; tr.req = &req; - qpair.ctrlr = &ctrlr; + qpair->ctrlr = &ctrlr; req.payload.md = (void *)0xDEADBEE0; req.md_offset = 0; @@ -508,7 +509,7 @@ test_nvme_pcie_qpair_build_metadata(void) tr.prp_sgl_bus_addr = 0xDBADBEEF; MOCK_SET(spdk_vtophys, 0xDCADBEE0); - rc = nvme_pcie_qpair_build_metadata(&qpair, &tr, true, true); + rc = nvme_pcie_qpair_build_metadata(qpair, &tr, true, true); CU_ASSERT(rc == 0); CU_ASSERT(req.cmd.psdt = SPDK_NVME_PSDT_SGL_MPTR_SGL); CU_ASSERT(tr.meta_sgl.address == 0xDCADBEE0); @@ -516,14 +517,29 @@ test_nvme_pcie_qpair_build_metadata(void) CU_ASSERT(tr.meta_sgl.unkeyed.length == 4096); CU_ASSERT(tr.meta_sgl.unkeyed.subtype == 0); CU_ASSERT(req.cmd.mptr == (0xDBADBEEF - sizeof(struct spdk_nvme_sgl_descriptor))); + + /* Non-IOVA contiguous metadata buffers should fail. */ + g_vtophys_size = 1024; + req.cmd.psdt = SPDK_NVME_PSDT_SGL_MPTR_CONTIG; + rc = nvme_pcie_qpair_build_metadata(qpair, &tr, true, true); + CU_ASSERT(rc == -EINVAL); + g_vtophys_size = 0; + MOCK_CLEAR(spdk_vtophys); /* Build non sgl metadata */ MOCK_SET(spdk_vtophys, 0xDDADBEE0); - rc = nvme_pcie_qpair_build_metadata(&qpair, &tr, false, true); + rc = nvme_pcie_qpair_build_metadata(qpair, &tr, false, true); CU_ASSERT(rc == 0); CU_ASSERT(req.cmd.mptr == 0xDDADBEE0); + + /* Non-IOVA contiguous metadata buffers should fail. */ + g_vtophys_size = 1024; + rc = nvme_pcie_qpair_build_metadata(qpair, &tr, false, true); + CU_ASSERT(rc == -EINVAL); + g_vtophys_size = 0; + MOCK_CLEAR(spdk_vtophys); }