[PATCH V3 0/2] nvmet-tcp: fix receive path error handling and state machine

[PATCH V3 0/2] nvmet-tcp: fix receive path error handling and state machine

[Maurizio Lombardi]

Patch 1 fixes a potential issue where network data could be read into an
uninitialized iterator. Currently, nvmet_tcp_build_pdu_iovec() returns void,
meaning callers are unaware if an out-of-bounds PDU length or offset triggers
an early return. Consequently, callers blindly overwrite the queue state to
NVMET_TCP_RECV_DATA. This patch modifies the function to return an error code,
shifting the handling responsibility to the callers to ensure proper socket
teardown.

Patch 2 cleans up redundant, localized calls to nvmet_tcp_fatal_error() scattered
across the receive path. It delegates the responsibility of executing the fatal
error function to the top-level caller by bubbling up the error codes.

V3: remove nvmet_tcp_fatal_error(), fold it into nvmet_tcp_socket_error()

V2: some cosmetic changes to Patch 1, no functional changes.

Maurizio Lombardi (2):
  nvmet-tcp: propagate nvmet_tcp_build_pdu_iovec() errors to its callers
  nvmet-tcp: remove redundant calls to nvmet_tcp_fatal_error()

 drivers/nvme/target/tcp.c | 88 ++++++++++++++++-----------------------
 1 file changed, 36 insertions(+), 52 deletions(-)

-- 
2.53.0

[PATCH V3 1/2] nvmet-tcp: propagate nvmet_tcp_build_pdu_iovec() errors to its callers

[Maurizio Lombardi]

Currently, when nvmet_tcp_build_pdu_iovec() detects an out-of-bounds
PDU length or offset, it triggers nvmet_tcp_fatal_error(cmd->queue)
and returns early. However, because the function returns void, the
callers are entirely unaware that a fatal error has occurred and
that the cmd->recv_msg.msg_iter was left uninitialized.

Callers such as nvmet_tcp_handle_h2c_data_pdu() proceed to blindly
overwrite the queue state with queue->rcv_state = NVMET_TCP_RECV_DATA
Consequently, the socket receiving loop may attempt to read incoming
network data into the uninitialized iterator.

Fix this by shifting the error handling responsibility to the callers.

Fixes: 52a0a9854934 ("nvmet-tcp: add bounds checks in nvmet_tcp_build_pdu_iovec")
Reviewed-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: Maurizio Lombardi <mlombard@redhat.com>
---
 drivers/nvme/target/tcp.c | 51 ++++++++++++++++++++++-----------------
 1 file changed, 29 insertions(+), 22 deletions(-)

diff --git a/drivers/nvme/target/tcp.c b/drivers/nvme/target/tcp.c
index acc71a26733f..1fbf12df1183 100644
--- a/drivers/nvme/target/tcp.c
+++ b/drivers/nvme/target/tcp.c
@@ -351,7 +351,7 @@ static void nvmet_tcp_free_cmd_buffers(struct nvmet_tcp_cmd *cmd)
 
 static void nvmet_tcp_fatal_error(struct nvmet_tcp_queue *queue);
 
