From: Jesper Wendel Devantier <[email protected]>
Addresses an issue reported whereby user-provided event type values
could trigger two issues:
1. if provided event_type == 0xff -> out-of-bounds access
2. if provided event_type > 7 -> generate a value too large for the
u8 event mask.
This patch fixes (1) by correctly adjusting the length of the look-up
array to be 256 values.
This patch fixes (2) by:
a. changing the event_type mask to 64bit, matching
NvmeRuHandle.event_filter
b. Matching the behavior of Get Feature - FDP Events by skipping
event type values which we do not support.
5.2.26.1.21 of the 2.3 Base specification does not explicitly
tell us to reject unsupported event type values.
c. Documenting in the event type lookup table, that supporting
event types greater than 63 requires refactoring the masking
code.
Cc: [email protected]
Reported-by: jaeyeong <[email protected]>
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3631
Signed-off-by: Jesper Wendel Devantier <[email protected]>
Reviewed-by: Klaus Jensen <[email protected]>
Signed-off-by: Klaus Jensen <[email protected]>
---
hw/nvme/ctrl.c | 22 ++++++++++++++++------
hw/nvme/nvme.h | 9 ++++++++-
2 files changed, 24 insertions(+), 7 deletions(-)
diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index 815f39173c8a..d60a680dbc15 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -6244,10 +6244,6 @@ static uint16_t nvme_get_feature_fdp_events(NvmeCtrl *n,
NvmeNamespace *ns,
for (uint8_t event_type = 0; event_type < FDP_EVT_MAX; event_type++) {
uint8_t shift = nvme_fdp_evf_shifts[event_type];
if (!shift && event_type) {
- /*
- * only first entry (event_type == 0) has a shift value of 0
- * other entries are simply unpopulated.
- */
continue;
}
@@ -6492,9 +6488,9 @@ static uint16_t nvme_set_feature_fdp_events(NvmeCtrl *n,
NvmeNamespace *ns,
uint8_t noet = (cdw11 >> 16) & 0xff;
uint16_t ret, ruhid;
uint8_t enable = le32_to_cpu(cmd->cdw12) & 0x1;
- uint8_t event_mask = 0;
+ uint64_t event_mask = 0;
unsigned int i;
- g_autofree uint8_t *events = g_malloc0(noet);
+ g_autofree uint8_t *events = NULL;
NvmeRuHandle *ruh = NULL;
assert(ns);
@@ -6507,15 +6503,29 @@ static uint16_t nvme_set_feature_fdp_events(NvmeCtrl
*n, NvmeNamespace *ns,
return NVME_INVALID_FIELD | NVME_DNR;
}
+ if (unlikely(noet == 0)) {
+ return NVME_SUCCESS;
+ }
+
ruhid = ns->fdp.phs[ph];
ruh = &n->subsys->endgrp.fdp.ruhs[ruhid];
+ events = g_malloc0(noet);
+
ret = nvme_h2c(n, events, noet, req);
if (ret) {
return ret;
}
for (i = 0; i < noet; i++) {
+ /*
+ * We ignore requests to enable tracking of unsupported FDP event types
+ */
+ uint8_t event_type = events[i];
+ uint8_t shift = nvme_fdp_evf_shifts[event_type];
+ if (!shift && event_type) {
+ continue;
+ }
event_mask |= (1 << nvme_fdp_evf_shifts[events[i]]);
}
diff --git a/hw/nvme/nvme.h b/hw/nvme/nvme.h
index 5ef3ebee29e5..9de9f347c542 100644
--- a/hw/nvme/nvme.h
+++ b/hw/nvme/nvme.h
@@ -160,7 +160,14 @@ typedef struct NvmeZone {
#define NVME_FDP_MAX_NS_RUHS 32u
#define FDPVSS 0
-static const uint8_t nvme_fdp_evf_shifts[FDP_EVT_MAX] = {
+/*
+ * NOTE: Apart from event type 0, any event type with a shift value of 0 is
+ * considered unsupported and thus skipped in get/set features calls.
+ *
+ * NOTE: NvmeRuHandle uses a 64bit event mask - refactor to support event types
+ * of 63 or greater.
+ */
+static const uint8_t nvme_fdp_evf_shifts[FDP_EVT_MAX + 1] = {
/* Host events */
[FDP_EVT_RU_NOT_FULLY_WRITTEN] = 0,
[FDP_EVT_RU_ATL_EXCEEDED] = 1,
--
2.53.0
