Commit e7625088 authored by Alexey Marchuk's avatar Alexey Marchuk Committed by Tomasz Zawadzki
Browse files

nvme_rdma: Add check for keyed SGL length 



The length of a keyed SGL data block is limited by 3 bytes.
Add a check to fail requests which length exceeds 3 bytes.
In other case we can send an incorrectly formed SGL request with
an invalid or zero length.

Fixes issue #1450

Change-Id: I77cdaff5fbf4be5754a3ac6008b8ccd532ac5905
Signed-off-by: default avatarAlexey Marchuk <alexeymar@mellanox.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/3056


Reviewed-by: default avatarShuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: default avatarBen Walker <benjamin.walker@intel.com>
Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
parent d09acccf

lib/nvme/nvme_rdma.c

+17 −0
Original line number Diff line number Diff line
@@ -95,6 +95,11 @@ 
 */

#define NVME_RDMA_DESTROYED_QPAIR_EXPIRATION_CYCLES	50



/*

 * The max length of keyed SGL data block (3 bytes)

 */

#define NVME_RDMA_MAX_KEYED_SGL_LENGTH ((1u << 24u) - 1)



#define WC_PER_QPAIR(queue_depth)	(queue_depth * 2)



enum nvme_rdma_wr_type {
@@ -1508,6 +1513,12 @@ nvme_rdma_build_contig_request(struct nvme_rdma_qpair *rqpair, 
	assert(req->payload_size != 0);

	assert(nvme_payload_type(&req->payload) == NVME_PAYLOAD_TYPE_CONTIG);



	if (spdk_unlikely(req->payload_size > NVME_RDMA_MAX_KEYED_SGL_LENGTH)) {

		SPDK_ERRLOG("SGL length %u exceeds max keyed SGL block size %u\n",

			    req->payload_size, NVME_RDMA_MAX_KEYED_SGL_LENGTH);

		return -1;

	}



	if (spdk_unlikely(!nvme_rdma_get_key(rqpair->mr_map->map, payload, req->payload_size,

					     NVME_RDMA_MR_RKEY, &rkey))) {

		return -1;
@@ -1566,6 +1577,12 @@ nvme_rdma_build_sgl_request(struct nvme_rdma_qpair *rqpair, 


		sge_length = spdk_min(remaining_size, sge_length);



		if (spdk_unlikely(sge_length > NVME_RDMA_MAX_KEYED_SGL_LENGTH)) {

			SPDK_ERRLOG("SGL length %u exceeds max keyed SGL block size %u\n",

				    sge_length, NVME_RDMA_MAX_KEYED_SGL_LENGTH);

			return -1;

		}



		if (spdk_unlikely(!nvme_rdma_get_key(rqpair->mr_map->map, virt_addr, sge_length,

						     NVME_RDMA_MR_RKEY, &rkey))) {

			return -1;

test/unit/lib/nvme/nvme_rdma.c/nvme_rdma_ut.c

+185 −1
Original line number Diff line number Diff line
@@ -193,7 +193,7 @@ test_nvme_rdma_build_sgl_request(void) 
	SPDK_CU_ASSERT_FATAL(rc != 0);

	CU_ASSERT(bio.iovpos == 1);



	/* Test case 4: Multiple SGL, SGL size smaller than I/O size */

	/* Test case 4: Multiple SGL, SGL size smaller than I/O size. Expected: FAIL */

	bio.iovpos = 0;

	req.payload_offset = 0;

	req.payload_size = 0x6000;
@@ -201,6 +201,187 @@ test_nvme_rdma_build_sgl_request(void) 
	rc = nvme_rdma_build_sgl_request(&rqpair, &rdma_req);

	SPDK_CU_ASSERT_FATAL(rc != 0);

	CU_ASSERT(bio.iovpos == NVME_RDMA_MAX_SGL_DESCRIPTORS);



	/* Test case 5: SGL length exceeds 3 bytes. Expected: FAIL */

	req.payload_size = 0x1000 + (1 << 24);

	bio.iovs[0].iov_len = 0x1000;

	bio.iovs[1].iov_len = 1 << 24;

	rc = nvme_rdma_build_sgl_request(&rqpair, &rdma_req);

	SPDK_CU_ASSERT_FATAL(rc != 0);

}



static void

test_nvme_rdma_build_sgl_inline_request(void)

{

	struct nvme_rdma_qpair rqpair;

	struct spdk_nvme_ctrlr ctrlr = {0};

	struct spdk_nvmf_cmd cmd = {{0}};

	struct spdk_nvme_rdma_req rdma_req = {0};

	struct nvme_request req = {{0}};

	struct nvme_rdma_ut_bdev_io bio;

	struct spdk_nvme_rdma_mr_map rmap = {0};

	struct spdk_mem_map *map = NULL;

	int rc;



	rmap.map = map;



	ctrlr.max_sges = NVME_RDMA_MAX_SGL_DESCRIPTORS;

	ctrlr.cdata.nvmf_specific.msdbd = 16;



	rqpair.mr_map = &rmap;

	rqpair.qpair.ctrlr = &ctrlr;

	rqpair.cmds = &cmd;

	cmd.sgl[0].address = 0x1111;

	rdma_req.id = 0;

	rdma_req.req = &req;



	req.payload.reset_sgl_fn = nvme_rdma_ut_reset_sgl;

	req.payload.next_sge_fn = nvme_rdma_ut_next_sge;

	req.payload.contig_or_cb_arg = &bio;

	req.qpair = &rqpair.qpair;



	g_nvme_rdma_mr.lkey = 2;



	/* Test case 1: single inline SGL. Expected: PASS */

	bio.iovpos = 0;

	req.payload_offset = 0;

	req.payload_size = 0x1000;

	bio.iovs[0].iov_base = (void *)0xdeadbeef;

	bio.iovs[0].iov_len = 0x1000;

	rc = nvme_rdma_build_sgl_inline_request(&rqpair, &rdma_req);

	SPDK_CU_ASSERT_FATAL(rc == 0);

	CU_ASSERT(bio.iovpos == 1);

	CU_ASSERT(req.cmd.dptr.sgl1.unkeyed.type == SPDK_NVME_SGL_TYPE_DATA_BLOCK);

	CU_ASSERT(req.cmd.dptr.sgl1.unkeyed.subtype == SPDK_NVME_SGL_SUBTYPE_OFFSET);

	CU_ASSERT(req.cmd.dptr.sgl1.unkeyed.length == req.payload_size);

	CU_ASSERT(req.cmd.dptr.sgl1.address == 0);

	CU_ASSERT(rdma_req.send_sgl[0].length == sizeof(struct spdk_nvme_cmd));

	CU_ASSERT(rdma_req.send_sgl[1].length == req.payload_size);

	CU_ASSERT(rdma_req.send_sgl[1].addr == (uint64_t)bio.iovs[0].iov_base);

	CU_ASSERT(rdma_req.send_sgl[1].lkey == g_nvme_rdma_mr.lkey);



	/* Test case 2: SGL length exceeds 3 bytes. Expected: PASS */

	bio.iovpos = 0;

	req.payload_offset = 0;

	req.payload_size = 1 << 24;

	bio.iovs[0].iov_len = 1 << 24;

	rc = nvme_rdma_build_sgl_inline_request(&rqpair, &rdma_req);

	SPDK_CU_ASSERT_FATAL(rc == 0);

	CU_ASSERT(bio.iovpos == 1);

	CU_ASSERT(req.cmd.dptr.sgl1.unkeyed.type == SPDK_NVME_SGL_TYPE_DATA_BLOCK);

	CU_ASSERT(req.cmd.dptr.sgl1.unkeyed.subtype == SPDK_NVME_SGL_SUBTYPE_OFFSET);

	CU_ASSERT(req.cmd.dptr.sgl1.unkeyed.length == req.payload_size);

	CU_ASSERT(req.cmd.dptr.sgl1.address == 0);

	CU_ASSERT(rdma_req.send_sgl[0].length == sizeof(struct spdk_nvme_cmd));

	CU_ASSERT(rdma_req.send_sgl[1].length == req.payload_size);

	CU_ASSERT(rdma_req.send_sgl[1].addr == (uint64_t)bio.iovs[0].iov_base);

	CU_ASSERT(rdma_req.send_sgl[1].lkey == g_nvme_rdma_mr.lkey);

}



static void

test_nvme_rdma_build_contig_request(void)

{

	struct nvme_rdma_qpair rqpair;

	struct spdk_nvme_ctrlr ctrlr = {0};

	struct spdk_nvmf_cmd cmd = {{0}};

	struct spdk_nvme_rdma_req rdma_req = {0};

	struct nvme_request req = {{0}};

	struct spdk_nvme_rdma_mr_map rmap = {0};

	struct spdk_mem_map *map = NULL;

	int rc;



	rmap.map = map;



	ctrlr.max_sges = NVME_RDMA_MAX_SGL_DESCRIPTORS;

	ctrlr.cdata.nvmf_specific.msdbd = 16;



	rqpair.mr_map = &rmap;

	rqpair.qpair.ctrlr = &ctrlr;

	rqpair.cmds = &cmd;

	cmd.sgl[0].address = 0x1111;

	rdma_req.id = 0;

	rdma_req.req = &req;



	req.payload.contig_or_cb_arg = (void *)0xdeadbeef;

	req.qpair = &rqpair.qpair;



	g_nvme_rdma_mr.rkey = 2;



	/* Test case 1: contig request. Expected: PASS */

	req.payload_offset = 0;

	req.payload_size = 0x1000;

	rc = nvme_rdma_build_contig_request(&rqpair, &rdma_req);

	SPDK_CU_ASSERT_FATAL(rc == 0);

	CU_ASSERT(req.cmd.dptr.sgl1.keyed.type == SPDK_NVME_SGL_TYPE_KEYED_DATA_BLOCK);

	CU_ASSERT(req.cmd.dptr.sgl1.keyed.subtype == SPDK_NVME_SGL_SUBTYPE_ADDRESS);

	CU_ASSERT(req.cmd.dptr.sgl1.keyed.length == req.payload_size);

	CU_ASSERT(req.cmd.dptr.sgl1.keyed.key == g_nvme_rdma_mr.rkey);

	CU_ASSERT(req.cmd.dptr.sgl1.address == (uint64_t)req.payload.contig_or_cb_arg);

	CU_ASSERT(rdma_req.send_sgl[0].length == sizeof(struct spdk_nvme_cmd));



	/* Test case 2: SGL length exceeds 3 bytes. Expected: FAIL */

	req.payload_offset = 0;

	req.payload_size = 1 << 24;

	rc = nvme_rdma_build_contig_request(&rqpair, &rdma_req);

	SPDK_CU_ASSERT_FATAL(rc != 0);

}



static void

test_nvme_rdma_build_contig_inline_request(void)

{

	struct nvme_rdma_qpair rqpair;

	struct spdk_nvme_ctrlr ctrlr = {0};

	struct spdk_nvmf_cmd cmd = {{0}};

	struct spdk_nvme_rdma_req rdma_req = {0};

	struct nvme_request req = {{0}};

	struct spdk_nvme_rdma_mr_map rmap = {0};

	struct spdk_mem_map *map = NULL;

	int rc;



	rmap.map = map;



	ctrlr.max_sges = NVME_RDMA_MAX_SGL_DESCRIPTORS;

	ctrlr.cdata.nvmf_specific.msdbd = 16;



	rqpair.mr_map = &rmap;

	rqpair.qpair.ctrlr = &ctrlr;

	rqpair.cmds = &cmd;

	cmd.sgl[0].address = 0x1111;

	rdma_req.id = 0;

	rdma_req.req = &req;



	req.payload.contig_or_cb_arg = (void *)0xdeadbeef;

	req.qpair = &rqpair.qpair;



	g_nvme_rdma_mr.rkey = 2;



	/* Test case 1: single inline SGL. Expected: PASS */

	req.payload_offset = 0;

	req.payload_size = 0x1000;

	rc = nvme_rdma_build_contig_inline_request(&rqpair, &rdma_req);

	SPDK_CU_ASSERT_FATAL(rc == 0);

	CU_ASSERT(req.cmd.dptr.sgl1.unkeyed.type == SPDK_NVME_SGL_TYPE_DATA_BLOCK);

	CU_ASSERT(req.cmd.dptr.sgl1.unkeyed.subtype == SPDK_NVME_SGL_SUBTYPE_OFFSET);

	CU_ASSERT(req.cmd.dptr.sgl1.unkeyed.length == req.payload_size);

	CU_ASSERT(req.cmd.dptr.sgl1.address == 0);

	CU_ASSERT(rdma_req.send_sgl[0].length == sizeof(struct spdk_nvme_cmd));

	CU_ASSERT(rdma_req.send_sgl[1].length == req.payload_size);

	CU_ASSERT(rdma_req.send_sgl[1].addr == (uint64_t)req.payload.contig_or_cb_arg);

	CU_ASSERT(rdma_req.send_sgl[1].lkey == g_nvme_rdma_mr.lkey);



	/* Test case 2: SGL length exceeds 3 bytes. Expected: PASS */

	req.payload_offset = 0;

	req.payload_size = 1 << 24;

	rc = nvme_rdma_build_contig_inline_request(&rqpair, &rdma_req);

	SPDK_CU_ASSERT_FATAL(rc == 0);

	CU_ASSERT(req.cmd.dptr.sgl1.unkeyed.type == SPDK_NVME_SGL_TYPE_DATA_BLOCK);

	CU_ASSERT(req.cmd.dptr.sgl1.unkeyed.subtype == SPDK_NVME_SGL_SUBTYPE_OFFSET);

	CU_ASSERT(req.cmd.dptr.sgl1.unkeyed.length == req.payload_size);

	CU_ASSERT(req.cmd.dptr.sgl1.address == 0);

	CU_ASSERT(rdma_req.send_sgl[0].length == sizeof(struct spdk_nvme_cmd));

	CU_ASSERT(rdma_req.send_sgl[1].length == req.payload_size);

	CU_ASSERT(rdma_req.send_sgl[1].addr == (uint64_t)req.payload.contig_or_cb_arg);

	CU_ASSERT(rdma_req.send_sgl[1].lkey == g_nvme_rdma_mr.lkey);

}



int main(int argc, char **argv)
@@ -213,6 +394,9 @@ int main(int argc, char **argv) 


	suite = CU_add_suite("nvme_rdma", NULL, NULL);

	CU_ADD_TEST(suite, test_nvme_rdma_build_sgl_request);

	CU_ADD_TEST(suite, test_nvme_rdma_build_sgl_inline_request);

	CU_ADD_TEST(suite, test_nvme_rdma_build_contig_request);

	CU_ADD_TEST(suite, test_nvme_rdma_build_contig_inline_request);



	CU_basic_set_mode(CU_BRM_VERBOSE);

	CU_basic_run_tests();