[PATCH 0/6] nvme: fix transport request and queue error handling

[PATCH 0/6] nvme: fix transport request and queue error handling

[shao.mingyin]

The six patches fix independent error paths in the NVMe transports.

Patch 1 releases the target request in nvme-loop when SG setup fails.
nvmet_req_init() has already taken the queue, namespace and reservation
references, and blk-mq retries the same request, so each failure leaked a
set of references.

Patch 2 makes the TCP target wait for in-capsule data only when the SGL
is really an offset data block descriptor.  The old check looked only at
the descriptor length, so a transport SGL of the same length was treated
as in-capsule data; the same check is used when request initialization
fails.

Patch 3 requires the R2T offset to equal the number of bytes already
sent.  A larger offset let the host advertise an offset past the data
that its next H2CData PDU carried.

Patch 4 frees the fcloop LS request when nvmet_fc_rcv_ls_req() fails
before taking ownership.

Patch 5 publishes an FC target queue only if its slot is still free.  A
duplicate Create Connection replaced the old pointer and leaked the queue
and its controller reference.

Patch 6 rolls back the SQ binding and the controller reference taken by
nvmet_install_queue() when the CQ is already bound to another controller,
and destroys the CQ binding when the transport cannot install the queue.

The six patches are independent and can be applied in any order or as a
subset.  All carry Fixes: and Cc: stable.

Shao Mingyin (6):
  nvme-loop: release the target request when SG setup fails
  nvmet-tcp: only wait for in-capsule data for inline SGLs
  nvme-tcp: require the R2T offset to match the data sent
  nvmet-fcloop: free the LS request when delivery fails
  nvmet-fc: reject duplicate queue IDs on Create Connection
  nvmet: roll back queue bindings on connection failure

 drivers/nvme/host/tcp.c           |  2 +-
 drivers/nvme/target/fabrics-cmd.c |  3 +++
 drivers/nvme/target/fc.c          | 13 +++++++++++--
 drivers/nvme/target/fcloop.c      |  2 ++
 drivers/nvme/target/loop.c        |  1 +
 drivers/nvme/target/tcp.c         | 35 ++++++++++++++++++++++-------------
 6 files changed, 40 insertions(+), 16 deletions(-)

---
base-commit: ddfe51b0a0f5dc9e415cc2c8ad8db17532dd426b

[PATCH 1/6] nvme-loop: release the target request when SG setup fails

[shao.mingyin]

From: Shao Mingyin <shao.mingyin@zte.com.cn>

nvme_loop_queue_rq() calls nvmet_req_init(), which takes a reference on
the submission queue percpu ref, pins the namespace and, for a
reservation-enabled namespace, takes a per-controller PR reference.  If
sg_alloc_table_chained() then fails under memory pressure, the function
returns BLK_STS_RESOURCE without calling nvmet_req_uninit().  blk-mq
retries the same request and nvmet_req_init() overwrites req->ns and
req->pc_ref, leaking the queue, namespace and PR references taken by the
failed attempt each time this happens.

Release the request before returning BLK_STS_RESOURCE.

Fixes: 3a85a5de29ea ("nvme-loop: add a NVMe loopback host driver")
Cc: stable@vger.kernel.org
Signed-off-by: Shao Mingyin <shao.mingyin@zte.com.cn>
---
Tested on 7.3.0-next-20261030 (QEMU/TCG guest, 8 vCPUs) with the loop
transport and a file-backed namespace on tmpfs.

The scatterlist allocation was failed with failslab, narrowed to
nvme_loop_queue_rq() using the stack-trace filter.  One request is not
enough: nvme-loop caps it at NVME_LOOP_MAX_SEGMENTS (256), i.e. two
128-entry chunks, and the sgpool mempool keeps two elements in reserve,
so the allocation only fails when several requests are in flight.
Eight concurrent 1020 KiB passthrough writes on /dev/ng0n1, with
non-contiguous pages so that every 4 KiB is its own scatterlist entry,
hit it; blk-mq then retries the same request.

  unpatched                              patched
  delete_controller write never returns  completes in about 200 ms
  blocked in nvmet_sq_destroy()          -

The blocked task is in

  nvmet_sq_destroy
  nvme_loop_destroy_io_queues
  nvme_loop_shutdown_ctrl

waiting for sq->confirm_done because the submission queue reference
was leaked.  The failed attempt also leaks its namespace and any PR
reference when the retry overwrites the request fields.

