[PULL 0/4] hw/nvme fixes

[PULL 0/4] hw/nvme fixes

[Klaus Jensen]

From: Klaus Jensen <k.jensen@samsung.com>

Hi,

The following changes since commit de5eb7c4dd3ef5688b14007134eb11ff4973580c:

  Merge tag 'hw-misc-20260728' of https://github.com/philmd/qemu into staging (2026-07-28 10:04:08 -0400)

are available in the Git repository at:

  https://gitlab.com/birkelund/qemu.git tags/pull-nvme-20260728

for you to fetch changes up to c39e05af439a3a2bd58a847fc30d7eb590fcf61e:

  hw/nvme: fix unintentional integer overflow in shift (2026-07-28 19:59:01 +0200)

----------------------------------------------------------------
nvme queue

----------------------------------------------------------------
Alexander Mikhalitsyn (1):
      hw/nvme: use GPtrArray for blocker_features

Daniel Paziyski (1):
      hw/nvme: fix assertion failure on subregion removal

Klaus Jensen (2):
      hw/nvme: fix cross-namespace copy dif buffer overflow
      hw/nvme: fix unintentional integer overflow in shift

 hw/nvme/ctrl.c | 57 +++++++++++++++++++++++++++------------------------------
 1 file changed, 27 insertions(+), 30 deletions(-)

[PULL 1/4] hw/nvme: use GPtrArray for blocker_features

[Klaus Jensen]

From: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@futurfusion.io>

Let's use GPtrArray to build a list of blocker features and then
g_strjoinv() to build a final comma-delimited string.

While previous approach was technically correct, it is fragile
(because we need to take care of static buffer size choice) and
Coverity dislikes it too.

Note, that we use g_ptr_array_new() to allocate array which means
that GDestroyNotify callback is not set, so we can pass pointers to
a static memory like g_ptr_array_add(..., (gpointer) "SR-IOV") without
any problems as there won't be any attempt to free that memory.

Resolves: Coverity CID 1663673
Suggested-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@futurfusion.io>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Klaus Jensen <k.jensen@samsung.com>
[k.jensen: change cast from gpointer to void ptr]
Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
---
 hw/nvme/ctrl.c | 41 +++++++++++++++++------------------------
 1 file changed, 17 insertions(+), 24 deletions(-)

diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index a67e1598891c..086048b689d3 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -9352,22 +9352,11 @@ static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice *pci_dev)
     }
 }
 
-#define BLOCKER_FEATURES_MAX_LEN 256
-
-static inline void nvme_add_blocker_feature(char *blocker_features,
-                                            const char *feature)
-{
-    if (strlen(blocker_features) > 0) {
-        g_strlcat(blocker_features, ", ", BLOCKER_FEATURES_MAX_LEN);
-    }
-    g_strlcat(blocker_features, feature, BLOCKER_FEATURES_MAX_LEN);
-}
-
 static bool nvme_set_migration_blockers(NvmeCtrl *n, PCIDevice *pci_dev,
                                         Error **errp)
 {
     uint64_t unsupported_cap, cap = ldq_le_p(&n->bar.cap);
-    char blocker_features[BLOCKER_FEATURES_MAX_LEN] = "";
+    g_autoptr(GPtrArray) blocker_features = g_ptr_array_new();
     bool adm_cmd_security_checked = false;
     bool cmd_io_mgmt_checked = false;
     bool cmd_zone_checked = false;
@@ -9416,15 +9405,15 @@ static bool nvme_set_migration_blockers(NvmeCtrl *n, PCIDevice *pci_dev,
             }
 
             if (namespaces_num > 1) {
-                nvme_add_blocker_feature(blocker_features,
-                                         "Namespace Attachment");
+                g_ptr_array_add(blocker_features,
+                                (void *) "Namespace Attachment");
             }
 
             break;
         }
         case NVME_ADM_CMD_VIRT_MNGMT:
             if (n->params.sriov_max_vfs) {
-                nvme_add_blocker_feature(blocker_features, "SR-IOV");
+                g_ptr_array_add(blocker_features, (void *) "SR-IOV");
             }
 
             break;