-static void nvmet_tcp_build_pdu_iovec(struct nvmet_tcp_cmd *cmd)
+static int nvmet_tcp_build_pdu_iovec(struct nvmet_tcp_cmd *cmd)
 {
 	struct bio_vec *iov = cmd->iov;
 	struct scatterlist *sg;
@@ -364,22 +364,19 @@ static void nvmet_tcp_build_pdu_iovec(struct nvmet_tcp_cmd *cmd)
 	offset = cmd->rbytes_done;
 	cmd->sg_idx = offset / PAGE_SIZE;
 	sg_offset = offset % PAGE_SIZE;
-	if (!cmd->req.sg_cnt || cmd->sg_idx >= cmd->req.sg_cnt) {
-		nvmet_tcp_fatal_error(cmd->queue);
-		return;
-	}
+	if (!cmd->req.sg_cnt || cmd->sg_idx >= cmd->req.sg_cnt)
+		return -EPROTO;
+
 	sg = &cmd->req.sg[cmd->sg_idx];
 	sg_remaining = cmd->req.sg_cnt - cmd->sg_idx;
 
 	while (length) {
-		if (!sg_remaining) {
-			nvmet_tcp_fatal_error(cmd->queue);
-			return;
-		}
-		if (!sg->length || sg->length <= sg_offset) {
-			nvmet_tcp_fatal_error(cmd->queue);
-			return;
-		}
+		if (!sg_remaining)
+			return -EPROTO;
+
+		if (!sg->length || sg->length <= sg_offset)
+			return -EPROTO;
+
 		u32 iov_len = min_t(u32, length, sg->length - sg_offset);
 
 		bvec_set_page(iov, sg_page(sg), iov_len,
@@ -394,6 +391,7 @@ static void nvmet_tcp_build_pdu_iovec(struct nvmet_tcp_cmd *cmd)
 
 	iov_iter_bvec(&cmd->recv_msg.msg_iter, ITER_DEST, cmd->iov,
 		      nr_pages, cmd->pdu_len);
+	return 0;
 }
 
 static void nvmet_tcp_fatal_error(struct nvmet_tcp_queue *queue)
@@ -931,7 +929,7 @@ static int nvmet_tcp_handle_icreq(struct nvmet_tcp_queue *queue)
 	return 0;
 }
 
-static void nvmet_tcp_handle_req_failure(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);
@@ -947,19 +945,23 @@ static void nvmet_tcp_handle_req_failure(struct nvmet_tcp_queue *queue,
 	if (!nvme_is_write(cmd->req.cmd) || !data_len ||
 	    data_len > cmd->req.port->inline_data_size) {
 		nvmet_prepare_receive_pdu(queue);
-		return;
+		return 0;
 	}
 
 	ret = nvmet_tcp_map_data(cmd);
 	if (unlikely(ret)) {
 		pr_err("queue %d: failed to map data\n", queue->idx);
 		nvmet_tcp_fatal_error(queue);
-		return;
+		return -EPROTO;
 	}
 
 	queue->rcv_state = NVMET_TCP_RECV_DATA;
-	nvmet_tcp_build_pdu_iovec(cmd);
 	cmd->flags |= NVMET_TCP_F_INIT_FAILED;
+	ret = nvmet_tcp_build_pdu_iovec(cmd);
+	if (unlikely(ret))
+		pr_err("queue %d: failed to build PDU iovec\n", queue->idx);
+
+	return ret;
 }
 
 static int nvmet_tcp_handle_h2c_data_pdu(struct nvmet_tcp_queue *queue)
@@ -1011,7 +1013,10 @@ static int nvmet_tcp_handle_h2c_data_pdu(struct nvmet_tcp_queue *queue)
 		goto err_proto;
 	}
 	cmd->pdu_recv = 0;
-	nvmet_tcp_build_pdu_iovec(cmd);
+	if (unlikely(nvmet_tcp_build_pdu_iovec(cmd))) {
+		pr_err("queue %d: failed to build PDU iovec\n", queue->idx);
+		goto err_proto;
+	}
 	queue->cmd = cmd;
 	queue->rcv_state = NVMET_TCP_RECV_DATA;
 
@@ -1074,8 +1079,7 @@ static int nvmet_tcp_done_recv_pdu(struct nvmet_tcp_queue *queue)
 			le32_to_cpu(req->cmd->common.dptr.sgl.length),
 			le16_to_cpu(req->cqe->status));
 
-		nvmet_tcp_handle_req_failure(queue, queue->cmd, req);
-		return 0;
+		return nvmet_tcp_handle_req_failure(queue, queue->cmd, req);
 	}
 
 	ret = nvmet_tcp_map_data(queue->cmd);
