mi: be pedantic with response message format

This change adds a few tests to check against invalid response data
received by the NVMe-MI device, and some tests to simulate these
conditions.

Because we're checking more of the response fields, we need a bit more
support for constructing default responses in the mi and mi-mctp tests,
so add that too.

Signed-off-by: Jeremy Kerr <[email protected]>

Author: jk-ozlabs

src/nvme/mi.c

@@ -192,6 +192,33 @@ int nvme_mi_submit(nvme_mi_ep_t ep, struct nvme_mi_req *req,
 		}
 	}
 
+	/* basic response checks */
+	if (resp->hdr_len < sizeof(struct nvme_mi_msg_hdr)) {
+		nvme_msg(ep->root, LOG_DEBUG,
+			 "Bad response header len: %zd\n", resp->hdr_len);
+		return -EIO;
+	}
+
+	if (resp->hdr->type != NVME_MI_MSGTYPE_NVME) {
+		nvme_msg(ep->root, LOG_DEBUG,
+			 "Invalid message type 0x%02x\n", resp->hdr->type);
+		return -EPROTO;
+	}
+
+	if (!(resp->hdr->nmp & (NVME_MI_ROR_RSP << 7))) {
+		nvme_msg(ep->root, LOG_DEBUG,
+			 "ROR value in response indicates a request\n");
+		return -EIO;
+	}
+
+	if ((resp->hdr->nmp & 0x1) != (req->hdr->nmp & 0x1)) {
+		nvme_msg(ep->root, LOG_WARNING,
+			 "Command slot mismatch: req %d, resp %d\n",
+			 req->hdr->nmp & 0x1,
+			 resp->hdr->nmp & 0x1);
+		return -EIO;
+	}
+
 	return 0;
 }
 

test/mi-mctp.c

@@ -112,10 +112,18 @@ ssize_t __wrap_sendmsg(int sd, const struct msghdr *hdr, int flags)
 
 	test_peer.rx_buf_len = pos;
 
-	if (test_peer.rx_fn)
+
+	if (test_peer.rx_fn) {
 		test_peer.rx_res = test_peer.rx_fn(&test_peer,
 						   test_peer.rx_buf,
 						   test_peer.rx_buf_len);
+	} else {
+		/* set up a few default response fields; caller may have
+		 * initialised the rest of the response */
+		test_peer.tx_buf[0] = NVME_MI_MSGTYPE_NVME;
+		test_peer.tx_buf[1] = test_peer.rx_buf[1] | (NVME_MI_ROR_RSP << 7);
+		test_set_tx_mic(&test_peer);
+	}
 
 	errno = test_peer.rx_errno;
 
@@ -191,9 +199,8 @@ static void test_read_mi_data(nvme_mi_ep_t ep, struct test_peer *peer)
 	struct nvme_mi_read_nvm_ss_info ss_info;
 	int rc;
 
-	/* empty response data, but with correct MIC */
+	/* empty response data */
 	peer->tx_buf_len = 8 + 32;
-	test_set_tx_mic(peer);
 
 	rc = nvme_mi_mi_read_mi_data_subsys(ep, &ss_info);
 	assert(rc == 0);

test/mi.c

