From e7c019874c248648a5be5a7bf1367784588b8a3e Mon Sep 17 00:00:00 2001 From: Nick Connolly Date: Wed, 24 Feb 2021 17:03:44 +0000 Subject: [PATCH] module/bdev/nvme: improve portability In nvme_bdev_ctrlr_create, calloc will be called with a zero size allocation request if the number of namespaces is zero. The behaviour is implementation defined if the size of the space requested is zero - calloc will either return a pointer that mustn't be dereferenced, or NULL. If NULL is returned, the nvme_bdev_ctrlr_create will fail. Only call calloc if there are a non-zero number of namespaces. Otherwise, leave the namespaces pointer with a NULL value. All references to namespaces[] are either known to be safe, or occur in the context of looping through the namespaces which will be skipped if the count is zero. The exception to this is in vbdev_opal_create, where an assert has been added to match equivalent code in bdev_ocssd_create_bdev. Tested by running unit tests on a system that returns a null pointer for a zero size allocation. Signed-off-by: Nick Connolly Change-Id: I058b0683fd9b3a20bf90e54db93ca48b9bb4e40e Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/6551 Tested-by: SPDK CI Jenkins Community-CI: Broadcom CI Community-CI: Mellanox Build Bot Reviewed-by: Jim Harris Reviewed-by: Shuhei Matsumoto Reviewed-by: sunshihao Reviewed-by: Aleksey Marchuk --- module/bdev/nvme/bdev_nvme.c | 12 +++++++----- module/bdev/nvme/vbdev_opal.c | 1 + 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/module/bdev/nvme/bdev_nvme.c b/module/bdev/nvme/bdev_nvme.c index d471b6ab7..e875b391f 100644 --- a/module/bdev/nvme/bdev_nvme.c +++ b/module/bdev/nvme/bdev_nvme.c @@ -1529,11 +1529,13 @@ nvme_bdev_ctrlr_create(struct spdk_nvme_ctrlr *ctrlr, TAILQ_INIT(&nvme_bdev_ctrlr->trids); nvme_bdev_ctrlr->num_ns = spdk_nvme_ctrlr_get_num_ns(ctrlr); - nvme_bdev_ctrlr->namespaces = calloc(nvme_bdev_ctrlr->num_ns, sizeof(struct nvme_bdev_ns *)); - if (!nvme_bdev_ctrlr->namespaces) { - SPDK_ERRLOG("Failed to allocate block namespaces pointer\n"); - rc = -ENOMEM; - goto err_alloc_namespaces; + if (nvme_bdev_ctrlr->num_ns != 0) { + nvme_bdev_ctrlr->namespaces = calloc(nvme_bdev_ctrlr->num_ns, sizeof(struct nvme_bdev_ns *)); + if (!nvme_bdev_ctrlr->namespaces) { + SPDK_ERRLOG("Failed to allocate block namespaces pointer\n"); + rc = -ENOMEM; + goto err_alloc_namespaces; + } } trid_entry = calloc(1, sizeof(*trid_entry)); diff --git a/module/bdev/nvme/vbdev_opal.c b/module/bdev/nvme/vbdev_opal.c index 1adefd89c..ce71daf37 100644 --- a/module/bdev/nvme/vbdev_opal.c +++ b/module/bdev/nvme/vbdev_opal.c @@ -356,6 +356,7 @@ vbdev_opal_create(const char *nvme_ctrlr_name, uint32_t nsid, uint8_t locking_ra opal_bdev->nvme_ctrlr = nvme_ctrlr; opal_bdev->opal_dev = nvme_ctrlr->opal_dev; + assert(nsid <= nvme_ctrlr->num_ns); nvme_bdev = nvme_bdev_ns_to_bdev(nvme_ctrlr->namespaces[nsid - 1]); assert(nvme_bdev != NULL); base_bdev_name = nvme_bdev->disk.name;