@@ -9435,7 +9424,7 @@ static bool nvme_set_migration_blockers(NvmeCtrl *n, PCIDevice *pci_dev,
             }
 
             if (pci_dev->spdm_port) {
-                nvme_add_blocker_feature(blocker_features, "SPDM");
+                g_ptr_array_add(blocker_features, (void *) "SPDM");
             }
 
             adm_cmd_security_checked = true;
@@ -9469,7 +9458,7 @@ static bool nvme_set_migration_blockers(NvmeCtrl *n, PCIDevice *pci_dev,
 
             /* check for NVME_IOMS_MO_RUH_UPDATE */
             if (n->subsys->params.fdp.enabled) {
-                nvme_add_blocker_feature(blocker_features, "FDP");
+                g_ptr_array_add(blocker_features, (void *) "FDP");
             }
 
             cmd_io_mgmt_checked = true;
@@ -9504,8 +9493,8 @@ static bool nvme_set_migration_blockers(NvmeCtrl *n, PCIDevice *pci_dev,
                 }
 
                 if (ns->params.zoned) {
-                    nvme_add_blocker_feature(blocker_features,
-                                             "Zoned Namespace");
+                    g_ptr_array_add(blocker_features,
+                                    (void *) "Zoned Namespace");
                     break;
                 }
             }
