From: Jesper Wendel Devantier <j.devantier@samsung.com>

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.

Reported-by: jaeyeong <fin@spl.team>
Signed-off-by: Jesper Wendel Devantier <foss@defmacro.it>
---
Adresses issues raised by Jaeyeong (thanks!) regarding the
handling of the user-supplied list of event types to track.

As reported, the event mask is much too small to cover the legal
range of 256 values (0-255) and would fail even for event types
0x80 and 0x81 which caused shifts of 32 and 33 places - beyond
the limit of the u8 event mask.

Secondarily, the report also pointed out that unsupported event
types would cause a lookup in the bit-shift table to return 0,
meaning unsupported events would effectively toggle the
tracking of the event type 0x0 (RU not fully written).

This patch skips processing of unsupported event types,
increases the event filter mask size to accommodate currently
supported event types and documents behavior in the lookup
table.

Signed-off-by: Jesper Wendel Devantier <j.devantier@samsung.com>
---
 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 815f39173c..d60a680dbc 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 5ef3ebee29..9de9f347c5 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,

---
base-commit: 91190a4303223c7971856f6b37f221cc91c62689
change-id: 20260519-patch-review-20260518131523-654-6799b6319f8e

Best regards,
-- 
Jesper Wendel Devantier <j.devantier@samsung.com>

On May 20 09:35, Jesper Devantier wrote:
> From: Jesper Wendel Devantier <j.devantier@samsung.com>
> 
> 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.
> 
> Reported-by: jaeyeong <fin@spl.team>
> Signed-off-by: Jesper Wendel Devantier <foss@defmacro.it>
> ---
> Adresses issues raised by Jaeyeong (thanks!) regarding the
> handling of the user-supplied list of event types to track.
> 
> As reported, the event mask is much too small to cover the legal
> range of 256 values (0-255) and would fail even for event types
> 0x80 and 0x81 which caused shifts of 32 and 33 places - beyond
> the limit of the u8 event mask.
> 
> Secondarily, the report also pointed out that unsupported event
> types would cause a lookup in the bit-shift table to return 0,
> meaning unsupported events would effectively toggle the
> tracking of the event type 0x0 (RU not fully written).
> 
> This patch skips processing of unsupported event types,
> increases the event filter mask size to accommodate currently
> supported event types and documents behavior in the lookup
> table.
> 
> Signed-off-by: Jesper Wendel Devantier <j.devantier@samsung.com>

Reviewed-by: Klaus Jensen <k.jensen@samsung.com>

> ---
>  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 815f39173c..d60a680dbc 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 5ef3ebee29..9de9f347c5 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,
> 
> ---
> base-commit: 91190a4303223c7971856f6b37f221cc91c62689
> change-id: 20260519-patch-review-20260518131523-654-6799b6319f8e
> 
> Best regards,
> -- 
> Jesper Wendel Devantier <j.devantier@samsung.com>
> 
>