From a7b6702d6f5211099c7102b98db845d6965bcc33 Mon Sep 17 00:00:00 2001 From: Tomasz Kulasek Date: Tue, 10 Dec 2019 08:57:12 -0500 Subject: [PATCH] lib/nvme: fix return -EINVAL for invalid io_flags Previously, invalid io_flags would results in -ENOMEM being returned to the user which was incorrect. Change-Id: I53dd0fa8684cb36f3d124baa92244e2ed30e2527 Signed-off-by: Tomasz Kulasek Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/476938 Reviewed-by: Jim Harris Reviewed-by: Tomasz Zawadzki Reviewed-by: Paul Luse Reviewed-by: Shuhei Matsumoto Tested-by: SPDK CI Jenkins Community-CI: SPDK CI Jenkins --- include/spdk/nvme_spec.h | 3 ++ lib/nvme/nvme_ns_cmd.c | 69 ++++++++++++++++++++++++++++++++++++---- 2 files changed, 65 insertions(+), 7 deletions(-) diff --git a/include/spdk/nvme_spec.h b/include/spdk/nvme_spec.h index eb4a8a100..f4071f98f 100644 --- a/include/spdk/nvme_spec.h +++ b/include/spdk/nvme_spec.h @@ -2826,6 +2826,9 @@ SPDK_STATIC_ASSERT(sizeof(struct spdk_nvme_fw_commit) == 4, "Incorrect size"); #define SPDK_NVME_IO_FLAGS_FORCE_UNIT_ACCESS (1U << 30) #define SPDK_NVME_IO_FLAGS_LIMITED_RETRY (1U << 31) +/** Mask of valid io flags mask */ +#define SPDK_NVME_IO_FLAGS_VALID_MASK 0xFFFF0000 + #ifdef __cplusplus } #endif diff --git a/lib/nvme/nvme_ns_cmd.c b/lib/nvme/nvme_ns_cmd.c index 598fe4ef6..a059c0c9b 100644 --- a/lib/nvme/nvme_ns_cmd.c +++ b/lib/nvme/nvme_ns_cmd.c @@ -129,6 +129,18 @@ _nvme_ns_cmd_split_request(struct spdk_nvme_ns *ns, return req; } +static inline bool +_is_io_flags_valid(uint32_t io_flags) +{ + if (io_flags & ~SPDK_NVME_IO_FLAGS_VALID_MASK) { + /* Invalid io_flags */ + SPDK_ERRLOG("Invalid io_flags 0x%x\n", io_flags); + return false; + } + + return true; +} + static void _nvme_ns_cmd_setup_request(struct spdk_nvme_ns *ns, struct nvme_request *req, uint32_t opc, uint64_t lba, uint32_t lba_count, @@ -136,6 +148,8 @@ _nvme_ns_cmd_setup_request(struct spdk_nvme_ns *ns, struct nvme_request *req, { struct spdk_nvme_cmd *cmd; + assert(_is_io_flags_valid(io_flags)); + cmd = &req->cmd; cmd->opc = opc; cmd->nsid = ns->id; @@ -375,13 +389,6 @@ _nvme_ns_cmd_rw(struct spdk_nvme_ns *ns, struct spdk_nvme_qpair *qpair, uint32_t sectors_per_max_io; uint32_t sectors_per_stripe; - if (io_flags & 0xFFFF) { - /* The bottom 16 bits must be empty */ - SPDK_ERRLOG("io_flags 0x%x bottom 16 bits is not empty\n", - io_flags); - return NULL; - } - sector_size = ns->extended_lba_size; sectors_per_max_io = ns->sectors_per_max_io; sectors_per_stripe = ns->sectors_per_stripe; @@ -444,6 +451,10 @@ spdk_nvme_ns_cmd_compare(struct spdk_nvme_ns *ns, struct spdk_nvme_qpair *qpair, struct nvme_request *req; struct nvme_payload payload; + if (!_is_io_flags_valid(io_flags)) { + return -EINVAL; + } + payload = NVME_PAYLOAD_CONTIG(buffer, NULL); req = _nvme_ns_cmd_rw(ns, qpair, &payload, 0, 0, lba, lba_count, cb_fn, cb_arg, @@ -473,6 +484,10 @@ spdk_nvme_ns_cmd_compare_with_md(struct spdk_nvme_ns *ns, struct spdk_nvme_qpair struct nvme_request *req; struct nvme_payload payload; + if (!_is_io_flags_valid(io_flags)) { + return -EINVAL; + } + payload = NVME_PAYLOAD_CONTIG(buffer, metadata); req = _nvme_ns_cmd_rw(ns, qpair, &payload, 0, 0, lba, lba_count, cb_fn, cb_arg, @@ -501,6 +516,10 @@ spdk_nvme_ns_cmd_comparev(struct spdk_nvme_ns *ns, struct spdk_nvme_qpair *qpair struct nvme_request *req; struct nvme_payload payload; + if (!_is_io_flags_valid(io_flags)) { + return -EINVAL; + } + if (reset_sgl_fn == NULL || next_sge_fn == NULL) { return -EINVAL; } @@ -531,6 +550,10 @@ spdk_nvme_ns_cmd_read(struct spdk_nvme_ns *ns, struct spdk_nvme_qpair *qpair, vo struct nvme_request *req; struct nvme_payload payload; + if (!_is_io_flags_valid(io_flags)) { + return -EINVAL; + } + payload = NVME_PAYLOAD_CONTIG(buffer, NULL); req = _nvme_ns_cmd_rw(ns, qpair, &payload, 0, 0, lba, lba_count, cb_fn, cb_arg, SPDK_NVME_OPC_READ, @@ -558,6 +581,10 @@ spdk_nvme_ns_cmd_read_with_md(struct spdk_nvme_ns *ns, struct spdk_nvme_qpair *q struct nvme_request *req; struct nvme_payload payload; + if (!_is_io_flags_valid(io_flags)) { + return -EINVAL; + } + payload = NVME_PAYLOAD_CONTIG(buffer, metadata); req = _nvme_ns_cmd_rw(ns, qpair, &payload, 0, 0, lba, lba_count, cb_fn, cb_arg, SPDK_NVME_OPC_READ, @@ -585,6 +612,10 @@ spdk_nvme_ns_cmd_readv(struct spdk_nvme_ns *ns, struct spdk_nvme_qpair *qpair, struct nvme_request *req; struct nvme_payload payload; + if (!_is_io_flags_valid(io_flags)) { + return -EINVAL; + } + if (reset_sgl_fn == NULL || next_sge_fn == NULL) { return -EINVAL; } @@ -616,6 +647,10 @@ spdk_nvme_ns_cmd_readv_with_md(struct spdk_nvme_ns *ns, struct spdk_nvme_qpair * struct nvme_request *req; struct nvme_payload payload; + if (!_is_io_flags_valid(io_flags)) { + return -EINVAL; + } + if (reset_sgl_fn == NULL || next_sge_fn == NULL) { return -EINVAL; } @@ -645,6 +680,10 @@ spdk_nvme_ns_cmd_write(struct spdk_nvme_ns *ns, struct spdk_nvme_qpair *qpair, struct nvme_request *req; struct nvme_payload payload; + if (!_is_io_flags_valid(io_flags)) { + return -EINVAL; + } + payload = NVME_PAYLOAD_CONTIG(buffer, NULL); req = _nvme_ns_cmd_rw(ns, qpair, &payload, 0, 0, lba, lba_count, cb_fn, cb_arg, SPDK_NVME_OPC_WRITE, @@ -670,6 +709,10 @@ spdk_nvme_ns_cmd_write_with_md(struct spdk_nvme_ns *ns, struct spdk_nvme_qpair * struct nvme_request *req; struct nvme_payload payload; + if (!_is_io_flags_valid(io_flags)) { + return -EINVAL; + } + payload = NVME_PAYLOAD_CONTIG(buffer, metadata); req = _nvme_ns_cmd_rw(ns, qpair, &payload, 0, 0, lba, lba_count, cb_fn, cb_arg, SPDK_NVME_OPC_WRITE, @@ -696,6 +739,10 @@ spdk_nvme_ns_cmd_writev(struct spdk_nvme_ns *ns, struct spdk_nvme_qpair *qpair, struct nvme_request *req; struct nvme_payload payload; + if (!_is_io_flags_valid(io_flags)) { + return -EINVAL; + } + if (reset_sgl_fn == NULL || next_sge_fn == NULL) { return -EINVAL; } @@ -727,6 +774,10 @@ spdk_nvme_ns_cmd_writev_with_md(struct spdk_nvme_ns *ns, struct spdk_nvme_qpair struct nvme_request *req; struct nvme_payload payload; + if (!_is_io_flags_valid(io_flags)) { + return -EINVAL; + } + if (reset_sgl_fn == NULL || next_sge_fn == NULL) { return -EINVAL; } @@ -757,6 +808,10 @@ spdk_nvme_ns_cmd_write_zeroes(struct spdk_nvme_ns *ns, struct spdk_nvme_qpair *q struct spdk_nvme_cmd *cmd; uint64_t *tmp_lba; + if (!_is_io_flags_valid(io_flags)) { + return -EINVAL; + } + if (lba_count == 0 || lba_count > UINT16_MAX + 1) { return -EINVAL; }