@@ -1092,8 +1096,11 @@ static int nvmet_tcp_done_recv_pdu(struct nvmet_tcp_queue *queue)
 	if (nvmet_tcp_need_data_in(queue->cmd)) {
 		if (nvmet_tcp_has_inline_data(queue->cmd)) {
 			queue->rcv_state = NVMET_TCP_RECV_DATA;
-			nvmet_tcp_build_pdu_iovec(queue->cmd);
-			return 0;
+			ret = nvmet_tcp_build_pdu_iovec(queue->cmd);
+			if (unlikely(ret))
+				pr_err("queue %d: failed to build PDU iovec\n",
+					queue->idx);
+			return ret;
 		}
 		/* send back R2T */
 		nvmet_tcp_queue_response(&queue->cmd->req);
-- 
2.53.0

Re: [PATCH V3 1/2] nvmet-tcp: propagate nvmet_tcp_build_pdu_iovec() errors to its callers

[yunje shin]

Looks good to me.

Reviewed-by: Yunje Shin <ioerts@kookmin.ac.kr>

On Mon, Mar 16, 2026 at 11:39 PM Maurizio Lombardi <mlombard@redhat.com> wrote:
>
> Currently, when nvmet_tcp_build_pdu_iovec() detects an out-of-bounds
> PDU length or offset, it triggers nvmet_tcp_fatal_error(cmd->queue)
> and returns early. However, because the function returns void, the
> callers are entirely unaware that a fatal error has occurred and
> that the cmd->recv_msg.msg_iter was left uninitialized.
>
> Callers such as nvmet_tcp_handle_h2c_data_pdu() proceed to blindly
> overwrite the queue state with queue->rcv_state = NVMET_TCP_RECV_DATA
> Consequently, the socket receiving loop may attempt to read incoming
> network data into the uninitialized iterator.
>
> Fix this by shifting the error handling responsibility to the callers.
>
> Fixes: 52a0a9854934 ("nvmet-tcp: add bounds checks in nvmet_tcp_build_pdu_iovec")
> Reviewed-by: Hannes Reinecke <hare@suse.de>
> Signed-off-by: Maurizio Lombardi <mlombard@redhat.com>
> ---
>  drivers/nvme/target/tcp.c | 51 ++++++++++++++++++++++-----------------
>  1 file changed, 29 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/nvme/target/tcp.c b/drivers/nvme/target/tcp.c
> index acc71a26733f..1fbf12df1183 100644
> --- a/drivers/nvme/target/tcp.c
> +++ b/drivers/nvme/target/tcp.c
> @@ -351,7 +351,7 @@ static void nvmet_tcp_free_cmd_buffers(struct nvmet_tcp_cmd *cmd)
>
>  static void nvmet_tcp_fatal_error(struct nvmet_tcp_queue *queue);
>
> -static void nvmet_tcp_build_pdu_iovec(struct nvmet_tcp_cmd *cmd)
> +static int nvmet_tcp_build_pdu_iovec(struct nvmet_tcp_cmd *cmd)
>  {
>         struct bio_vec *iov = cmd->iov;
>         struct scatterlist *sg;
> @@ -364,22 +364,19 @@ static void nvmet_tcp_build_pdu_iovec(struct nvmet_tcp_cmd *cmd)
>         offset = cmd->rbytes_done;
>         cmd->sg_idx = offset / PAGE_SIZE;
>         sg_offset = offset % PAGE_SIZE;
> -       if (!cmd->req.sg_cnt || cmd->sg_idx >= cmd->req.sg_cnt) {
> -               nvmet_tcp_fatal_error(cmd->queue);
> -               return;
> -       }
> +       if (!cmd->req.sg_cnt || cmd->sg_idx >= cmd->req.sg_cnt)
> +               return -EPROTO;
> +
>         sg = &cmd->req.sg[cmd->sg_idx];
>         sg_remaining = cmd->req.sg_cnt - cmd->sg_idx;
>
>         while (length) {
> -               if (!sg_remaining) {
> -                       nvmet_tcp_fatal_error(cmd->queue);
> -                       return;
> -               }
> -               if (!sg->length || sg->length <= sg_offset) {
> -                       nvmet_tcp_fatal_error(cmd->queue);
> -                       return;
> -               }
> +               if (!sg_remaining)
> +                       return -EPROTO;
> +
> +               if (!sg->length || sg->length <= sg_offset)
> +                       return -EPROTO;
> +
>                 u32 iov_len = min_t(u32, length, sg->length - sg_offset);
>
>                 bvec_set_page(iov, sg_page(sg), iov_len,
> @@ -394,6 +391,7 @@ static void nvmet_tcp_build_pdu_iovec(struct nvmet_tcp_cmd *cmd)
>
>         iov_iter_bvec(&cmd->recv_msg.msg_iter, ITER_DEST, cmd->iov,
>                       nr_pages, cmd->pdu_len);
> +       return 0;
>  }
>
>  static void nvmet_tcp_fatal_error(struct nvmet_tcp_queue *queue)
> @@ -931,7 +929,7 @@ static int nvmet_tcp_handle_icreq(struct nvmet_tcp_queue *queue)
>         return 0;
>  }
>
> -static void nvmet_tcp_handle_req_failure(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);
> @@ -947,19 +945,23 @@ static void nvmet_tcp_handle_req_failure(struct nvmet_tcp_queue *queue,
>         if (!nvme_is_write(cmd->req.cmd) || !data_len ||
>             data_len > cmd->req.port->inline_data_size) {
>                 nvmet_prepare_receive_pdu(queue);
> -               return;
> +               return 0;
>         }
>
>         ret = nvmet_tcp_map_data(cmd);
>         if (unlikely(ret)) {
>                 pr_err("queue %d: failed to map data\n", queue->idx);
>                 nvmet_tcp_fatal_error(queue);
> -               return;
> +               return -EPROTO;
>         }
>
>         queue->rcv_state = NVMET_TCP_RECV_DATA;
> -       nvmet_tcp_build_pdu_iovec(cmd);
>         cmd->flags |= NVMET_TCP_F_INIT_FAILED;
> +       ret = nvmet_tcp_build_pdu_iovec(cmd);
> +       if (unlikely(ret))
> +               pr_err("queue %d: failed to build PDU iovec\n", queue->idx);
> +
> +       return ret;
>  }
>
>  static int nvmet_tcp_handle_h2c_data_pdu(struct nvmet_tcp_queue *queue)
> @@ -1011,7 +1013,10 @@ static int nvmet_tcp_handle_h2c_data_pdu(struct nvmet_tcp_queue *queue)
>                 goto err_proto;
>         }
>         cmd->pdu_recv = 0;
> -       nvmet_tcp_build_pdu_iovec(cmd);
> +       if (unlikely(nvmet_tcp_build_pdu_iovec(cmd))) {
> +               pr_err("queue %d: failed to build PDU iovec\n", queue->idx);
> +               goto err_proto;
> +       }
>         queue->cmd = cmd;
>         queue->rcv_state = NVMET_TCP_RECV_DATA;
>
> @@ -1074,8 +1079,7 @@ static int nvmet_tcp_done_recv_pdu(struct nvmet_tcp_queue *queue)
>                         le32_to_cpu(req->cmd->common.dptr.sgl.length),
>                         le16_to_cpu(req->cqe->status));
>
> -               nvmet_tcp_handle_req_failure(queue, queue->cmd, req);
> -               return 0;
> +               return nvmet_tcp_handle_req_failure(queue, queue->cmd, req);
>         }
>
>         ret = nvmet_tcp_map_data(queue->cmd);
> @@ -1092,8 +1096,11 @@ static int nvmet_tcp_done_recv_pdu(struct nvmet_tcp_queue *queue)
>         if (nvmet_tcp_need_data_in(queue->cmd)) {
>                 if (nvmet_tcp_has_inline_data(queue->cmd)) {
>                         queue->rcv_state = NVMET_TCP_RECV_DATA;
> -                       nvmet_tcp_build_pdu_iovec(queue->cmd);
> -                       return 0;
> +                       ret = nvmet_tcp_build_pdu_iovec(queue->cmd);
> +                       if (unlikely(ret))
> +                               pr_err("queue %d: failed to build PDU iovec\n",
> +                                       queue->idx);
> +                       return ret;
>                 }
>                 /* send back R2T */
>                 nvmet_tcp_queue_response(&queue->cmd->req);
> --
> 2.53.0
>

[PATCH V3 2/2] nvmet-tcp: remove redundant calls to nvmet_tcp_fatal_error()

[Maurizio Lombardi]

Executing nvmet_tcp_fatal_error() is generally the responsibility
of the caller (nvmet_tcp_try_recv); all other functions should
just return the error code.

Remove the nvmet_tcp_fatal_error() function, it's not needed
anymore.

Signed-off-by: Maurizio Lombardi <mlombard@redhat.com>
---
 drivers/nvme/target/tcp.c | 37 +++++++------------------------------
 1 file changed, 7 insertions(+), 30 deletions(-)

diff --git a/drivers/nvme/target/tcp.c b/drivers/nvme/target/tcp.c
index 1fbf12df1183..462010654059 100644
--- a/drivers/nvme/target/tcp.c
+++ b/drivers/nvme/target/tcp.c
@@ -349,8 +349,6 @@ static void nvmet_tcp_free_cmd_buffers(struct nvmet_tcp_cmd *cmd)
 	cmd->req.sg = NULL;
 }
 
