All of lore.kernel.org
 help / color / mirror / Atom feed
From: Klaus Jensen <its@irrelevant.dk>
To: qemu-devel@nongnu.org
Cc: Peter Maydell <peter.maydell@linaro.org>,
	Jesper Wendel Devantier <j.devantier@samsung.com>,
	qemu-stable@nongnu.org, jaeyeong <fin@spl.team>,
	Jesper Wendel Devantier <foss@defmacro.it>,
	Klaus Jensen <k.jensen@samsung.com>,
	Keith Busch <kbusch@kernel.org>, Klaus Jensen <its@irrelevant.dk>,
	qemu-block@nongnu.org
Subject: [PULL 01/11] hw/nvme: fix FDP set FDP events
Date: Tue,  7 Jul 2026 00:44:13 +0200	[thread overview]
Message-ID: <20260706224426.14156-2-its@irrelevant.dk> (raw)
In-Reply-To: <20260706224426.14156-1-its@irrelevant.dk>

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.

Cc: qemu-stable@nongnu.org
Reported-by: jaeyeong <fin@spl.team>
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3631
Signed-off-by: Jesper Wendel Devantier <foss@defmacro.it>
Reviewed-by: Klaus Jensen <k.jensen@samsung.com>
Signed-off-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 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



  reply	other threads:[~2026-07-06 22:45 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-06 22:44 [PULL 00/11] hw/nvme queue Klaus Jensen
2026-07-06 22:44 ` Klaus Jensen [this message]
2026-07-08  9:37   ` [PULL 01/11] hw/nvme: fix FDP set FDP events Peter Maydell
2026-07-06 22:44 ` [PULL 02/11] hw/nvme: ensure sgl forward progress Klaus Jensen
2026-07-06 22:44 ` [PULL 03/11] tests/functional/migration: add VM launch/configure hooks Klaus Jensen
2026-07-06 22:44 ` [PULL 04/11] hw/nvme: add migration blockers for non-supported cases Klaus Jensen
2026-07-08 10:11   ` Peter Maydell
2026-07-08 10:18     ` Peter Maydell
2026-07-08 10:23     ` Alexander Mikhalitsyn
2026-07-08 10:27       ` Klaus Jensen
2026-07-08 10:32         ` Alexander Mikhalitsyn
2026-07-06 22:44 ` [PULL 05/11] hw/nvme: split nvme_init_sq/nvme_init_cq into helpers Klaus Jensen
2026-07-06 22:44 ` [PULL 06/11] hw/nvme: set CQE.sq_id earlier in nvme_process_sq Klaus Jensen
2026-07-06 22:44 ` [PULL 07/11] hw/nvme: unmap req->sg earlier in nvme_enqueue_req_completion Klaus Jensen
2026-07-06 22:44 ` [PULL 08/11] hw/nvme: add basic live migration support Klaus Jensen
2026-07-06 22:44 ` [PULL 09/11] tests/functional/x86_64: add migration test for NVMe device Klaus Jensen
2026-07-06 22:44 ` [PULL 10/11] tests/qtest/nvme-test: add migration test with full CQ Klaus Jensen
2026-07-06 22:44 ` [PULL 11/11] hw/nvme: add namespace hotplug support Klaus Jensen
2026-07-07 17:11 ` [PULL 00/11] hw/nvme queue Stefan Hajnoczi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260706224426.14156-2-its@irrelevant.dk \
    --to=its@irrelevant.dk \
    --cc=fin@spl.team \
    --cc=foss@defmacro.it \
    --cc=j.devantier@samsung.com \
    --cc=k.jensen@samsung.com \
    --cc=kbusch@kernel.org \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-stable@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.