From 3921756dee6dd7beb7b60167f368e8b981c77365 Mon Sep 17 00:00:00 2001 From: Klaus Jensen Date: Mon, 15 Mar 2021 08:41:41 +0100 Subject: [PATCH] hw/block/nvme: assert namespaces array indices MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Coverity complains about a possible memory corruption in the nvme_ns_attach and _detach functions. While we should not (famous last words) be able to reach this function without nsid having previously been validated, this is still an open door for future misuse. Make Coverity and maintainers happy by asserting that the index into the array is valid. Also, while not detected by Coverity (yet), add an assert in nvme_subsys_ns and nvme_subsys_register_ns as well since a similar issue is exists there. Fixes: 037953b5b299 ("hw/block/nvme: support namespace detach") Fixes: CID 1450757 Fixes: CID 1450758 Cc: Minwoo Im Signed-off-by: Klaus Jensen Reviewed-by: Philippe Mathieu-Daudé --- hw/block/nvme-subsys.c | 7 +++++-- hw/block/nvme-subsys.h | 2 ++ hw/block/nvme.h | 10 ++++++++-- 3 files changed, 15 insertions(+), 4 deletions(-) diff --git a/hw/block/nvme-subsys.c b/hw/block/nvme-subsys.c index af4804a819..9fadef8cec 100644 --- a/hw/block/nvme-subsys.c +++ b/hw/block/nvme-subsys.c @@ -47,15 +47,18 @@ int nvme_subsys_register_ns(NvmeNamespace *ns, Error **errp) { NvmeSubsystem *subsys = ns->subsys; NvmeCtrl *n; + uint32_t nsid = nvme_nsid(ns); int i; - if (subsys->namespaces[nvme_nsid(ns)]) { + assert(nsid && nsid <= NVME_SUBSYS_MAX_NAMESPACES); + + if (subsys->namespaces[nsid]) { error_setg(errp, "namespace %d already registerd to subsy %s", nvme_nsid(ns), subsys->parent_obj.id); return -1; } - subsys->namespaces[nvme_nsid(ns)] = ns; + subsys->namespaces[nsid] = ns; for (i = 0; i < ARRAY_SIZE(subsys->ctrls); i++) { n = subsys->ctrls[i]; diff --git a/hw/block/nvme-subsys.h b/hw/block/nvme-subsys.h index fb66ae752a..aafa04b848 100644 --- a/hw/block/nvme-subsys.h +++ b/hw/block/nvme-subsys.h @@ -54,6 +54,8 @@ static inline NvmeNamespace *nvme_subsys_ns(NvmeSubsystem *subsys, return NULL; } + assert(nsid && nsid <= NVME_SUBSYS_MAX_NAMESPACES); + return subsys->namespaces[nsid]; } diff --git a/hw/block/nvme.h b/hw/block/nvme.h index 4955d649c7..5ba2efaedf 100644 --- a/hw/block/nvme.h +++ b/hw/block/nvme.h @@ -236,12 +236,18 @@ static inline bool nvme_ns_is_attached(NvmeCtrl *n, NvmeNamespace *ns) static inline void nvme_ns_attach(NvmeCtrl *n, NvmeNamespace *ns) { - n->namespaces[nvme_nsid(ns) - 1] = ns; + uint32_t nsid = nvme_nsid(ns); + assert(nsid && nsid <= NVME_MAX_NAMESPACES); + + n->namespaces[nsid - 1] = ns; } static inline void nvme_ns_detach(NvmeCtrl *n, NvmeNamespace *ns) { - n->namespaces[nvme_nsid(ns) - 1] = NULL; + uint32_t nsid = nvme_nsid(ns); + assert(nsid && nsid <= NVME_MAX_NAMESPACES); + + n->namespaces[nsid - 1] = NULL; } static inline NvmeCQueue *nvme_cq(NvmeRequest *req) -- 2.11.0