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 <jk@codeconstruct.com.au>

diff --git a/src/nvme/mi.c b/src/nvme/mi.c
index ee4713d3a1e937bb2f0639510e824cfd2a9dfc40..7cfaa8fff08f316cde74cfb3368eb3b99a877c9e 100644 (file)
--- a/src/nvme/mi.c
+++ b/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;
 }
 

diff --git a/test/mi-mctp.c b/test/mi-mctp.c
index a5c4ec62c2725dea9846daed2209e71f1046d5f5..6bdf833f4931211d2136bfea6714f1807f7bbf4a 100644 (file)
--- a/test/mi-mctp.c
+++ b/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);

diff --git a/test/mi.c b/test/mi.c
index 7ce449fe0bb87d144c8e5483df8e40b6b349522d..2cc7dde2a1904d09e58fc13547d73e0eb18fdd63 100644 (file)
--- a/test/mi.c
+++ b/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)