@@ -41,9 +41,11 @@ static int test_transport_submit(struct nvme_mi_ep *ep,
 
 	assert(tpd->magic == test_transport_magic);
 
-	/* start from a zeroed response */
+	/* start from a minimal response: zeroed data, nmp to match request */
 	memset(resp->hdr, 0, resp->hdr_len);
 	memset(resp->data, 0, resp->data_len);
+	resp->hdr->type = NVME_MI_MSGTYPE_NVME;
+	resp->hdr->nmp = req->hdr->nmp | (NVME_MI_ROR_RSP << 7);
 
 	if (tpd->submit_cb)
 		return tpd->submit_cb(ep, req, resp, tpd->submit_cb_data);
@@ -417,8 +419,6 @@ static int test_admin_id_cb(struct nvme_mi_ep *ep,
 
 	/* create valid (but somewhat empty) response */
 	hdr = (__u8 *)resp->hdr;
-	memset(resp->hdr, 0, resp->hdr_len);
-	memset(resp->data, 0, resp->data_len);
 	hdr[4] = 0x00; /* status: success */
 
 	test_transport_resp_calc_mic(resp);
@@ -555,6 +555,94 @@ static void test_admin_invalid_formats(nvme_mi_ep_t ep)
 	assert(rc != 0);
 }
 
+/* test: header length too small */
+static int test_resp_hdr_small_cb(struct nvme_mi_ep *ep,
+				  struct nvme_mi_req *req,
+				  struct nvme_mi_resp *resp,
+				  void *data)
+{
+	resp->hdr_len = 2;
+	test_transport_resp_calc_mic(resp);
+	return 0;
+}
+
+static void test_resp_hdr_small(nvme_mi_ep_t ep)
+{
+	struct nvme_mi_read_nvm_ss_info ss_info;
+	int rc;
+
+	test_set_transport_callback(ep, test_resp_hdr_small_cb, NULL);
+
+	rc = nvme_mi_mi_read_mi_data_subsys(ep, &ss_info);
+	assert(rc != 0);
+}
+
+/* test: respond with a request message */
+static int test_resp_req_cb(struct nvme_mi_ep *ep,
+			    struct nvme_mi_req *req,
+			    struct nvme_mi_resp *resp,
+			    void *data)
+{
+	resp->hdr->nmp &= ~(NVME_MI_ROR_RSP << 7);
+	test_transport_resp_calc_mic(resp);
+	return 0;
+}
+
+static void test_resp_req(nvme_mi_ep_t ep)
+{
+	struct nvme_mi_read_nvm_ss_info ss_info;
+	int rc;
+
+	test_set_transport_callback(ep, test_resp_req_cb, NULL);
+
+	rc = nvme_mi_mi_read_mi_data_subsys(ep, &ss_info);
+	assert(rc != 0);
+}
+
+/* test: invalid MCTP type in response */
+static int test_resp_invalid_type_cb(struct nvme_mi_ep *ep,
+				     struct nvme_mi_req *req,
+				     struct nvme_mi_resp *resp,
+				     void *data)
+{
+	resp->hdr->type = 0x3;
+	test_transport_resp_calc_mic(resp);
+	return 0;
+}
+
+static void test_resp_invalid_type(nvme_mi_ep_t ep)
+{
+	struct nvme_mi_read_nvm_ss_info ss_info;
+	int rc;
+
+	test_set_transport_callback(ep, test_resp_invalid_type_cb, NULL);
+
+	rc = nvme_mi_mi_read_mi_data_subsys(ep, &ss_info);
+	assert(rc != 0);
+}
+
+/* test: response with mis-matching command slot */
+static int test_resp_csi_cb(struct nvme_mi_ep *ep,
+			    struct nvme_mi_req *req,
+			    struct nvme_mi_resp *resp,
+			    void *data)
+{
+	resp->hdr->nmp ^= 0x1;
+	test_transport_resp_calc_mic(resp);
+	return 0;
+}
+
+static void test_resp_csi(nvme_mi_ep_t ep)
+{
+	struct nvme_mi_read_nvm_ss_info ss_info;
+	int rc;
+
+	test_set_transport_callback(ep, test_resp_csi_cb, NULL);
+
+	rc = nvme_mi_mi_read_mi_data_subsys(ep, &ss_info);
+	assert(rc != 0);
+}
+
 #define DEFINE_TEST(name) { #name, test_ ## name }
 struct test {
 	const char *name;
@@ -570,6 +658,10 @@ struct test {
 	DEFINE_TEST(admin_id),
 	DEFINE_TEST(admin_err_resp),
 	DEFINE_TEST(admin_invalid_formats),
+	DEFINE_TEST(resp_req),
+	DEFINE_TEST(resp_hdr_small),
+	DEFINE_TEST(resp_invalid_type),
+	DEFINE_TEST(resp_csi),
 };
 
 static void run_test(struct test *test, FILE *logfd, nvme_mi_ep_t ep)