diff --git a/drivers/nvme/target/loop.c b/drivers/nvme/target/loop.c
index e579a307798b..e95167a13708 100644
--- a/drivers/nvme/target/loop.c
+++ b/drivers/nvme/target/loop.c
@@ -159,6 +159,7 @@ static blk_status_t nvme_loop_queue_rq(struct blk_mq_hw_ctx *hctx,
 				blk_rq_nr_phys_segments(req),
 				iod->sg_table.sgl, NVME_INLINE_SG_CNT)) {
 			nvme_cleanup_cmd(req);
+			nvmet_req_uninit(&iod->req);
 			return BLK_STS_RESOURCE;
 		}
 
-- 
2.27.0

Re: [PATCH 1/6] nvme-loop: release the target request when SG setup fails

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

[PATCH 2/6] nvmet-tcp: only wait for in-capsule data for inline SGLs

[shao.mingyin]

From: Shao Mingyin <shao.mingyin@zte.com.cn>

nvmet-tcp decides whether to wait for in-capsule data using only the
write direction and SGL length.  A write using a transport SGL can fit
within inline_data_size without carrying any in-capsule data.  The
response path then withholds its R2T, leaving the command stuck.

If request initialization fails, the same length-only test both
withholds the error response and attempts to drain inline data that
will never arrive.  This can also consume the following PDU as data
or a digest.

Require an offset data-block SGL in both checks.  Share a helper that
only uses command and port fields, as the completion callback can run
before nvmet_req_init() has finished.

Fixes: bdaf13279192 ("nvmet-tcp: fix a segmentation fault during io parsing error")
Fixes: 872d26a391da ("nvmet-tcp: add NVMe over TCP target driver")
Cc: stable@vger.kernel.org
Signed-off-by: Shao Mingyin <shao.mingyin@zte.com.cn>
---
Tested on 7.3.0-next-20261030 (QEMU/TCG guest) with nvmet-tcp over
loopback.  The standard nvme-tcp host driver only sends in-capsule data
with an inline SGL, so it can never produce the triggering combination;
the test instead drives a minimal user-space NVMe/TCP host that crafts
the ICReq/Connect/command PDUs itself.

It sends a valid 4 KiB write with a transport SGL (type 0x5A) on an
enabled controller and I/O queue:

  unpatched                            patched
  no R2T, write hangs                  R2T received, H2CData sent,
                                       write completes (status 0x0000)

diff --git a/drivers/nvme/target/tcp.c b/drivers/nvme/target/tcp.c
index 8092ce4914b6..a8a435aaa163 100644
--- a/drivers/nvme/target/tcp.c
+++ b/drivers/nvme/target/tcp.c
@@ -269,6 +269,23 @@ static inline bool nvmet_tcp_has_inline_data(struct nvmet_tcp_cmd *cmd)
 		!cmd->rbytes_done;
 }
 
+/*
+ * Check whether the SGL of @cmd carries in-capsule data that the host
+ * will send right after the command PDU.  Only the offset form of the
+ * data block descriptor is in-capsule; a regular transport SGL is
+ * fetched through an R2T instead.
+ */
+static inline bool nvmet_tcp_sgl_is_inline(struct nvmet_tcp_cmd *cmd)
+{
+	struct nvme_sgl_desc *sgl = &cmd->req.cmd->common.dptr.sgl;
+	u32 len = le32_to_cpu(sgl->length);
+
+	return sgl->type == ((NVME_SGL_FMT_DATA_DESC << 4) |
+			     NVME_SGL_FMT_OFFSET) &&
+		nvme_is_write(cmd->req.cmd) && len &&
+		len <= cmd->req.port->inline_data_size;
+}
+
 static inline struct nvmet_tcp_cmd *
 nvmet_tcp_get_cmd(struct nvmet_tcp_queue *queue)
 {
@@ -625,25 +642,19 @@ static void nvmet_tcp_queue_response(struct nvmet_req *req)
 	struct nvmet_tcp_queue	*queue = cmd->queue;
 	enum nvmet_tcp_recv_state queue_state;
 	struct nvmet_tcp_cmd *queue_cmd;
-	struct nvme_sgl_desc *sgl;
-	u32 len;
 
 	/* Pairs with store_release in nvmet_prepare_receive_pdu() */
 	queue_state = smp_load_acquire(&queue->rcv_state);
 	queue_cmd = READ_ONCE(queue->cmd);
 
 	if (unlikely(cmd == queue_cmd)) {
-		sgl = &cmd->req.cmd->common.dptr.sgl;
-		len = le32_to_cpu(sgl->length);
-
 		/*
-		 * Wait for inline data before processing the response.
-		 * Avoid using helpers, this might happen before
-		 * nvmet_req_init is completed.
+		 * Wait for in-capsule data before processing the response.
+		 * This can happen before nvmet_req_init() has completed, so
+		 * only look at the SGL descriptor itself.
 		 */
 		if (queue_state == NVMET_TCP_RECV_PDU &&
-		    len && len <= cmd->req.port->inline_data_size &&
-		    nvme_is_write(cmd->req.cmd))
+		    nvmet_tcp_sgl_is_inline(cmd))
 			return;
 	}
 