@@ -9525,24 +9514,28 @@ static bool nvme_set_migration_blockers(NvmeCtrl *n, PCIDevice *pci_dev,
      * covered by unsupported_cap check.
      */
     if (NVME_CAP_CMBS(cap)) {
-        nvme_add_blocker_feature(blocker_features, "CMB");
+        g_ptr_array_add(blocker_features, (void *) "CMB");
         cap &= ~((uint64_t)CAP_CMBS_MASK << CAP_CMBS_SHIFT);
     }
 
     if (NVME_CAP_PMRS(cap)) {
-        nvme_add_blocker_feature(blocker_features, "PMR");
+        g_ptr_array_add(blocker_features, (void *) "PMR");
         cap &= ~((uint64_t)CAP_PMRS_MASK << CAP_PMRS_SHIFT);
     }
 
     unsupported_cap = cap & ~NVME_MIGRATION_SUPPORTED_CAP_BITS;
     if (unsupported_cap) {
-        nvme_add_blocker_feature(blocker_features, "unknown capability");
+        g_ptr_array_add(blocker_features, (void *) "unknown capability");
     }
 
     assert(n->migration_blocker == NULL);
-    if (strlen(blocker_features) > 0) {
+    if (blocker_features->len > 0) {
+        g_autofree char *blocker_list = NULL;
+
+        g_ptr_array_add(blocker_features, NULL);
+        blocker_list = g_strjoinv(", ", (void *)blocker_features->pdata);
         error_setg(&n->migration_blocker,
-                   "Migration is not supported for %s", blocker_features);
+                   "Migration is not supported for %s", blocker_list);
         if (migrate_add_blocker(&n->migration_blocker, errp) < 0) {
             return false;
         }
-- 
2.53.0

[PULL 2/4] hw/nvme: fix assertion failure on subregion removal

[Klaus Jensen]

From: Daniel Paziyski <danielpaziyski@gmail.com>

When a controller is created with a MSI-X exclusive BAR, the bar0 memory region
is not used at all, and so, the iomem region is not added as a subregion of it.
However, when removing a NVMe controller, the iomem region is unconditionally
removed as a subregion of bar0, causing an assertion failure. Remove the iomem
memory region as a subregion of bar0 only if not using a MSI-X exclusive BAR.

QEMU options (requires a hotunplug-aware OS):

        -M q35 -device pcie-root-port,id=rp0 \
        -device nvme,serial=ctrl0,id=ctrl0,bus=rp0,msix-exclusive-bar=on

In the QEMU monitor, or by causing an ejection from the OS:

        device_del ctrl0

Message in stderr:

qemu-system-x86_64: ../system/memory.c:2617: memory_region_del_subregion: Assertion `subregion->container == mr' failed.

Fixes: fa905f65c554 ("hw/nvme: add machine compatibility parameter to enable msix exclusive bar")
Fixes: 9162f1012576 ("hw/nvme: fix msix_uninit with exclusive bar")
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4090
Signed-off-by: Daniel Paziyski <danielpaziyski@gmail.com>
Reviewed-by: Klaus Jensen <k.jensen@samsung.com>
Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
---
 hw/nvme/ctrl.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index 086048b689d3..e1639b3839f7 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -9703,10 +9703,9 @@ static void nvme_exit(PCIDevice *pci_dev)
         msix_uninit_exclusive_bar(pci_dev);
     } else {
         msix_uninit(pci_dev, &n->bar0, &n->bar0);
+        memory_region_del_subregion(&n->bar0, &n->iomem);
     }
 
-    memory_region_del_subregion(&n->bar0, &n->iomem);
-
     migrate_del_blocker(&n->migration_blocker);
 }
 
-- 
2.53.0

[PULL 3/4] hw/nvme: fix cross-namespace copy dif buffer overflow

[Klaus Jensen]

From: Klaus Jensen <k.jensen@samsung.com>

The NVMe specification allows a controller with multiple namespaces to
use different LBA formats per namespace. One implication of this is that
the destination namespace may have a metadata area for PI, but the
source does not. In that case, the controller shall generate the
protection information, but the bounce buffer is erroneously allocated
without space for that, causing a buffer overflow.

Fix the allocation.

Cc: qemu-stable@nongnu.org
Fixes: d522aef88d42 ("hw/nvme: add cross namespace copy support")
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3387
Reported-by: Jihe Wang <wangjihe.mail@gmail.com>
Reported-by: boy juju <agx1657748706@gmail.com>
Reported-by: contact <contact@xchglabs.com>
Reported-by: david korczynski <david@adalogics.com>
Reported-by: Brian Chastain (off_by_one / Curious-Keeper) <brian@scalingsuccess.io>
Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>
Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
---
 hw/nvme/ctrl.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index e1639b3839f7..acd10fcc67fa 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -3210,7 +3210,7 @@ static void nvme_do_copy(NvmeCopyAIOCB *iocb)
     uint16_t prinfow = ((copy->control[2] >> 2) & 0xf);
     uint64_t slba;
     uint32_t nlb;
-    size_t len;
+    size_t len, blen;
     uint16_t status;
     uint32_t dnsid = le32_to_cpu(req->cmd.nsid);
     uint32_t snsid = dnsid;
@@ -3331,10 +3331,13 @@ static void nvme_do_copy(NvmeCopyAIOCB *iocb)
     }
 
     g_free(iocb->bounce);
-    iocb->bounce = g_malloc_n(le16_to_cpu(sns->id_ns.mssrl),
-                              sns->lbasz + sns->lbaf.ms);
+    assert(g_size_checked_mul(&blen, le16_to_cpu(sns->id_ns.mssrl),
+                              sns->lbasz + MAX(sns->lbaf.ms, dns->lbaf.ms)));
+
+    iocb->bounce = g_malloc(blen);
 
     qemu_iovec_reset(&iocb->iov);
+    assert(len <= blen);
     qemu_iovec_add(&iocb->iov, iocb->bounce, len);
 
     block_acct_start(blk_get_stats(sns->blkconf.blk), &iocb->acct.read, 0,
-- 
2.53.0

[PULL 4/4] hw/nvme: fix unintentional integer overflow in shift

[Klaus Jensen]

From: Klaus Jensen <k.jensen@samsung.com>

Fix potentially overflowing shift operation.

Cc: qemu-stable@nongnu.org
Suggested-by: Peter Maydell <peter.maydell@linaro.org>
Suggested-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>
Resolves: Coverity CID 1663674
Fixes: ec917cd49918 ("hw/nvme: fix FDP set FDP events")
Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>
Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
---
 hw/nvme/ctrl.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index acd10fcc67fa..bd6ad64b2000 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -196,6 +196,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/bitops.h"
 #include "qemu/cutils.h"
 #include "qemu/error-report.h"
 #include "qemu/log.h"
@@ -6625,7 +6626,8 @@ static uint16_t nvme_set_feature_fdp_events(NvmeCtrl *n, NvmeNamespace *ns,
         if (!shift && event_type) {
             continue;
         }
-        event_mask |= (1 << nvme_fdp_evf_shifts[events[i]]);
+        event_mask =
+            deposit64(event_mask, nvme_fdp_evf_shifts[events[i]], 1, 1);
     }
 
     if (enable) {
-- 
2.53.0