mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: <shao.mingyin@zte.com.cn>
To: <kbusch@kernel.org>, <axboe@kernel.dk>, <hch@lst.de>,
	<sagi@grimberg.me>, <linux-nvme@lists.infradead.org>
Cc: <justin.tee@broadcom.com>, <nareshgottumukkala83@gmail.com>,
	<paul.ely@broadcom.com>, <kch@nvidia.com>,
	<linux-kernel@vger.kernel.org>, <stable@vger.kernel.org>
Subject: [PATCH v2 2/6] nvmet-tcp: only wait for in-capsule data for inline SGLs
Date: Sat, 14 Nov 2026 02:02:49 +0800 (CST)	[thread overview]
Message-ID: <20261114020249092lDPe3OpMiMq8nakVU-IJ6@zte.com.cn> (raw)
In-Reply-To: <20261114020245882Mvd6t10TkCZuRWpYFR3m5@zte.com.cn>

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>
---
Changes in v2 (Christoph Hellwig):
- Split nvmet_tcp_sgl_is_inline() into separate conditions. No
  functional change; keep the existing request paths unchanged.
v1: https://lore.kernel.org/all/20261110230310932grXKWow1mHtiisyby_D4-@zte.com.cn/

The original and revised helpers were compiled with GCC 13 and UBSAN
and compared across every SGL type and opcode, with zero, boundary and
large lengths and inline limits. All results matched. This function
test uses command/port stubs and does not exercise TCP or request
initialization. The runtime results below are from v1.

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)

 drivers/nvme/target/tcp.c | 42 +++++++++++++++++++++++++++------------
 1 file changed, 29 insertions(+), 13 deletions(-)

diff --git a/drivers/nvme/target/tcp.c b/drivers/nvme/target/tcp.c
index 8092ce4914b62c80504083cc0519cf7908381d81..dc97534c5b7a83b98cf4f0fd4429476afe16c889 100644
--- a/drivers/nvme/target/tcp.c
+++ b/drivers/nvme/target/tcp.c
@@ -269,6 +269,30 @@ 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);
+
+	if (sgl->type != ((NVME_SGL_FMT_DATA_DESC << 4) |
+			  NVME_SGL_FMT_OFFSET))
+		return false;
+
+	if (!nvme_is_write(cmd->req.cmd))
+		return false;
+
+	if (!len)
+		return false;
+
+	return len <= cmd->req.port->inline_data_size;
+}
+
 static inline struct nvmet_tcp_cmd *
 nvmet_tcp_get_cmd(struct nvmet_tcp_queue *queue)
 {
@@ -625,25 +649,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 +999,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 +1008,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;
 	}
-- 
2.27.0

  parent reply	other threads:[~2026-11-13 18:02 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-13 18:02 [PATCH v2 0/6] nvme: fix transport request and queue error handling shao.mingyin
2026-11-13 18:02 ` [PATCH v2 1/6] nvme-loop: release the target request when SG setup fails shao.mingyin
2026-11-13 18:02 ` shao.mingyin [this message]
2026-11-13 18:02 ` [PATCH v2 3/6] nvme-tcp: require the R2T offset to match the data sent shao.mingyin
2026-11-13 18:02 ` [PATCH v2 4/6] nvmet-fcloop: free the LS request when delivery fails shao.mingyin
2026-11-13 18:02 ` [PATCH v2 5/6] nvmet-fc: reject duplicate queue IDs on Create Connection shao.mingyin
2026-11-13 18:02 ` [PATCH v2 6/6] nvmet: roll back queue bindings on connection failure shao.mingyin

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=20261114020249092lDPe3OpMiMq8nakVU-IJ6@zte.com.cn \
    --to=shao.mingyin@zte.com.cn \
    --cc=axboe@kernel.dk \
    --cc=hch@lst.de \
    --cc=justin.tee@broadcom.com \
    --cc=kbusch@kernel.org \
    --cc=kch@nvidia.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nvme@lists.infradead.org \
    --cc=nareshgottumukkala83@gmail.com \
    --cc=paul.ely@broadcom.com \
    --cc=sagi@grimberg.me \
    --cc=stable@vger.kernel.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®