-static void nvmet_tcp_fatal_error(struct nvmet_tcp_queue *queue);
-
 static int nvmet_tcp_build_pdu_iovec(struct nvmet_tcp_cmd *cmd)
 {
 	struct bio_vec *iov = cmd->iov;
@@ -394,22 +392,13 @@ static int nvmet_tcp_build_pdu_iovec(struct nvmet_tcp_cmd *cmd)
 	return 0;
 }
 
-static void nvmet_tcp_fatal_error(struct nvmet_tcp_queue *queue)
-{
-	queue->rcv_state = NVMET_TCP_RECV_ERR;
-	if (queue->nvme_sq.ctrl)
-		nvmet_ctrl_fatal_error(queue->nvme_sq.ctrl);
-	else
-		kernel_sock_shutdown(queue->sock, SHUT_RDWR);
-}
-
 static void nvmet_tcp_socket_error(struct nvmet_tcp_queue *queue, int status)
 {
 	queue->rcv_state = NVMET_TCP_RECV_ERR;
-	if (status == -EPIPE || status == -ECONNRESET)
+	if (status == -EPIPE || status == -ECONNRESET || !queue->nvme_sq.ctrl)
 		kernel_sock_shutdown(queue->sock, SHUT_RDWR);
 	else
-		nvmet_tcp_fatal_error(queue);
+		nvmet_ctrl_fatal_error(queue->nvme_sq.ctrl);
 }
 
 static int nvmet_tcp_map_data(struct nvmet_tcp_cmd *cmd)
