[PATCH 0/2] nvmet-pci: validate endpoint queue IDs

[PATCH 0/2] nvmet-pci: validate endpoint queue IDs

[Michael Bommarito]

A PCI root-complex host can crash an NVMe PCI endpoint target with
malformed queue IDs. The endpoint transport allocates its SQ/CQ arrays
using ctrl->nr_queues, which is capped by endpoint interrupt capacity,
but the common target admin validation only checks queue IDs against
subsys->max_qid. A host can therefore submit Create/Delete SQ/CQ commands
with qids that pass the common checks yet index past the smaller endpoint
transport arrays.

Patch 1 rejects queue IDs outside ctrl->nr_queues before the endpoint
SQ/CQ arrays are indexed. Patch 2 adds same-translation-unit KUnit/KASAN
coverage: a valid queue ID that must still be accepted and the
out-of-range Create/Delete SQ/CQ cases that must now be rejected.

Reproduced with the KUnit/KASAN test: the stock Create CQ path faults in
nvmet_pci_epf_create_cq() after nvmet_check_io_cqid() accepts qid 2 with
max_qid 8 and nr_queues 2; patched rejects the malformed cases while the
benign control still passes.

Cc: stable@vger.kernel.org

Michael Bommarito (2):
  nvmet-pci: validate queue IDs against endpoint queues
  nvmet-pci: add KUnit coverage for endpoint queue IDs

 drivers/nvme/target/Kconfig   |  11 +++
 drivers/nvme/target/pci-epf.c | 151 ++++++++++++++++++++++++++++++++--
 2 files changed, 157 insertions(+), 5 deletions(-)

--
2.53.0

[PATCH 1/2] nvmet-pci: validate queue IDs against endpoint queues

[Michael Bommarito]

The NVMe PCI endpoint transport allocates SQ/CQ arrays using
ctrl->nr_queues, which is capped by endpoint interrupt capacity. Common
target admin validation only checks queue IDs against subsys->max_qid, so
a root-complex host can submit Create/Delete SQ/CQ commands with qids that
pass the common checks but index past the smaller endpoint transport
arrays.

Impact: A PCI root-complex host can crash an NVMe PCI endpoint target with
malformed queue IDs.

Reject queue IDs that are outside ctrl->nr_queues before indexing the
endpoint SQ/CQ arrays.

Fixes: 0faa0fe6f90e ("nvmet: New NVMe PCI endpoint function target driver")
Cc: stable@vger.kernel.org
Assisted-by: Codex:gpt-5-5-xhigh
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
---

I reproduced this with a same-translation-unit KUnit/KASAN test. The stock
Create CQ path faults in nvmet_pci_epf_create_cq() after
nvmet_check_io_cqid() accepts qid 2 with max_qid 8 and nr_queues 2. The
patched checks reject malformed Create/Delete SQ/CQ cases while the benign
control still passes.
 drivers/nvme/target/pci-epf.c | 31 ++++++++++++++++++++++++++-----
 1 file changed, 26 insertions(+), 5 deletions(-)

