mi: fix get_log_page chunked offset check

In our get_log_page helper, we're incorrectly checking the requested
chunk offset against the chunk len, rather than the overall length -
this means we can't query anything but the first chunk.

This change fixes this to check against the overall length instead, and
adds an additional check to ensure we're not requesting more than the
full data size.

We also add a testcase for the chunking code.

Reported-by: Hao Jiang <jianghao@google.com>
Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>

diff --git a/src/nvme/mi.c b/src/nvme/mi.c
index eb77e2619aa2e3cdd4b4e3b5c5f2530620f5fa6a..e5ef0a0cd152c97b11c68ffbc38b875406335906 100644 (file)
--- a/src/nvme/mi.c
+++ b/src/nvme/mi.c
@@ -498,7 +498,7 @@ static int __nvme_mi_admin_get_log(nvme_mi_ctrl_t ctrl,
                return -1;
        }
 
-       if (offset < 0 || offset >= len) {
+       if (offset < 0 || offset >= args->len || offset + len > args->len) {
                errno = EINVAL;
                return -1;
        }

diff --git a/test/mi.c b/test/mi.c
index b9cffa33876fed85598933d8170bea3fbe10c1cf..c21f9dbbb541ca85d74c00a6a7f3a1f10a66c6cd 100644 (file)
--- a/test/mi.c
+++ b/test/mi.c
@@ -1699,6 +1699,86 @@ static void test_admin_sanitize_nvm(struct nvme_mi_ep *ep)
        assert(!rc);
 }
 
+/* test that we set the correct offset and size on get_log() calls that
+ * are split into multiple requests */
+struct log_data {
+       int n;
+};
+
+static int test_admin_get_log_split_cb(struct nvme_mi_ep *ep,
+                                      struct nvme_mi_req *req,
+                                      struct nvme_mi_resp *resp,
+                                      void *data)
+{
+       struct log_data *ldata = data;
+       uint32_t len, off;
+       __u8 *rq_hdr;
+
+       assert(req->data_len == 0);
+
+       rq_hdr = (__u8 *)req->hdr;
+
+       assert(rq_hdr[4] == nvme_admin_get_log_page);
+
+       /* from the MI message's DOFST/DLEN fields */
+       off = rq_hdr[31] << 24 | rq_hdr[30] << 16 | rq_hdr[29] << 8 | rq_hdr[28];
+       len = rq_hdr[35] << 24 | rq_hdr[34] << 16 | rq_hdr[33] << 8 | rq_hdr[32];
+
+       /* we should have a full-sized start and middle, and a short end */
+       switch (ldata->n) {
+       case 0:
+               assert(len == 4096);
+               assert(off == 0);
+               break;
+       case 1:
+               assert(len == 4096);
+               assert(off == 4096);
+               break;
+       case 2:
+               assert(len == 4);
+               assert(off == 4096 * 2);
+               break;
+       default:
+               assert(0);
+       }
+
+       /* ensure we've sized the expected response correctly */
+       assert(resp->data_len == len);
+       memset(resp->data, ldata->n & 0xff, len);
+
+       test_transport_resp_calc_mic(resp);
+
+       ldata->n++;
+
+       return 0;
+}
+
+static void test_admin_get_log_split(struct nvme_mi_ep *ep)
+{
+       unsigned char buf[4096 * 2 + 4];
+       struct nvme_get_log_args args;
+       struct log_data ldata;
+       nvme_mi_ctrl_t ctrl;
+       int rc;
+
+       ldata.n = 0;
+       test_set_transport_callback(ep, test_admin_get_log_split_cb, &ldata);
+
+       ctrl = nvme_mi_init_ctrl(ep, 5);
+
+       args.args_size = sizeof(args);
+       args.lid = 1;
+       args.log = buf;
+       args.len = sizeof(buf);
+
+       rc = nvme_mi_admin_get_log(ctrl, &args);
+
+       assert(!rc);
+
+       /* we should have sent three commands */
+       assert(ldata.n == 3);
+}
+
 #define DEFINE_TEST(name) { #name, test_ ## name }
 struct test {
        const char *name;
@@ -1738,6 +1818,7 @@ struct test {
        DEFINE_TEST(admin_fw_commit),
        DEFINE_TEST(admin_format_nvm),
        DEFINE_TEST(admin_sanitize_nvm),
+       DEFINE_TEST(admin_get_log_split),
 };
 
 static void run_test(struct test *test, FILE *logfd, nvme_mi_ep_t ep)