@@ -885,7 +874,6 @@ static int nvmet_tcp_handle_icreq(struct nvmet_tcp_queue *queue)
 	if (le32_to_cpu(icreq->hdr.plen) != sizeof(struct nvme_tcp_icreq_pdu)) {
 		pr_err("bad nvme-tcp pdu length (%d)\n",
 			le32_to_cpu(icreq->hdr.plen));
-		nvmet_tcp_fatal_error(queue);
 		return -EPROTO;
 	}
 
@@ -951,7 +939,6 @@ static int nvmet_tcp_handle_req_failure(struct nvmet_tcp_queue *queue,
 	ret = nvmet_tcp_map_data(cmd);
 	if (unlikely(ret)) {
 		pr_err("queue %d: failed to map data\n", queue->idx);
-		nvmet_tcp_fatal_error(queue);
 		return -EPROTO;
 	}
 
@@ -1024,7 +1011,6 @@ static int nvmet_tcp_handle_h2c_data_pdu(struct nvmet_tcp_queue *queue)
 
 err_proto:
 	/* FIXME: use proper transport errors */
-	nvmet_tcp_fatal_error(queue);
 	return -EPROTO;
 }
 
@@ -1039,7 +1025,6 @@ static int nvmet_tcp_done_recv_pdu(struct nvmet_tcp_queue *queue)
 		if (hdr->type != nvme_tcp_icreq) {
 			pr_err("unexpected pdu type (%d) before icreq\n",
 				hdr->type);
-			nvmet_tcp_fatal_error(queue);
 			return -EPROTO;
 		}
 		return nvmet_tcp_handle_icreq(queue);
@@ -1048,7 +1033,6 @@ static int nvmet_tcp_done_recv_pdu(struct nvmet_tcp_queue *queue)
 	if (unlikely(hdr->type == nvme_tcp_icreq)) {
 		pr_err("queue %d: received icreq pdu in state %d\n",
 			queue->idx, queue->state);
-		nvmet_tcp_fatal_error(queue);
 		return -EPROTO;
 	}
 
@@ -1065,7 +1049,6 @@ static int nvmet_tcp_done_recv_pdu(struct nvmet_tcp_queue *queue)
 		pr_err("queue %d: out of commands (%d) send_list_len: %d, opcode: %d",
 			queue->idx, queue->nr_cmds, queue->send_list_len,
 			nvme_cmd->common.opcode);
-		nvmet_tcp_fatal_error(queue);
 		return -ENOMEM;
 	}
 
@@ -1086,9 +1069,9 @@ static int nvmet_tcp_done_recv_pdu(struct nvmet_tcp_queue *queue)
 	if (unlikely(ret)) {
 		pr_err("queue %d: failed to map data\n", queue->idx);
 		if (nvmet_tcp_has_inline_data(queue->cmd))
-			nvmet_tcp_fatal_error(queue);
-		else
-			nvmet_req_complete(req, ret);
+			return -EPROTO;
+
+		nvmet_req_complete(req, ret);
 		ret = -EAGAIN;
 		goto out;
 	}
