Commit ad521730 authored by John Levon's avatar John Levon Committed by Jim Harris
Browse files

lib/nvmf: fix req->data usage in nvmf_ctrlr_get_features() handlers 



This code has a similar potential problem as the identify
and log page commands did: stop using req->data in favour of IOVs.

We also need to fix the unit tests to initialize the iovs.

We don't change the existing "set" behaviour of requiring a single IOV
here.

Signed-off-by: default avatarJohn Levon <john.levon@nutanix.com>
Change-Id: I257567a7abd5fc3ed9ee21b432c7da7d70fbbde0
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/16122


Reviewed-by: default avatarJim Harris <james.r.harris@intel.com>
Reviewed-by: default avatarThanos Makatos <thanos.makatos@nutanix.com>
Reviewed-by: default avatarShuhei Matsumoto <smatsumoto@nvidia.com>
Tested-by: default avatarSPDK CI Jenkins <sys_sgci@intel.com>
parent acc4d176

lib/nvmf/ctrlr.c

+8 −2
Original line number Diff line number Diff line
@@ -1687,6 +1687,7 @@ nvmf_ctrlr_get_features_host_identifier(struct spdk_nvmf_request *req) 
	struct spdk_nvmf_ctrlr *ctrlr = req->qpair->ctrlr;

	struct spdk_nvme_cmd *cmd = &req->cmd->nvme_cmd;

	struct spdk_nvme_cpl *response = &req->rsp->nvme_cpl;

	struct copy_iovs_ctx copy_ctx;



	SPDK_DEBUGLOG(nvmf, "Get Features - Host Identifier\n");
@@ -1703,7 +1704,9 @@ nvmf_ctrlr_get_features_host_identifier(struct spdk_nvmf_request *req) 
		return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE;

	}



	spdk_uuid_copy((struct spdk_uuid *)req->data, &ctrlr->hostid);

	_init_copy_iovs_ctx(&copy_ctx, req->iov, req->iovcnt);

	_copy_buf_to_iovs(&copy_ctx, &ctrlr->hostid, sizeof(ctrlr->hostid));



	return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE;

}
@@ -1830,6 +1833,7 @@ nvmf_ctrlr_get_features_host_behavior_support(struct spdk_nvmf_request *req) 
	struct spdk_nvmf_ctrlr *ctrlr = req->qpair->ctrlr;

	struct spdk_nvme_cpl *response = &req->rsp->nvme_cpl;

	struct spdk_nvme_host_behavior host_behavior = {};

	struct copy_iovs_ctx copy_ctx;



	SPDK_DEBUGLOG(nvmf, "Get Features - Host Behavior Support\n");
@@ -1841,7 +1845,9 @@ nvmf_ctrlr_get_features_host_behavior_support(struct spdk_nvmf_request *req) 
	}



	host_behavior.acre = ctrlr->acre_enabled;

	memcpy(req->data, &host_behavior, sizeof(host_behavior));



	_init_copy_iovs_ctx(&copy_ctx, req->iov, req->iovcnt);

	_copy_buf_to_iovs(&copy_ctx, &host_behavior, sizeof(host_behavior));



	return SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE;

}

test/unit/lib/nvmf/ctrlr.c/ctrlr_ut.c

+10 −1
Original line number Diff line number Diff line
@@ -2766,8 +2766,10 @@ test_nvmf_ctrlr_get_features_host_behavior_support(void) 
	req.rsp = &rsp;



	/* Invalid data */

	req.data = NULL;

	req.length = sizeof(struct spdk_nvme_host_behavior);

	req.data = NULL;



	req.iovcnt = 0;



	rc = nvmf_ctrlr_get_features_host_behavior_support(&req);

	CU_ASSERT(rc == SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE);
@@ -2778,6 +2780,9 @@ test_nvmf_ctrlr_get_features_host_behavior_support(void) 
	/* Wrong structure length */

	req.data = &behavior;

	req.length = sizeof(struct spdk_nvme_host_behavior) - 1;

	req.iovcnt = 1;

	req.iov[0].iov_base = req.data;

	req.iov[0].iov_len = req.length;



	rc = nvmf_ctrlr_get_features_host_behavior_support(&req);

	CU_ASSERT(rc == SPDK_NVMF_REQUEST_EXEC_STATUS_COMPLETE);
@@ -2787,6 +2792,10 @@ test_nvmf_ctrlr_get_features_host_behavior_support(void) 
	/* Get Features Host Behavior Support Success */

	req.data = &behavior;

	req.length = sizeof(struct spdk_nvme_host_behavior);

	req.iovcnt = 1;

	req.iov[0].iov_base = req.data;

	req.iov[0].iov_len = req.length;



	ctrlr.acre_enabled = true;

	host_behavior = (struct spdk_nvme_host_behavior *)req.data;

	host_behavior->acre = false;