@@ -981,7 +992,6 @@ static int nvmet_tcp_handle_icreq(struct nvmet_tcp_queue *queue)
 static int nvmet_tcp_handle_req_failure(struct nvmet_tcp_queue *queue,
 		struct nvmet_tcp_cmd *cmd, struct nvmet_req *req)
 {
-	size_t data_len = le32_to_cpu(req->cmd->common.dptr.sgl.length);
 	int ret;
 
 	/*
@@ -991,8 +1001,7 @@ static int nvmet_tcp_handle_req_failure(struct nvmet_tcp_queue *queue,
 	 * otherwise we will need to prepare a buffer and receive the
 	 * stale data before continuing forward.
 	 */
-	if (!nvme_is_write(cmd->req.cmd) || !data_len ||
-	    data_len > cmd->req.port->inline_data_size) {
+	if (!nvmet_tcp_sgl_is_inline(cmd)) {
 		nvmet_prepare_receive_pdu(queue);
 		return 0;
 	}

Re: [PATCH 2/6] nvmet-tcp: only wait for in-capsule data for inline SGLs

[Christoph Hellwig]

On Tue, Nov 10, 2026 at 11:03:10PM +0800, shao.mingyin@zte.com.cn wrote:
> +/*
> + * Check whether the SGL of @cmd carries in-capsule data that the host
> + * will send right after the command PDU.  Only the offset form of the
> + * data block descriptor is in-capsule; a regular transport SGL is
> + * fetched through an R2T instead.
> + */
> +static inline bool nvmet_tcp_sgl_is_inline(struct nvmet_tcp_cmd *cmd)
> +{
> +	struct nvme_sgl_desc *sgl = &cmd->req.cmd->common.dptr.sgl;
> +	u32 len = le32_to_cpu(sgl->length);
> +
> +	return sgl->type == ((NVME_SGL_FMT_DATA_DESC << 4) |
> +			     NVME_SGL_FMT_OFFSET) &&
> +		nvme_is_write(cmd->req.cmd) && len &&
> +		len <= cmd->req.port->inline_data_size;

Nit: this would become much more readable if split into multiple
conditions instead of this giant compound expression.

Otherwise looks good.

[PATCH 3/6] nvme-tcp: require the R2T offset to match the data sent

[shao.mingyin]

From: Shao Mingyin <shao.mingyin@zte.com.cn>

nvme_tcp_handle_r2t() rejects offsets below data_sent but accepts
larger offsets.  The send iterator still points at data_sent, so the
next H2CData PDU carries bytes from that position while advertising the
larger offset.  A target that accepts the PDU can place the data at the
wrong position; the Linux target rejects it and fails the connection.

Require the R2T offset to equal data_sent, rejecting the invalid request
before transmitting data with a mismatched offset.

Fixes: 3f2304f8c6d6 ("nvme-tcp: add NVMe over TCP host driver")
Cc: stable@vger.kernel.org
Signed-off-by: Shao Mingyin <shao.mingyin@zte.com.cn>
---
Tested on 7.3.0-next-20261030 (QEMU/TCG guest) with the nvmet-tcp
target on 127.0.0.1; no extra hardware is needed.

A test-only hook in the nvmet-tcp target shifts the R2T offset by one
4 KiB block.  The 4 KiB write to a loopback namespace is then answered
with an R2T at offset 4096 although the host has sent 0 bytes, so the
host either transmits an H2CData PDU with the wrong offset (unpatched)
or rejects the R2T (patched):

  unpatched                            patched
  nvmet_tcp: ttag 2 unexpected data    nvme nvme0: req 99 unexpected
  offset 4096 (expected 0)             r2t offset 4096 (expected 0)

In the unpatched case the target detects the mismatched H2CData offset
and rejects it.  This test demonstrates transmission of an incorrect
offset, not a successful corrupted write.  With the patch the host
rejects the R2T before transmitting H2CData.

diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
index bc46e5f3eaa7..2672aeae086a 100644
--- a/drivers/nvme/host/tcp.c
+++ b/drivers/nvme/host/tcp.c
@@ -846,7 +846,7 @@ static int nvme_tcp_handle_r2t(struct nvme_tcp_queue *queue,
 		return -EPROTO;
 	}
 
-	if (unlikely(r2t_offset < req->data_sent)) {
+	if (unlikely(r2t_offset != req->data_sent)) {
 		dev_err(queue->ctrl->ctrl.device,
 			"req %d unexpected r2t offset %u (expected %zu)\n",
 			rq->tag, r2t_offset, req->data_sent);

Re: [PATCH 3/6] nvme-tcp: require the R2T offset to match the data sent

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

[PATCH 4/6] nvmet-fcloop: free the LS request when delivery fails

[shao.mingyin]

From: Shao Mingyin <shao.mingyin@zte.com.cn>

fcloop_h2t_ls_req() allocates a fcloop_lsreq from lsreq_cache and hands
it to nvmet_fc_rcv_ls_req().  That function only takes ownership on
success and returns an error without invoking any completion callback
when the target port is shutting down, the LS payload is too large, or
the target has no free LS request slot.  The error is returned as-is, so
the fcloop_lsreq is leaked; every failed LS during a target port
teardown or an LS slot exhaustion loses one slab object.

The target-to-host direction already frees the request when
nvme_fc_rcv_ls_req() fails.  Do the same here.

Fixes: 772042dd38ee ("nvmet-fcloop: allocate/free fcloop_lsreq directly")
Cc: stable@vger.kernel.org
Signed-off-by: Shao Mingyin <shao.mingyin@zte.com.cn>
---
Tested on 7.3.0-next-20261030 (QEMU/TCG guest) with the fcloop
loopback transport (nvme-fc host + nvmet-fc/fcloop target); no real
FC HBA is required.

The -ESHUTDOWN error path is otherwise only hit in a sub-instruction
teardown race, so it is exercised with two TEST-ONLY changes (not part
of this patch): a debugfs knob that makes nvmet_fc_rcv_ls_req() return
-ESHUTDOWN, and SLAB_NO_MERGE on lsreq_cache so the 72-byte
fcloop_lsreq does not merge into a kmalloc cache.  A stock host sends
120 Create Association LS with ctrl_loss_tmo=0 (each failure deletes
the controller); the cache is shrunk afterwards, which only reclaims
empty slabs:

  unpatched                            patched
  lsreq_cache objects 0 -> 120         lsreq_cache objects stays 0
  slabs 0 -> 4                         slabs stay 0

diff --git a/drivers/nvme/target/fcloop.c b/drivers/nvme/target/fcloop.c
index b63af3b643a6..a9cde536c33b 100644
--- a/drivers/nvme/target/fcloop.c
+++ b/drivers/nvme/target/fcloop.c
@@ -379,6 +379,8 @@ fcloop_h2t_ls_req(struct nvme_fc_local_port *localport,
 	ret = nvmet_fc_rcv_ls_req(rport->targetport, rport,
 				  &tls_req->ls_rsp,
 				  lsreq->rqstaddr, lsreq->rqstlen);
+	if (ret)
+		kmem_cache_free(lsreq_cache, tls_req);
 
 	return ret;
 }

Re: [PATCH 4/6] nvmet-fcloop: free the LS request when delivery fails

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

[PATCH 5/6] nvmet-fc: reject duplicate queue IDs on Create Connection

[shao.mingyin]

From: Shao Mingyin <shao.mingyin@zte.com.cn>

nvmet_fc_alloc_target_queue() stores the new queue with

	WARN_ON(assoc->queues[qid]);
	assoc->queues[qid] = queue;

The queue ID comes straight from the Create Connection LS and is only
checked against NVMET_NR_QUEUES.  A remote initiator can send a second
Create Connection for a queue ID that already exists, or send queue ID 0
which was already created as the admin queue by Create Association.  The
WARN fires and the old pointer is overwritten: the old queue's base
reference is never put, so its workqueue, nvmet_sq and nvmet_cq are
never released.  For the admin queue this also leaves the controller
reference unreachable from nvmet_fc_delete_target_assoc(), so the
controller and its subsystem reference leak permanently.

Publish the queue with cmpxchg() and destroy the newly created queue if
the slot is already taken, returning the normal queue allocation error
to the initiator.  This keeps the existing error path and makes both
duplicate and admin queue IDs fail cleanly.

Fixes: c53432030d86 ("nvme-fabrics: Add target support for FC transport")
Cc: stable@vger.kernel.org
Signed-off-by: Shao Mingyin <shao.mingyin@zte.com.cn>
---
Tested on 7.3.0-next-20261030 (QEMU/TCG guest) with nvmet-fc over the
fcloop loopback transport; no FC HBA or fabric is needed.

A misbehaving FC initiator sends a duplicate Create Connection for a
queue ID that already exists, or for queue ID 0 which was already
created as the admin queue by Create Association.  A normal host never
does this, so the initiator is emulated by driving raw LS requests
through fcloop.  The LS response and the target debugfs controller
count after controller teardown are checked:

  unpatched                            patched
  duplicate conn -> ACC + WARN         duplicate conn -> RJT (0x03)
  ctrl1 dir leaked after delete        ctrl1 dir gone after delete

diff --git a/drivers/nvme/target/fc.c b/drivers/nvme/target/fc.c
index 2b7dc3f03912..181f8028a841 100644
--- a/drivers/nvme/target/fc.c
+++ b/drivers/nvme/target/fc.c
@@ -818,8 +818,17 @@ nvmet_fc_alloc_target_queue(struct nvmet_fc_tgt_assoc *assoc,
 	if (ret)
 		goto out_fail_iodlist;
 
-	WARN_ON(assoc->queues[qid]);
-	assoc->queues[qid] = queue;
+	/*
+	 * Publish the queue only if the slot is still free.  A Create
+	 * Connection may arrive twice for the same queue ID, or with the
+	 * admin queue ID that was created by Create Association; replacing
+	 * the old pointer would leak that queue and its controller
+	 * reference forever.
+	 */
+	if (cmpxchg(&assoc->queues[qid], NULL, queue)) {
+		nvmet_sq_destroy(&queue->nvme_sq);
+		goto out_fail_iodlist;
+	}
 
 	return queue;
 
-- 
2.27.0

Re: [PATCH 5/6] nvmet-fc: reject duplicate queue IDs on Create Connection

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

[PATCH 6/6] nvmet: roll back queue bindings on connection failure

[shao.mingyin]

From: Shao Mingyin <shao.mingyin@zte.com.cn>

nvmet_install_queue() publishes the SQ controller pointer before
claiming the CQ. If the CQ is already connected, it leaves the SQ
pointer and the controller reference taken for it behind on failure.

An install_queue() callback failure also leaves the CQ bound after
clearing the SQ pointer. The reference taken in nvmet_install_queue()
is in addition to the one the caller already holds: the caller's put
releases the reference acquired before the call, while this one belongs
to the CQ binding. It is not released on this path, so the controller
stays alive and a Connect retry on the same queue keeps finding the
stale binding.

Clear the tentative SQ binding and release the extra reference on a CQ
conflict. On a transport installation failure, destroy the CQ binding
as well, so its controller table entry and reference are released.

Link: https://lore.kernel.org/all/20261106015842.3534898-6-kanie@linux.alibaba.com/
Fixes: bb78836b3a7c ("nvmet: fabrics: add CQ init and destroy")
Cc: stable@vger.kernel.org
Signed-off-by: Shao Mingyin <shao.mingyin@zte.com.cn>
---
 drivers/nvme/target/fabrics-cmd.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/nvme/target/fabrics-cmd.c b/drivers/nvme/target/fabrics-cmd.c
index fc626c2fd2c9..6f0fc695e649 100644
--- a/drivers/nvme/target/fabrics-cmd.c
+++ b/drivers/nvme/target/fabrics-cmd.c
@@ -216,6 +216,8 @@ static u16 nvmet_install_queue(struct nvmet_ctrl *ctrl, struct nvmet_req *req)
 	if (old) {
 		pr_warn("queue already connected!\n");
 		req->error_loc = offsetof(struct nvmf_connect_command, opcode);
+		req->sq->ctrl = NULL;
+		nvmet_ctrl_put(ctrl);
 		return NVME_SC_CONNECT_CTRL_BUSY | NVME_STATUS_DNR;
 	}
 
@@ -242,6 +244,7 @@ static u16 nvmet_install_queue(struct nvmet_ctrl *ctrl, struct nvmet_req *req)
 
 err:
 	req->sq->ctrl = NULL;
+	nvmet_cq_destroy(req->cq);
 	return ret;
 }
 
-- 
2.27.0

Re: [PATCH 6/6] nvmet: roll back queue bindings on connection failure

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>