Commit 2be196c6 authored by Jim Harris's avatar Jim Harris Committed by Tomasz Zawadzki
Browse files

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: default avatarJim Harris <james.r.harris@intel.com>
Change-Id: Iaa369be341eb4eba394f248990e56dce001d3940
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/15579


Reviewed-by: default avatarMariusz Barczak <mariusz.barczak@intel.com>
Reviewed-by: default avatarWojciech Malikowski <wojciech.malikowski@intel.com>
Reviewed-by: default avatarChangpeng Liu <changpeng.liu@intel.com>
Reviewed-by: default avatarAleksey Marchuk <alexeymar@nvidia.com>
Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
parent ef9168dc

lib/nvme/nvme_pcie_common.c

+7 −4
Original line number Diff line number Diff line
@@ -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;

			}

		}

test/unit/lib/nvme/nvme_pcie.c/nvme_pcie_ut.c

+21 −5
Original line number Diff line number Diff line
@@ -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);

}