Skip to content

Commit 275cb31

Browse files
committed
mi: be pedantic with message sizes and offsets
Check that we have enough data for a base header, and that the lengths are properly aligned, for both request and response messages. For the raw admin API, ensure we have correct data lengths and offsets. Add some tests to suit, using the raw admin API Signed-off-by: Jeremy Kerr <[email protected]>
1 parent 7155c48 commit 275cb31

2 files changed

Lines changed: 99 additions & 1 deletion

File tree

src/nvme/mi.c

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,24 @@ int nvme_mi_submit(nvme_mi_ep_t ep, struct nvme_mi_req *req,
157157
{
158158
int rc;
159159

160+
if (req->hdr_len < sizeof(struct nvme_mi_msg_hdr))
161+
return -EINVAL;
162+
163+
if (req->hdr_len & 0x3)
164+
return -EINVAL;
165+
166+
if (req->data_len & 0x3)
167+
return -EINVAL;
168+
169+
if (resp->hdr_len < sizeof(struct nvme_mi_msg_hdr))
170+
return -EINVAL;
171+
172+
if (resp->hdr_len & 0x3)
173+
return -EINVAL;
174+
175+
if (resp->data_len & 0x3)
176+
return -EINVAL;
177+
160178
if (ep->transport->mic_enabled)
161179
nvme_mi_calc_req_mic(req);
162180

@@ -213,11 +231,30 @@ int nvme_mi_admin_xfer(nvme_mi_ctrl_t ctrl,
213231
struct nvme_mi_req req;
214232
int rc;
215233

216-
if (*resp_data_size > 0xffffffff)
234+
/* length/offset checks. The common _submit() API will do further
235+
* checking on the message lengths too, so these are kept specific
236+
* to the requirements of the Admin command set
237+
*/
238+
239+
/* NVMe-MI v1.2 imposes a limit of 4096 bytes on the dlen field */
240+
if (*resp_data_size > 4096)
217241
return -EINVAL;
242+
243+
/* we only have 32 bits of offset */
218244
if (resp_data_offset > 0xffffffff)
219245
return -EINVAL;
220246

247+
/* must be aligned */
248+
if (resp_data_offset & 0x3)
249+
return -EINVAL;
250+
251+
/* bidirectional not permitted (see DLEN definition) */
252+
if (req_data_size && *resp_data_size)
253+
return -EINVAL;
254+
255+
if (!*resp_data_size && resp_data_offset)
256+
return -EINVAL;
257+
221258
admin_req->hdr.type = NVME_MI_MSGTYPE_NVME;
222259
admin_req->hdr.nmp = (NVME_MI_ROR_REQ << 7) |
223260
(NVME_MI_MT_ADMIN << 3);

test/mi.c

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -495,6 +495,66 @@ static void test_admin_err_resp(nvme_mi_ep_t ep)
495495
assert(rc != 0);
496496
}
497497

498+
/* invalid Admin command transfers */
499+
static int test_admin_invalid_formats_cb(struct nvme_mi_ep *ep,
500+
struct nvme_mi_req *req,
501+
struct nvme_mi_resp *resp,
502+
void *data)
503+
{
504+
/* none of the tests should result in message transfer */
505+
assert(0);
506+
return -1;
507+
}
508+
509+
static void test_admin_invalid_formats(nvme_mi_ep_t ep)
510+
{
511+
struct nvme_mi_admin_resp_hdr resp = { 0 };
512+
struct nvme_mi_admin_req_hdr req = { 0 };
513+
nvme_mi_ctrl_t ctrl;
514+
size_t len;
515+
int rc;
516+
517+
test_set_transport_callback(ep, test_admin_invalid_formats_cb, NULL);
518+
519+
ctrl = nvme_mi_init_ctrl(ep, 1);
520+
assert(ctrl);
521+
522+
/* unaligned req size */
523+
len = 0;
524+
rc = nvme_mi_admin_xfer(ctrl, &req, 1, &resp, 0, &len);
525+
assert(rc != 0);
526+
527+
/* unaligned resp size */
528+
len = 1;
529+
rc = nvme_mi_admin_xfer(ctrl, &req, 0, &resp, 0, &len);
530+
assert(rc != 0);
531+
532+
/* unaligned resp offset */
533+
len = 4;
534+
rc = nvme_mi_admin_xfer(ctrl, &req, 0, &resp, 1, &len);
535+
assert(rc != 0);
536+
537+
/* resp too large */
538+
len = 4096 + 4;
539+
rc = nvme_mi_admin_xfer(ctrl, &req, 0, &resp, 0, &len);
540+
assert(rc != 0);
541+
542+
/* resp offset too large */
543+
len = 4;
544+
rc = nvme_mi_admin_xfer(ctrl, &req, 0, &resp, (off_t)1 << 32, &len);
545+
assert(rc != 0);
546+
547+
/* resp offset with no len */
548+
len = 0;
549+
rc = nvme_mi_admin_xfer(ctrl, &req, 0, &resp, 4, &len);
550+
assert(rc != 0);
551+
552+
/* req and resp payloads */
553+
len = 4;
554+
rc = nvme_mi_admin_xfer(ctrl, &req, 4, &resp, 0, &len);
555+
assert(rc != 0);
556+
}
557+
498558
#define DEFINE_TEST(name) { #name, test_ ## name }
499559
struct test {
500560
const char *name;
@@ -509,6 +569,7 @@ struct test {
509569
DEFINE_TEST(invalid_crc),
510570
DEFINE_TEST(admin_id),
511571
DEFINE_TEST(admin_err_resp),
572+
DEFINE_TEST(admin_invalid_formats),
512573
};
513574

514575
static void run_test(struct test *test, FILE *logfd, nvme_mi_ep_t ep)

0 commit comments

Comments
 (0)