From 091aa2b6810ec9c534d2e439ee09abb9c07913a4 Mon Sep 17 00:00:00 2001 From: Changpeng Liu Date: Sun, 26 Sep 2021 17:13:19 +0800 Subject: [PATCH] nvmf/vfio-user: fix potential overflow for qsize/MQES/NLB The spec treats the sizes (MQES or qsize from create/delete IO queue command) as a 0-based value of uint16_t, but vfio-user treats them as 1-based value, so we need to use uint32_t to make sure the value can't overflow. The same for NLB(number of logical blocks). Change-Id: I7654b7e12234525c0fce78a713dd50097e9b3d58 Signed-off-by: Changpeng Liu Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/9632 Tested-by: SPDK CI Jenkins Community-CI: Broadcom CI Community-CI: Mellanox Build Bot Reviewed-by: Ben Walker Reviewed-by: Jim Harris --- lib/nvmf/vfio_user.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/lib/nvmf/vfio_user.c b/lib/nvmf/vfio_user.c index b79e49e84..02fb4a176 100644 --- a/lib/nvmf/vfio_user.c +++ b/lib/nvmf/vfio_user.c @@ -140,7 +140,7 @@ struct nvmf_vfio_user_qpair { struct spdk_nvmf_transport_poll_group *group; struct nvmf_vfio_user_ctrlr *ctrlr; struct nvmf_vfio_user_req *reqs_internal; - uint16_t qsize; + uint32_t qsize; struct nvme_q cq; struct nvme_q sq; enum nvmf_vfio_user_qpair_state state; @@ -589,7 +589,7 @@ err: return NULL; } -static uint16_t +static uint32_t max_queue_size(struct nvmf_vfio_user_ctrlr const *ctrlr) { assert(ctrlr != NULL); @@ -1016,9 +1016,9 @@ free_qp(struct nvmf_vfio_user_ctrlr *ctrlr, uint16_t qid) /* This function can only fail because of memory allocation errors. */ static int init_qp(struct nvmf_vfio_user_ctrlr *ctrlr, struct spdk_nvmf_transport *transport, - const uint16_t qsize, const uint16_t id) + const uint32_t qsize, const uint16_t id) { - uint16_t i; + uint32_t i; struct nvmf_vfio_user_qpair *qpair; struct nvmf_vfio_user_req *vu_req, *tmp; struct spdk_nvmf_request *req; @@ -1094,7 +1094,8 @@ static int handle_create_io_q(struct nvmf_vfio_user_ctrlr *ctrlr, struct spdk_nvme_cmd *cmd, const bool is_cq) { - uint16_t qid, qsize; + uint16_t qid; + uint32_t qsize; uint16_t sc = SPDK_NVME_SC_SUCCESS; uint16_t sct = SPDK_NVME_SCT_GENERIC; int err = 0; @@ -1123,7 +1124,7 @@ handle_create_io_q(struct nvmf_vfio_user_ctrlr *ctrlr, qsize = cmd->cdw10_bits.create_io_q.qsize + 1; if (qsize > max_queue_size(ctrlr)) { - SPDK_ERRLOG("%s: queue too big, want=%d, max=%d\n", ctrlr_id(ctrlr), + SPDK_ERRLOG("%s: queue too big, want=%u, max=%u\n", ctrlr_id(ctrlr), qsize, max_queue_size(ctrlr)); sct = SPDK_NVME_SCT_COMMAND_SPECIFIC; sc = SPDK_NVME_SC_INVALID_QUEUE_SIZE; @@ -2580,8 +2581,8 @@ get_nvmf_req(struct nvmf_vfio_user_qpair *qpair) static int get_nvmf_io_req_length(struct spdk_nvmf_request *req) { - uint16_t nlb, nr; - uint32_t nsid; + uint16_t nr; + uint32_t nlb, nsid; struct spdk_nvme_cmd *cmd = &req->cmd->nvme_cmd; struct spdk_nvmf_ctrlr *ctrlr = req->qpair->ctrlr; struct spdk_nvmf_ns *ns; @@ -2882,7 +2883,8 @@ nvmf_vfio_user_qpair_abort_request(struct spdk_nvmf_qpair *qpair, { struct nvmf_vfio_user_qpair *vu_qpair; struct nvmf_vfio_user_req *vu_req, *vu_req_to_abort = NULL; - uint16_t i, cid; + uint32_t i; + uint16_t cid; vu_qpair = SPDK_CONTAINEROF(qpair, struct nvmf_vfio_user_qpair, qpair);