From: Han Zhang <ihanzh@outlook.com>
Static analysis reported that the default branches in
nvme_directive_receive() were unreachable because dtype and doper were
validated before the switch statements.
Keep the switch statements as explicit documentation of supported
directive type/operation combinations, and remove the redundant early
dtype/doper checks instead.
Also move namespace lookup into the RETURN_PARAMS path so invalid
dtype/doper values are rejected via switch defaults without an
unnecessary nvme_ns() lookup.
Reported-by: Ekaterina Zilotina
Fixes: https://gitlab.com/qemu-project/qemu/-/issues/2472
Signed-off-by: Han Zhang <ihanzhzh@gmail.com>
---
hw/nvme/ctrl.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index cc4593cd42..479a4b8725 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -7540,13 +7540,7 @@ static uint16_t nvme_directive_receive(NvmeCtrl *n, NvmeRequest *req)
trans_len = MIN(sizeof(NvmeDirectiveIdentify), numd << 2);
- if (nsid == NVME_NSID_BROADCAST || dtype != NVME_DIRECTIVE_IDENTIFY ||
- doper != NVME_DIRECTIVE_RETURN_PARAMS) {
- return NVME_INVALID_FIELD | NVME_DNR;
- }
-
- ns = nvme_ns(n, nsid);
- if (!ns) {
+ if (nsid == NVME_NSID_BROADCAST) {
return NVME_INVALID_FIELD | NVME_DNR;
}
@@ -7554,6 +7548,11 @@ static uint16_t nvme_directive_receive(NvmeCtrl *n, NvmeRequest *req)
case NVME_DIRECTIVE_IDENTIFY:
switch (doper) {
case NVME_DIRECTIVE_RETURN_PARAMS:
+ ns = nvme_ns(n, nsid);
+ if (!ns) {
+ return NVME_INVALID_FIELD | NVME_DNR;
+ }
+
if (ns->endgrp && ns->endgrp->fdp.enabled) {
id.supported |= 1 << NVME_DIRECTIVE_DATA_PLACEMENT;
id.enabled |= 1 << NVME_DIRECTIVE_DATA_PLACEMENT;
@@ -7567,7 +7566,7 @@ static uint16_t nvme_directive_receive(NvmeCtrl *n, NvmeRequest *req)
}
default:
- return NVME_INVALID_FIELD;
+ return NVME_INVALID_FIELD | NVME_DNR;
}
}
--
2.34.1