@@ -1211,7 +1194,6 @@ static int nvmet_tcp_try_recv_pdu(struct nvmet_tcp_queue *queue)
 
 		if (unlikely(!nvmet_tcp_pdu_valid(hdr->type))) {
 			pr_err("unexpected pdu type %d\n", hdr->type);
-			nvmet_tcp_fatal_error(queue);
 			return -EIO;
 		}
 
@@ -1225,16 +1207,12 @@ static int nvmet_tcp_try_recv_pdu(struct nvmet_tcp_queue *queue)
 	}
 
 	if (queue->hdr_digest &&
-	    nvmet_tcp_verify_hdgst(queue, &queue->pdu, hdr->hlen)) {
-		nvmet_tcp_fatal_error(queue); /* fatal */
+	    nvmet_tcp_verify_hdgst(queue, &queue->pdu, hdr->hlen))
 		return -EPROTO;
-	}
 
 	if (queue->data_digest &&
-	    nvmet_tcp_check_ddgst(queue, &queue->pdu)) {
-		nvmet_tcp_fatal_error(queue); /* fatal */
+	    nvmet_tcp_check_ddgst(queue, &queue->pdu))
 		return -EPROTO;
-	}
 
 	return nvmet_tcp_done_recv_pdu(queue);
 }
@@ -1319,7 +1297,6 @@ static int nvmet_tcp_try_recv_ddgst(struct nvmet_tcp_queue *queue)
 			le32_to_cpu(cmd->exp_ddgst));
 		nvmet_req_uninit(&cmd->req);
 		nvmet_tcp_free_cmd_buffers(cmd);
-		nvmet_tcp_fatal_error(queue);
 		ret = -EPROTO;
 		goto out;
 	}
-- 
2.53.0

Re: [PATCH V3 2/2] nvmet-tcp: remove redundant calls to nvmet_tcp_fatal_error()

[Hannes Reinecke]

On 3/16/26 15:39, Maurizio Lombardi wrote:
> Executing nvmet_tcp_fatal_error() is generally the responsibility
> of the caller (nvmet_tcp_try_recv); all other functions should
> just return the error code.
> 
> Remove the nvmet_tcp_fatal_error() function, it's not needed
> anymore.
> 
> Signed-off-by: Maurizio Lombardi <mlombard@redhat.com>
> ---
>   drivers/nvme/target/tcp.c | 37 +++++++------------------------------
>   1 file changed, 7 insertions(+), 30 deletions(-)
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@suse.de                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich

Re: [PATCH V3 0/2] nvmet-tcp: fix receive path error handling and state machine

[Chaitanya Kulkarni]

On 3/16/26 07:39, Maurizio Lombardi wrote:
> Patch 1 fixes a potential issue where network data could be read into an
> uninitialized iterator. Currently, nvmet_tcp_build_pdu_iovec() returns void,
> meaning callers are unaware if an out-of-bounds PDU length or offset triggers
> an early return. Consequently, callers blindly overwrite the queue state to
> NVMET_TCP_RECV_DATA. This patch modifies the function to return an error code,
> shifting the handling responsibility to the callers to ensure proper socket
> teardown.
>
> Patch 2 cleans up redundant, localized calls to nvmet_tcp_fatal_error() scattered
> across the receive path. It delegates the responsibility of executing the fatal
> error function to the top-level caller by bubbling up the error codes.
>
> V3: remove nvmet_tcp_fatal_error(), fold it into nvmet_tcp_socket_error()
>
> V2: some cosmetic changes to Patch 1, no functional changes.
>
> Maurizio Lombardi (2):
>    nvmet-tcp: propagate nvmet_tcp_build_pdu_iovec() errors to its callers
>    nvmet-tcp: remove redundant calls to nvmet_tcp_fatal_error()
>
>   drivers/nvme/target/tcp.c | 88 ++++++++++++++++-----------------------
>   1 file changed, 36 insertions(+), 52 deletions(-)
>
Assuming that you have ran block test on this series with nvme_trtype=tcp
and this series is passing without any problem.

With that :-

Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>

-ck

Re: [PATCH V3 0/2] nvmet-tcp: fix receive path error handling and state machine

[Maurizio Lombardi]

