hw/block/nvme: assert namespaces array indices

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 <minwoo.im.dev@gmail.com>
Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

diff --git a/hw/block/nvme-subsys.c b/hw/block/nvme-subsys.c
index af4804a819eed34bd0d15943f0e7c3dd58757746..9fadef8cec99da8e5eafa1ba8649d04c0444fe7b 100644 (file)
--- 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 fb66ae752ad515200976aff6a59dda061c9a3e83..aafa04b84829cd4098563e662251f84b9a81313c 100644 (file)
--- 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 4955d649c7d49329f1812b77e9aa5f6a90da96fb..5ba2efaedfd2f7e6fb7e6788ad40bddb04ccfc21 100644 (file)
--- 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)