diff --git a/drivers/nvme/target/pci-epf.c b/drivers/nvme/target/pci-epf.c
index 4e9db96ebfecd..5bddda09c0538 100644
--- a/drivers/nvme/target/pci-epf.c
+++ b/drivers/nvme/target/pci-epf.c
@@ -1267,10 +1267,15 @@ static u16 nvmet_pci_epf_create_cq(struct nvmet_ctrl *tctrl,
 		u16 cqid, u16 flags, u16 qsize, u64 pci_addr, u16 vector)
 {
 	struct nvmet_pci_epf_ctrl *ctrl = tctrl->drvdata;
-	struct nvmet_pci_epf_queue *cq = &ctrl->cq[cqid];
+	struct nvmet_pci_epf_queue *cq;
 	u16 status;
 	int ret;
 
+	if (cqid >= ctrl->nr_queues)
+		return NVME_SC_QID_INVALID | NVME_STATUS_DNR;
+
+	cq = &ctrl->cq[cqid];
+
 	if (test_bit(NVMET_PCI_EPF_Q_LIVE, &cq->flags))
 		return NVME_SC_QID_INVALID | NVME_STATUS_DNR;
 
@@ -1348,7 +1353,12 @@ static u16 nvmet_pci_epf_create_cq(struct nvmet_ctrl *tctrl,
 static u16 nvmet_pci_epf_delete_cq(struct nvmet_ctrl *tctrl, u16 cqid)
 {
 	struct nvmet_pci_epf_ctrl *ctrl = tctrl->drvdata;
-	struct nvmet_pci_epf_queue *cq = &ctrl->cq[cqid];
+	struct nvmet_pci_epf_queue *cq;
+
+	if (cqid >= ctrl->nr_queues)
+		return NVME_SC_QID_INVALID | NVME_STATUS_DNR;
+
+	cq = &ctrl->cq[cqid];
 
 	if (!test_and_clear_bit(NVMET_PCI_EPF_Q_LIVE, &cq->flags))
 		return NVME_SC_QID_INVALID | NVME_STATUS_DNR;
@@ -1367,10 +1377,16 @@ static u16 nvmet_pci_epf_create_sq(struct nvmet_ctrl *tctrl,
 		u16 sqid, u16 cqid, u16 flags, u16 qsize, u64 pci_addr)
 {
 	struct nvmet_pci_epf_ctrl *ctrl = tctrl->drvdata;
-	struct nvmet_pci_epf_queue *sq = &ctrl->sq[sqid];
-	struct nvmet_pci_epf_queue *cq = &ctrl->cq[cqid];
+	struct nvmet_pci_epf_queue *sq;
+	struct nvmet_pci_epf_queue *cq;
 	u16 status;
 
+	if (sqid >= ctrl->nr_queues || cqid >= ctrl->nr_queues)
+		return NVME_SC_QID_INVALID | NVME_STATUS_DNR;
+
+	sq = &ctrl->sq[sqid];
+	cq = &ctrl->cq[cqid];
+
 	if (test_bit(NVMET_PCI_EPF_Q_LIVE, &sq->flags))
 		return NVME_SC_QID_INVALID | NVME_STATUS_DNR;
 
@@ -1419,7 +1435,12 @@ static u16 nvmet_pci_epf_create_sq(struct nvmet_ctrl *tctrl,
 static u16 nvmet_pci_epf_delete_sq(struct nvmet_ctrl *tctrl, u16 sqid)
 {
 	struct nvmet_pci_epf_ctrl *ctrl = tctrl->drvdata;
-	struct nvmet_pci_epf_queue *sq = &ctrl->sq[sqid];
+	struct nvmet_pci_epf_queue *sq;
+
+	if (sqid >= ctrl->nr_queues)
+		return NVME_SC_QID_INVALID | NVME_STATUS_DNR;
+
+	sq = &ctrl->sq[sqid];
 
 	if (!test_and_clear_bit(NVMET_PCI_EPF_Q_LIVE, &sq->flags))
 		return NVME_SC_QID_INVALID | NVME_STATUS_DNR;
-- 
2.53.0

[PATCH 2/2] nvmet-pci: add KUnit coverage for endpoint queue IDs

[Michael Bommarito]

Add KUnit coverage for the PCI endpoint target queue-id boundary. The tests
model the case where target-core max_qid is larger than the endpoint
transport's ctrl->nr_queues, confirm the common qid check accepts the
malformed id, and verify the endpoint callbacks reject out-of-range
Create/Delete SQ/CQ requests before indexing transport-private arrays.

This covers the regression fixed by the preceding patch.

Assisted-by: Codex:gpt-5-5-xhigh
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
---
 drivers/nvme/target/Kconfig   |  11 ++++
 drivers/nvme/target/pci-epf.c | 120 ++++++++++++++++++++++++++++++++++
 2 files changed, 131 insertions(+)

diff --git a/drivers/nvme/target/Kconfig b/drivers/nvme/target/Kconfig
index 4904097dfd490..ea64bbe9882c5 100644
--- a/drivers/nvme/target/Kconfig
+++ b/drivers/nvme/target/Kconfig
@@ -127,3 +127,14 @@ config NVME_TARGET_PCI_EPF
 	  capable PCI controller.
 
 	  If unsure, say N.
+
+config NVMET_PCI_EPF_KUNIT_TEST
+	bool "NVMe PCI endpoint target KUnit tests" if !KUNIT_ALL_TESTS
+	depends on KUNIT
+	depends on NVME_TARGET_PCI_EPF=y
+	default KUNIT_ALL_TESTS
+	help
+	  KUnit tests for the NVMe PCI endpoint target transport.
+	  These tests exercise transport-private queue ID checks for
+	  Create/Delete SQ/CQ commands when target-core max_qid is larger than
+	  the endpoint controller's available queue arrays.
diff --git a/drivers/nvme/target/pci-epf.c b/drivers/nvme/target/pci-epf.c
index 5bddda09c0538..e2eb96f32fab5 100644
--- a/drivers/nvme/target/pci-epf.c
+++ b/drivers/nvme/target/pci-epf.c
@@ -20,6 +20,9 @@
 #include <linux/pci-epf.h>
 #include <linux/pci_regs.h>
 #include <linux/slab.h>
+#if IS_ENABLED(CONFIG_NVMET_PCI_EPF_KUNIT_TEST)
+#include <kunit/test.h>
+#endif
 
 #include "nvmet.h"
 
@@ -2667,3 +2670,120 @@ module_exit(nvmet_pci_epf_cleanup_module);
 MODULE_DESCRIPTION("NVMe PCI Endpoint Function target driver");
 MODULE_AUTHOR("Damien Le Moal <dlemoal@kernel.org>");
 MODULE_LICENSE("GPL");
+
+#if IS_ENABLED(CONFIG_NVMET_PCI_EPF_KUNIT_TEST)
+
+struct nvmet_pci_epf_kunit_ctx {
+	struct nvmet_ctrl tctrl;
+	struct nvmet_subsys subsys;
+	struct nvmet_pci_epf_ctrl ctrl;
+	struct nvmet_pci_epf nvme_epf;
+};
+
+static int nvmet_pci_epf_kunit_init(struct kunit *test)
+{
+	struct nvmet_pci_epf_kunit_ctx *ctx;
+	unsigned int qid;
+
+	ctx = kunit_kzalloc(test, sizeof(*ctx), GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, ctx);
+
+	ctx->subsys.max_qid = 8;
+	ctx->tctrl.subsys = &ctx->subsys;
+	ctx->tctrl.drvdata = &ctx->ctrl;
+	ctx->tctrl.cqs = kunit_kcalloc(test, ctx->subsys.max_qid + 1,
+				       sizeof(*ctx->tctrl.cqs), GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, ctx->tctrl.cqs);
+	ctx->tctrl.sqs = kunit_kcalloc(test, ctx->subsys.max_qid + 1,
+				       sizeof(*ctx->tctrl.sqs), GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, ctx->tctrl.sqs);
+
+	ctx->ctrl.nr_queues = 2;
+	ctx->ctrl.tctrl = &ctx->tctrl;
+	ctx->ctrl.nvme_epf = &ctx->nvme_epf;
+	ctx->ctrl.sq = kunit_kcalloc(test, ctx->ctrl.nr_queues,
+				     sizeof(*ctx->ctrl.sq), GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, ctx->ctrl.sq);
+	ctx->ctrl.cq = kunit_kcalloc(test, ctx->ctrl.nr_queues,
+				     sizeof(*ctx->ctrl.cq), GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, ctx->ctrl.cq);
+
+	for (qid = 0; qid < ctx->ctrl.nr_queues; qid++) {
+		nvmet_pci_epf_init_queue(&ctx->ctrl, qid, true);
+		nvmet_pci_epf_init_queue(&ctx->ctrl, qid, false);
+	}
+
+	test->priv = ctx;
+	return 0;
+}
+
+static void nvmet_pci_epf_qid_control_test(struct kunit *test)
+{
+	struct nvmet_pci_epf_kunit_ctx *ctx = test->priv;
+	u16 status;
+
+	status = nvmet_check_io_cqid(&ctx->tctrl, 1, true);
+	KUNIT_EXPECT_EQ(test, status, (u16)NVME_SC_SUCCESS);
+
+	status = nvmet_pci_epf_create_cq(&ctx->tctrl, 1, 0, 1, 0, 0);
+	KUNIT_EXPECT_EQ(test, status,
+			(u16)(NVME_SC_INVALID_QUEUE | NVME_STATUS_DNR));
+}
+
+static void nvmet_pci_epf_qid_oob_test(struct kunit *test)
+{
+	struct nvmet_pci_epf_kunit_ctx *ctx = test->priv;
+	u16 bad_qid = ctx->ctrl.nr_queues;
+	u16 status;
+
+	status = nvmet_check_io_cqid(&ctx->tctrl, bad_qid, true);
+	KUNIT_EXPECT_EQ(test, status, (u16)NVME_SC_SUCCESS);
+
+	status = nvmet_pci_epf_create_cq(&ctx->tctrl, bad_qid, 0, 1, 0, 0);
+	KUNIT_EXPECT_EQ(test, status,
+			(u16)(NVME_SC_QID_INVALID | NVME_STATUS_DNR));
+}
+
+static void nvmet_pci_epf_qid_reject_all_test(struct kunit *test)
+{
+	struct nvmet_pci_epf_kunit_ctx *ctx = test->priv;
+	u16 bad_qid = ctx->ctrl.nr_queues;
+	u16 status;
+
+	status = nvmet_pci_epf_create_cq(&ctx->tctrl, bad_qid, 0, 1, 0, 0);
+	KUNIT_EXPECT_EQ(test, status,
+			(u16)(NVME_SC_QID_INVALID | NVME_STATUS_DNR));
+
+	status = nvmet_pci_epf_create_sq(&ctx->tctrl, bad_qid, 1, 0, 1, 0);
+	KUNIT_EXPECT_EQ(test, status,
+			(u16)(NVME_SC_QID_INVALID | NVME_STATUS_DNR));
+
+	status = nvmet_pci_epf_create_sq(&ctx->tctrl, 1, bad_qid, 0, 1, 0);
+	KUNIT_EXPECT_EQ(test, status,
+			(u16)(NVME_SC_QID_INVALID | NVME_STATUS_DNR));
+
+	status = nvmet_pci_epf_delete_cq(&ctx->tctrl, bad_qid);
+	KUNIT_EXPECT_EQ(test, status,
+			(u16)(NVME_SC_QID_INVALID | NVME_STATUS_DNR));
+
+	status = nvmet_pci_epf_delete_sq(&ctx->tctrl, bad_qid);
+	KUNIT_EXPECT_EQ(test, status,
+			(u16)(NVME_SC_QID_INVALID | NVME_STATUS_DNR));
+}
+
+static struct kunit_case nvmet_pci_epf_qid_test_cases[] = {
+	KUNIT_CASE(nvmet_pci_epf_qid_control_test),
+	KUNIT_CASE(nvmet_pci_epf_qid_oob_test),
+	KUNIT_CASE(nvmet_pci_epf_qid_reject_all_test),
+	{}
+};
+
+static struct kunit_suite nvmet_pci_epf_qid_test_suite = {
+	.name = "nvmet_pci_epf_qid",
+	.init = nvmet_pci_epf_kunit_init,
+	.test_cases = nvmet_pci_epf_qid_test_cases,
+};
+
+kunit_test_suite(nvmet_pci_epf_qid_test_suite);
+
+#endif
-- 
2.53.0

Re: [PATCH 1/2] nvmet-pci: validate queue IDs against endpoint queues

[Damien Le Moal]

On 7/10/26 11:30, Michael Bommarito wrote:
> The NVMe PCI endpoint transport allocates SQ/CQ arrays using
> ctrl->nr_queues, which is capped by endpoint interrupt capacity. Common
> target admin validation only checks queue IDs against subsys->max_qid, so
> a root-complex host can submit Create/Delete SQ/CQ commands with qids that
> pass the common checks but index past the smaller endpoint transport
> arrays.
> 
> Impact: A PCI root-complex host can crash an NVMe PCI endpoint target with
> malformed queue IDs.
> 
> Reject queue IDs that are outside ctrl->nr_queues before indexing the
> endpoint SQ/CQ arrays.
> 
> Fixes: 0faa0fe6f90e ("nvmet: New NVMe PCI endpoint function target driver")
> Cc: stable@vger.kernel.org
> Assisted-by: Codex:gpt-5-5-xhigh
> Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>

Looks good.

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>

-- 
Damien Le Moal
Western Digital Research

Re: [PATCH 2/2] nvmet-pci: add KUnit coverage for endpoint queue IDs

[Damien Le Moal]

On 7/10/26 11:30, Michael Bommarito wrote:
> Add KUnit coverage for the PCI endpoint target queue-id boundary. The tests
> model the case where target-core max_qid is larger than the endpoint
> transport's ctrl->nr_queues, confirm the common qid check accepts the
> malformed id, and verify the endpoint callbacks reject out-of-range
> Create/Delete SQ/CQ requests before indexing transport-private arrays.
> 
> This covers the regression fixed by the preceding patch.
> 
> Assisted-by: Codex:gpt-5-5-xhigh
> Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>

I am not very familiar with kunit tests, but looks OK.

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>

-- 
Damien Le Moal
Western Digital Research