On Wed Mar 18, 2026 at 1:24 AM CET, Chaitanya Kulkarni wrote:
> On 3/16/26 07:39, Maurizio Lombardi wrote:
>> Patch 1 fixes a potential issue where network data could be read into an
>> uninitialized iterator. Currently, nvmet_tcp_build_pdu_iovec() returns void,
>> meaning callers are unaware if an out-of-bounds PDU length or offset triggers
>> an early return. Consequently, callers blindly overwrite the queue state to
>> NVMET_TCP_RECV_DATA. This patch modifies the function to return an error code,
>> shifting the handling responsibility to the callers to ensure proper socket
>> teardown.
>>
>> Patch 2 cleans up redundant, localized calls to nvmet_tcp_fatal_error() scattered
>> across the receive path. It delegates the responsibility of executing the fatal
>> error function to the top-level caller by bubbling up the error codes.
>>
>> V3: remove nvmet_tcp_fatal_error(), fold it into nvmet_tcp_socket_error()
>>
>> V2: some cosmetic changes to Patch 1, no functional changes.
>>
>> Maurizio Lombardi (2):
>>    nvmet-tcp: propagate nvmet_tcp_build_pdu_iovec() errors to its callers
>>    nvmet-tcp: remove redundant calls to nvmet_tcp_fatal_error()
>>
>>   drivers/nvme/target/tcp.c | 88 ++++++++++++++++-----------------------
>>   1 file changed, 36 insertions(+), 52 deletions(-)
>>
> Assuming that you have ran block test on this series with nvme_trtype=tcp
> and this series is passing without any problem.

Yes, Yi Zhang did run the blktests against this patchset

The only problem he reported is a circular locking
warning in the host driver that I think is due to the bug here:

https://lore.kernel.org/linux-nvme/f5065c2c-e30d-418d-8c7a-723e958b54c1@nvidia.com/T/#mb3a8789682514d5f63b711703d32fd6bd197bcec

No other issues were detected.

Thanks,
Maurizio

Re: [PATCH V3 0/2] nvmet-tcp: fix receive path error handling and state machine

[Maurizio Lombardi]

Hello Keith,

this patchset has been reviewed already, could you pick it up?

Thanks,
Maurizio

On Mon Mar 16, 2026 at 3:39 PM CET, Maurizio Lombardi wrote:
> Patch 1 fixes a potential issue where network data could be read into an
> uninitialized iterator. Currently, nvmet_tcp_build_pdu_iovec() returns void,
> meaning callers are unaware if an out-of-bounds PDU length or offset triggers
> an early return. Consequently, callers blindly overwrite the queue state to
> NVMET_TCP_RECV_DATA. This patch modifies the function to return an error code,
> shifting the handling responsibility to the callers to ensure proper socket
> teardown.
>
> Patch 2 cleans up redundant, localized calls to nvmet_tcp_fatal_error() scattered
> across the receive path. It delegates the responsibility of executing the fatal
> error function to the top-level caller by bubbling up the error codes.
>
> V3: remove nvmet_tcp_fatal_error(), fold it into nvmet_tcp_socket_error()
>
> V2: some cosmetic changes to Patch 1, no functional changes.
>
> Maurizio Lombardi (2):
>   nvmet-tcp: propagate nvmet_tcp_build_pdu_iovec() errors to its callers
>   nvmet-tcp: remove redundant calls to nvmet_tcp_fatal_error()
>
>  drivers/nvme/target/tcp.c | 88 ++++++++++++++++-----------------------
>  1 file changed, 36 insertions(+), 52 deletions(-)

Re: [PATCH V3 0/2] nvmet-tcp: fix receive path error handling and state machine

[Keith Busch]

On Mon, Mar 16, 2026 at 03:39:34PM +0100, Maurizio Lombardi wrote:
> Patch 1 fixes a potential issue where network data could be read into an
> uninitialized iterator. Currently, nvmet_tcp_build_pdu_iovec() returns void,
> meaning callers are unaware if an out-of-bounds PDU length or offset triggers
> an early return. Consequently, callers blindly overwrite the queue state to
> NVMET_TCP_RECV_DATA. This patch modifies the function to return an error code,
> shifting the handling responsibility to the callers to ensure proper socket
> teardown.
> 
> Patch 2 cleans up redundant, localized calls to nvmet_tcp_fatal_error() scattered
> across the receive path. It delegates the responsibility of executing the fatal
> error function to the top-level caller by bubbling up the error codes.

Thanks, applied to nvme-7.1.