[PATCH 0/2] nvme-tcp: handle TLS control records at connect and on receive errors

[PATCH 0/2] nvme-tcp: handle TLS control records at connect and on receive errors

[Martin Belanger via B4 Relay]

A TLS 1.3 peer can send records that carry no application data, for
example a NewSessionTicket right after the handshake. nvme-tcp does not
handle them correctly. This series fixes two cases.

Patch 1: when read_sock() returns an error, nvme-tcp does not start
error recovery. The queue stops receiving until a command times out.
With a NewSessionTicket on a live queue, I/O stalls until the I/O
timeout (30 seconds by default). With this patch, recovery starts at
once. This also applies to plain TCP.

Patch 2: a NewSessionTicket that arrives before the ICResp fails the
connection with "failed to receive icresp, error -104". A target that
sends a ticket at once, such as an OpenSSL-based target, cannot be
reached. With this patch, the ticket is discarded and the connection
succeeds. An alert in place of the ICResp is now reported with its code,
instead of as error -104.

These patches replace patch 1/2 of Hannes's "nvme,tls: minimal handling
for TLS records":
https://lore.kernel.org/all/20260922133708.84522-1-hare@kernel.org/

Patch 2/2 of that series (the net/tls errno change) will be posted to
netdev separately after 7.4-rc1. It will not change
tls_sw_splice_read(), because that errno is visible to user space.
After that, a follow-up nvme-tcp patch will read control records on a
live queue, so a NewSessionTicket no longer causes a reconnect.

Tested with host and target in one VM, with a test-only change to
net/tls that makes the target send a NewSessionTicket or an alert.
Also tested on plain TCP, and built with CONFIG_NVME_TCP_TLS=n.

Signed-off-by: Martin Belanger <martin.belanger@dell.com>
---
Martin Belanger (2):
      nvme-tcp: start error recovery when read_sock fails
      nvme-tcp: skip TLS control records while waiting for the ICResp

 drivers/nvme/host/tcp.c | 173 +++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 150 insertions(+), 23 deletions(-)
---
base-commit: 6b4b543fa18757297ac756a6fe6e64f506b7844a
change-id: 20261026-nvme-tcp-ctrlrec-fixes-5f1cd54073ba

Best regards,
--  
Martin Belanger <martin.nvme.dev@gmail.com>

[PATCH 0/2] nvme-tcp: handle TLS control records at connect and on receive errors

[Martin Belanger]

A TLS 1.3 peer can send records that carry no application data, for
example a NewSessionTicket right after the handshake. nvme-tcp does not
handle them correctly. This series fixes two cases.

Patch 1: when read_sock() returns an error, nvme-tcp does not start
error recovery. The queue stops receiving until a command times out.
With a NewSessionTicket on a live queue, I/O stalls until the I/O
timeout (30 seconds by default). With this patch, recovery starts at
once. This also applies to plain TCP.

Patch 2: a NewSessionTicket that arrives before the ICResp fails the
connection with "failed to receive icresp, error -104". A target that
sends a ticket at once, such as an OpenSSL-based target, cannot be
reached. With this patch, the ticket is discarded and the connection
succeeds. An alert in place of the ICResp is now reported with its code,
instead of as error -104.

These patches replace patch 1/2 of Hannes's "nvme,tls: minimal handling
for TLS records":
https://lore.kernel.org/all/20260922133708.84522-1-hare@kernel.org/

Patch 2/2 of that series (the net/tls errno change) will be posted to
netdev separately after 7.4-rc1. It will not change
tls_sw_splice_read(), because that errno is visible to user space.
After that, a follow-up nvme-tcp patch will read control records on a
live queue, so a NewSessionTicket no longer causes a reconnect.

Tested with host and target in one VM, with a test-only change to
net/tls that makes the target send a NewSessionTicket or an alert.
Also tested on plain TCP, and built with CONFIG_NVME_TCP_TLS=n.

Signed-off-by: Martin Belanger <martin.belanger@dell.com>
---
Martin Belanger (2):
      nvme-tcp: start error recovery when read_sock fails
      nvme-tcp: skip TLS control records while waiting for the ICResp

 drivers/nvme/host/tcp.c | 173 +++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 150 insertions(+), 23 deletions(-)
---
base-commit: 6b4b543fa18757297ac756a6fe6e64f506b7844a
change-id: 20261026-nvme-tcp-ctrlrec-fixes-5f1cd54073ba

Best regards,
--  
Martin Belanger <martin.nvme.dev@gmail.com>

[PATCH 1/2] nvme-tcp: start error recovery when read_sock fails

[Martin Belanger via B4 Relay]

From: Martin Belanger <martin.belanger@dell.com>

nvme_tcp_recv_skb() disables the queue and starts error recovery for
errors it detects itself. An error returned by the transport reaches
nvme_tcp_try_recv() without either action.

nvme_tcp_io_work() returns on a negative result and does not requeue
itself, so the queue stops receiving and remains stalled until a command
times out.

Report the error and start error recovery when the queue is still
enabled, which indicates that the error was not handled by the actor.

Assisted-by: LLM
Signed-off-by: Martin Belanger <martin.belanger@dell.com>
---
 drivers/nvme/host/tcp.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
index 7190eb10c248..a03476f0f69d 100644
--- a/drivers/nvme/host/tcp.c
+++ b/drivers/nvme/host/tcp.c
@@ -1431,7 +1431,21 @@ static int nvme_tcp_try_recv(struct nvme_tcp_queue *queue)
 	queue->nr_cqe = 0;
 	consumed = sock->ops->read_sock(sk, &rd_desc, nvme_tcp_recv_skb);
 	release_sock(sk);
-	return consumed == -EAGAIN ? 0 : consumed;
+	if (consumed == -EAGAIN)
+		return 0;
+
+	/* nvme_tcp_recv_skb() does both of these itself, so a queue that is
+	 * still enabled failed inside read_sock() instead
+	 */
+	if (unlikely(consumed < 0 && queue->rd_enabled)) {
+		dev_err(queue->ctrl->ctrl.device,
+			"queue %d: receive failed: %d\n",
+			nvme_tcp_queue_id(queue), consumed);
+		queue->rd_enabled = false;
+		nvme_tcp_error_recovery(&queue->ctrl->ctrl);
+	}
+
+	return consumed;
 }
 
 static void nvme_tcp_io_work(struct work_struct *w)

-- 
2.53.0

[PATCH 1/2] nvme-tcp: start error recovery when read_sock fails

[Martin Belanger]

From: Martin Belanger <martin.belanger@dell.com>

nvme_tcp_recv_skb() disables the queue and starts error recovery for
errors it detects itself. An error returned by the transport reaches
nvme_tcp_try_recv() without either action.

nvme_tcp_io_work() returns on a negative result and does not requeue
itself, so the queue stops receiving and remains stalled until a command
times out.

Report the error and start error recovery when the queue is still
enabled, which indicates that the error was not handled by the actor.

Assisted-by: LLM
Signed-off-by: Martin Belanger <martin.belanger@dell.com>
---
 drivers/nvme/host/tcp.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
index 7190eb10c248..a03476f0f69d 100644
--- a/drivers/nvme/host/tcp.c
+++ b/drivers/nvme/host/tcp.c
@@ -1431,7 +1431,21 @@ static int nvme_tcp_try_recv(struct nvme_tcp_queue *queue)
 	queue->nr_cqe = 0;
 	consumed = sock->ops->read_sock(sk, &rd_desc, nvme_tcp_recv_skb);
 	release_sock(sk);
-	return consumed == -EAGAIN ? 0 : consumed;
+	if (consumed == -EAGAIN)
+		return 0;
+
+	/* nvme_tcp_recv_skb() does both of these itself, so a queue that is
+	 * still enabled failed inside read_sock() instead
+	 */
+	if (unlikely(consumed < 0 && queue->rd_enabled)) {
+		dev_err(queue->ctrl->ctrl.device,
+			"queue %d: receive failed: %d\n",
+			nvme_tcp_queue_id(queue), consumed);
+		queue->rd_enabled = false;
+		nvme_tcp_error_recovery(&queue->ctrl->ctrl);
+	}
+
+	return consumed;
 }
 
 static void nvme_tcp_io_work(struct work_struct *w)

-- 
2.53.0

Re: [PATCH 1/2] nvme-tcp: start error recovery when read_sock fails

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] nvme-tcp: race on queue->rd_enabled after release_sock() in nvme_tcp_try_recv()
--

commit 3915b73f53c8a994bf45e1efec593bc2bb4f9f8b
Author: Martin Belanger <martin.belanger@dell.com>

nvme-tcp: start error recovery when read_sock fails

This commit updates nvme_tcp_try_recv() to log an error, disable the queue,
and trigger controller error recovery when sock->ops->read_sock() returns a
negative error code other than -EAGAIN and queue->rd_enabled is still set.

> diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
> index 7190eb10c2481..a03476f0f69d5 100644
> --- a/drivers/nvme/host/tcp.c
> +++ b/drivers/nvme/host/tcp.c
> @@ -1431,7 +1431,21 @@ static int nvme_tcp_try_recv(struct nvme_tcp_queue *queue)
>  	queue->nr_cqe = 0;
>  	consumed = sock->ops->read_sock(sk, &rd_desc, nvme_tcp_recv_skb);
>  	release_sock(sk);
> -	return consumed == -EAGAIN ? 0 : consumed;
> +	if (consumed == -EAGAIN)
> +		return 0;
> +
> +	/* nvme_tcp_recv_skb() does both of these itself, so a queue that is
> +	 * still enabled failed inside read_sock() instead
> +	 */
> +	if (unlikely(consumed < 0 && queue->rd_enabled)) {
> +		dev_err(queue->ctrl->ctrl.device,
> +			"queue %d: receive failed: %d\n",
> +			nvme_tcp_queue_id(queue), consumed);
> +		queue->rd_enabled = false;
> +		nvme_tcp_error_recovery(&queue->ctrl->ctrl);
> +	}

[Severity: Medium]
Should the check and clearing of queue->rd_enabled happen before calling
release_sock(sk) in drivers/nvme/host/tcp.c:nvme_tcp_try_recv()?

In nvme_tcp_recv_skb(), queue->rd_enabled is checked, cleared, and followed
by nvme_tcp_error_recovery() while lock_sock(sk) is held:

drivers/nvme/host/tcp.c:nvme_tcp_recv_skb() {
	if (unlikely(!queue->rd_enabled))
		return -EFAULT;
...
		if (result) {
			dev_err(queue->ctrl->ctrl.device,
				"receive failed:  %d\n", result);
			queue->rd_enabled = false;
			nvme_tcp_error_recovery(&queue->ctrl->ctrl);
			return result;
		}
}

When sock->ops->read_sock() fails directly without hitting the error path in
nvme_tcp_recv_skb() (for example, tls_sw_read_sock() returning -EINVAL on a
non-data TLS record, -EBADMSG, -EKEYEXPIRED, -ENOMEM, or sock_error()),
calling release_sock(sk) first drains sk->sk_backlog via __release_sock(sk)
while queue->rd_enabled is still true.

Any TCP segments that arrived in softirq while the socket was owned will
trigger sk->sk_data_ready:

release_sock()
  -> __release_sock()
    -> nvme_tcp_data_ready()

drivers/nvme/host/tcp.c:nvme_tcp_data_ready() {
	read_lock_bh(&sk->sk_callback_lock);
	queue = sk->sk_user_data;
	if (likely(queue && queue->rd_enabled) &&
	    !test_bit(NVME_TCP_Q_POLLING, &queue->flags))
		queue_work_on(queue->io_cpu, nvme_tcp_wq, &queue->io_work);
	read_unlock_bh(&sk->sk_callback_lock);
}

Because queue->rd_enabled has not been cleared yet during release_sock(sk)
and the subsequent dev_err() call, nvme_tcp_data_ready() sees
queue->rd_enabled as true and re-queues queue->io_work on the failed queue.

In addition, nvme_tcp_try_recv() can be called concurrently from
nvme_tcp_poll() and nvme_tcp_io_work(), relying on lock_sock(sk) for
serialization:

drivers/nvme/host/tcp.c:nvme_tcp_poll() {
	set_bit(NVME_TCP_Q_POLLING, &queue->flags);
	if (sk_can_busy_loop(sk) && skb_queue_empty_lockless(&sk->sk_receive_queue))
		sk_busy_loop(sk, true);
	ret = nvme_tcp_try_recv(queue);
	clear_bit(NVME_TCP_Q_POLLING, &queue->flags);
}

Could releasing lock_sock(sk) before checking and clearing queue->rd_enabled
allow a concurrent nvme_tcp_try_recv() caller to acquire lock_sock(sk) while
queue->rd_enabled is still true, either logging duplicate errors or entering
nvme_tcp_recv_skb() after a transient read_sock() error?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261026-nvme-tcp-ctrlrec-fixes-v1-0-ebf6ff7eff34@gmail.com?part=1

Re: [PATCH 1/2] nvme-tcp: start error recovery when read_sock fails

[Hannes Reinecke]

On 10/26/26 7:09 PM, Martin Belanger via B4 Relay wrote:
> From: Martin Belanger <martin.belanger@dell.com>
> 
> nvme_tcp_recv_skb() disables the queue and starts error recovery for
> errors it detects itself. An error returned by the transport reaches
> nvme_tcp_try_recv() without either action.
> 
> nvme_tcp_io_work() returns on a negative result and does not requeue
> itself, so the queue stops receiving and remains stalled until a command
> times out.
> 
> Report the error and start error recovery when the queue is still
> enabled, which indicates that the error was not handled by the actor.
> 
> Assisted-by: LLM
> Signed-off-by: Martin Belanger <martin.belanger@dell.com>
> ---
>   drivers/nvme/host/tcp.c | 16 +++++++++++++++-
>   1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
> index 7190eb10c248..a03476f0f69d 100644
> --- a/drivers/nvme/host/tcp.c
> +++ b/drivers/nvme/host/tcp.c
> @@ -1431,7 +1431,21 @@ static int nvme_tcp_try_recv(struct nvme_tcp_queue *queue)
>   	queue->nr_cqe = 0;
>   	consumed = sock->ops->read_sock(sk, &rd_desc, nvme_tcp_recv_skb);
>   	release_sock(sk);
> -	return consumed == -EAGAIN ? 0 : consumed;
> +	if (consumed == -EAGAIN)
> +		return 0;
> +
> +	/* nvme_tcp_recv_skb() does both of these itself, so a queue that is
> +	 * still enabled failed inside read_sock() instead
> +	 */
> +	if (unlikely(consumed < 0 && queue->rd_enabled)) {
> +		dev_err(queue->ctrl->ctrl.device,
> +			"queue %d: receive failed: %d\n",
> +			nvme_tcp_queue_id(queue), consumed);
> +		queue->rd_enabled = false;
> +		nvme_tcp_error_recovery(&queue->ctrl->ctrl);
> +	}
> +
> +	return consumed;
>   }
>   
>   static void nvme_tcp_io_work(struct work_struct *w)
> 
Reviewed-by: Hannes Reinecke <hare@kernel.org>

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

[PATCH 2/2] nvme-tcp: skip TLS control records while waiting for the ICResp

[Martin Belanger via B4 Relay]

From: Martin Belanger <martin.belanger@dell.com>

A peer may send a NewSessionTicket as soon as the handshake completes,
ahead of the ICResp. nvme_tcp_init_connection() fails the connection on
any record that is not application data, so such a target cannot be
reached at all. An OpenSSL-based target does this by default.

Discard such a record and read again, up to a fixed number of records.
Report an alert, and fail the connection for any other record type.

The record type is now checked before the length. A control record
shorter than the ICResp was reported as -ECONNRESET, which hid the real
cause.

Fixes: 2837966ab2a8 ("nvme-tcp: control message handling for recvmsg()")
Assisted-by: LLM
Signed-off-by: Martin Belanger <martin.belanger@dell.com>
---
 drivers/nvme/host/tcp.c | 157 +++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 135 insertions(+), 22 deletions(-)

diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
index a03476f0f69d..2d361a9e8710 100644
--- a/drivers/nvme/host/tcp.c
+++ b/drivers/nvme/host/tcp.c
@@ -55,6 +55,12 @@ MODULE_PARM_DESC(tls_handshake_timeout,
 		 "nvme TLS handshake timeout in seconds (default 10)");
 #endif
 
+/* read a TLS control record in chunks of this size */
+#define NVME_TCP_TLS_REC_CHUNK 256
+
+/* TLS records read while waiting for the ICResp, the ICResp included */
+#define NVME_TCP_ICRESP_MAX_RECORDS 8
+
 static int nvme_tcp_cpu_queues[NR_CPUS];
 static DEFINE_SPINLOCK(nvme_tcp_cpu_queues_lock);
 
@@ -1548,12 +1554,138 @@ static void nvme_tcp_free_queue(struct nvme_ctrl *nctrl, int qid)
 	mutex_destroy(&queue->pf_cache_lock);
 }
 
+static int nvme_tcp_recv_tls_alert(struct nvme_tcp_queue *queue,
+				   struct msghdr *msg, int len)
+{
+	struct sock *sk = queue->sock->sk;
+	u8 level, description;
+
+	if (len < 2) {
+		dev_err(queue->ctrl->ctrl.device,
+			"queue %d: truncated TLS alert\n",
+			nvme_tcp_queue_id(queue));
+		return -ENOTCONN;
+	}
+
+	/* tls_alert_recv() reads through the iterator */
+	iov_iter_revert(&msg->msg_iter, len);
+	tls_alert_recv(sk, msg, &level, &description);
+
+	if (level == TLS_ALERT_LEVEL_FATAL) {
+		dev_err(queue->ctrl->ctrl.device,
+			"queue %d: fatal TLS alert %u\n",
+			nvme_tcp_queue_id(queue), description);
+		return -ENOTCONN;
+	}
+
+	/* a peer shutting down normally is not worth a warning */
+	if (description == TLS_ALERT_DESC_CLOSE_NOTIFY)
+		dev_dbg(queue->ctrl->ctrl.device,
+			"queue %d: TLS close_notify\n",
+			nvme_tcp_queue_id(queue));
+	else
+		dev_warn(queue->ctrl->ctrl.device,
+			 "queue %d: TLS alert %u\n",
+			 nvme_tcp_queue_id(queue), description);
+
+	return 0;
+}
+
+/*
+ * Discard a TLS record that arrived in place of the ICResp. A peer may send
+ * a NewSessionTicket as soon as the handshake completes.
+ *
+ * Returns 0 when the record was discarded and the ICResp can be read, or a
+ * negative errno if the connection cannot continue.
+ */
+static int nvme_tcp_skip_tls_record(struct nvme_tcp_queue *queue,
+				    struct msghdr *msg, u8 ctype, int len)
+{
+	u8 buf[NVME_TCP_TLS_REC_CHUNK];
+	int ret;
+
+	switch (ctype) {
+	case TLS_RECORD_TYPE_HANDSHAKE:
+		break;
+	case TLS_RECORD_TYPE_ALERT:
+		ret = nvme_tcp_recv_tls_alert(queue, msg, len);
+		return ret ? : -ENOTCONN;
+	default:
+		pr_err("queue %d: unhandled TLS record %u\n",
+		       nvme_tcp_queue_id(queue), ctype);
+		return -ENOTCONN;
+	}
+
+	/* the ICResp is in a later record, drain this one first */
+	while (!(msg->msg_flags & MSG_EOR)) {
+		struct kvec iov = {
+			.iov_base = buf,
+			.iov_len = sizeof(buf),
+		};
+		struct msghdr drain = {};
+
+		ret = kernel_recvmsg(queue->sock, &drain, &iov, 1, sizeof(buf),
+				     0);
+		if (ret <= 0) {
+			pr_warn("queue %d: failed to drain TLS record: %d\n",
+				nvme_tcp_queue_id(queue), ret);
+			return ret ? : -ECONNRESET;
+		}
+		msg->msg_flags = drain.msg_flags;
+	}
+
+	return 0;
+}
+
+/*
+ * Receive the ICResp. Over TLS, skip the records that a peer may send
+ * ahead of it.
+ *
+ * Returns the length received, or a negative errno.
+ */
+static int nvme_tcp_recv_icresp(struct nvme_tcp_queue *queue,
+				struct nvme_tcp_icresp_pdu *icresp)
+{
+	char cbuf[CMSG_LEN(sizeof(char))];
+	struct msghdr msg;
+	struct kvec iov;
+	u8 ctype;
+	int i, ret;
+
+	for (i = 0; i < NVME_TCP_ICRESP_MAX_RECORDS; i++) {
+		memset(&msg, 0, sizeof(msg));
+		memset(cbuf, 0, sizeof(cbuf));
+		iov.iov_base = icresp;
+		iov.iov_len = sizeof(*icresp);
+		if (nvme_tcp_queue_tls(queue)) {
+			msg.msg_control = cbuf;
+			msg.msg_controllen = sizeof(cbuf);
+		}
+		msg.msg_flags = MSG_WAITALL;
+		ret = kernel_recvmsg(queue->sock, &msg, &iov, 1,
+				     iov.iov_len, msg.msg_flags);
+		if (ret < 0 || !nvme_tcp_queue_tls(queue))
+			return ret;
+
+		ctype = tls_get_record_type(queue->sock->sk,
+					    (struct cmsghdr *)cbuf);
+		if (ctype == TLS_RECORD_TYPE_DATA)
+			return ret;
+
+		ret = nvme_tcp_skip_tls_record(queue, &msg, ctype, ret);
+		if (ret < 0)
+			return ret;
+	}
+
+	pr_err("queue %d: too many TLS records before the ICResp\n",
+	       nvme_tcp_queue_id(queue));
+	return -EPROTO;
+}
+
 static int nvme_tcp_init_connection(struct nvme_tcp_queue *queue)
 {
 	struct nvme_tcp_icreq_pdu *icreq;
 	struct nvme_tcp_icresp_pdu *icresp;
-	char cbuf[CMSG_LEN(sizeof(char))] = {};
-	u8 ctype;
 	struct msghdr msg = {};
 	struct kvec iov;
 	bool ctrl_hdgst, ctrl_ddgst;
@@ -1591,16 +1723,7 @@ static int nvme_tcp_init_connection(struct nvme_tcp_queue *queue)
 		goto free_icresp;
 	}
 
-	memset(&msg, 0, sizeof(msg));
-	iov.iov_base = icresp;
-	iov.iov_len = sizeof(*icresp);
-	if (nvme_tcp_queue_tls(queue)) {
-		msg.msg_control = cbuf;
-		msg.msg_controllen = sizeof(cbuf);
-	}
-	msg.msg_flags = MSG_WAITALL;
-	ret = kernel_recvmsg(queue->sock, &msg, &iov, 1,
-			iov.iov_len, msg.msg_flags);
+	ret = nvme_tcp_recv_icresp(queue, icresp);
 	if (ret >= 0 && ret < sizeof(*icresp))
 		ret = -ECONNRESET;
 	if (ret < 0) {
@@ -1608,16 +1731,6 @@ static int nvme_tcp_init_connection(struct nvme_tcp_queue *queue)
 			nvme_tcp_queue_id(queue), ret);
 		goto free_icresp;
 	}
-	ret = -ENOTCONN;
-	if (nvme_tcp_queue_tls(queue)) {
-		ctype = tls_get_record_type(queue->sock->sk,
-					    (struct cmsghdr *)cbuf);
-		if (ctype != TLS_RECORD_TYPE_DATA) {
-			pr_err("queue %d: unhandled TLS record %d\n",
-			       nvme_tcp_queue_id(queue), ctype);
-			goto free_icresp;
-		}
-	}
 	ret = -EINVAL;
 	if (icresp->hdr.type != nvme_tcp_icresp) {
 		pr_err("queue %d: bad type returned %d\n",

-- 
2.53.0

[PATCH 2/2] nvme-tcp: skip TLS control records while waiting for the ICResp

[Martin Belanger]

From: Martin Belanger <martin.belanger@dell.com>

A peer may send a NewSessionTicket as soon as the handshake completes,
ahead of the ICResp. nvme_tcp_init_connection() fails the connection on
any record that is not application data, so such a target cannot be
reached at all. An OpenSSL-based target does this by default.

Discard such a record and read again, up to a fixed number of records.
Report an alert, and fail the connection for any other record type.

The record type is now checked before the length. A control record
shorter than the ICResp was reported as -ECONNRESET, which hid the real
cause.

Fixes: 2837966ab2a8 ("nvme-tcp: control message handling for recvmsg()")
Assisted-by: LLM
Signed-off-by: Martin Belanger <martin.belanger@dell.com>
---
 drivers/nvme/host/tcp.c | 157 +++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 135 insertions(+), 22 deletions(-)

diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
index a03476f0f69d..2d361a9e8710 100644
--- a/drivers/nvme/host/tcp.c
+++ b/drivers/nvme/host/tcp.c
@@ -55,6 +55,12 @@ MODULE_PARM_DESC(tls_handshake_timeout,
 		 "nvme TLS handshake timeout in seconds (default 10)");
 #endif
 
+/* read a TLS control record in chunks of this size */
+#define NVME_TCP_TLS_REC_CHUNK 256
+
+/* TLS records read while waiting for the ICResp, the ICResp included */
+#define NVME_TCP_ICRESP_MAX_RECORDS 8
+
 static int nvme_tcp_cpu_queues[NR_CPUS];
 static DEFINE_SPINLOCK(nvme_tcp_cpu_queues_lock);
 
@@ -1548,12 +1554,138 @@ static void nvme_tcp_free_queue(struct nvme_ctrl *nctrl, int qid)
 	mutex_destroy(&queue->pf_cache_lock);
 }
 
+static int nvme_tcp_recv_tls_alert(struct nvme_tcp_queue *queue,
+				   struct msghdr *msg, int len)
+{
+	struct sock *sk = queue->sock->sk;
+	u8 level, description;
+
+	if (len < 2) {
+		dev_err(queue->ctrl->ctrl.device,
+			"queue %d: truncated TLS alert\n",
+			nvme_tcp_queue_id(queue));
+		return -ENOTCONN;
+	}
+
+	/* tls_alert_recv() reads through the iterator */
+	iov_iter_revert(&msg->msg_iter, len);
+	tls_alert_recv(sk, msg, &level, &description);
+
+	if (level == TLS_ALERT_LEVEL_FATAL) {
+		dev_err(queue->ctrl->ctrl.device,
+			"queue %d: fatal TLS alert %u\n",
+			nvme_tcp_queue_id(queue), description);
+		return -ENOTCONN;
+	}
+
+	/* a peer shutting down normally is not worth a warning */
+	if (description == TLS_ALERT_DESC_CLOSE_NOTIFY)
+		dev_dbg(queue->ctrl->ctrl.device,
+			"queue %d: TLS close_notify\n",
+			nvme_tcp_queue_id(queue));
+	else
+		dev_warn(queue->ctrl->ctrl.device,
+			 "queue %d: TLS alert %u\n",
+			 nvme_tcp_queue_id(queue), description);
+
+	return 0;
+}
+
+/*
+ * Discard a TLS record that arrived in place of the ICResp. A peer may send
+ * a NewSessionTicket as soon as the handshake completes.
+ *
+ * Returns 0 when the record was discarded and the ICResp can be read, or a
+ * negative errno if the connection cannot continue.
+ */
+static int nvme_tcp_skip_tls_record(struct nvme_tcp_queue *queue,
+				    struct msghdr *msg, u8 ctype, int len)
+{
+	u8 buf[NVME_TCP_TLS_REC_CHUNK];
+	int ret;
+
+	switch (ctype) {
+	case TLS_RECORD_TYPE_HANDSHAKE:
+		break;
+	case TLS_RECORD_TYPE_ALERT:
+		ret = nvme_tcp_recv_tls_alert(queue, msg, len);
+		return ret ? : -ENOTCONN;
+	default:
+		pr_err("queue %d: unhandled TLS record %u\n",
+		       nvme_tcp_queue_id(queue), ctype);
+		return -ENOTCONN;
+	}
+
+	/* the ICResp is in a later record, drain this one first */
+	while (!(msg->msg_flags & MSG_EOR)) {
+		struct kvec iov = {
+			.iov_base = buf,
+			.iov_len = sizeof(buf),
+		};
+		struct msghdr drain = {};
+
+		ret = kernel_recvmsg(queue->sock, &drain, &iov, 1, sizeof(buf),
+				     0);
+		if (ret <= 0) {
+			pr_warn("queue %d: failed to drain TLS record: %d\n",
+				nvme_tcp_queue_id(queue), ret);
+			return ret ? : -ECONNRESET;
+		}
+		msg->msg_flags = drain.msg_flags;
+	}
+
+	return 0;
+}
+
+/*
+ * Receive the ICResp. Over TLS, skip the records that a peer may send
+ * ahead of it.
+ *
+ * Returns the length received, or a negative errno.
+ */
+static int nvme_tcp_recv_icresp(struct nvme_tcp_queue *queue,
+				struct nvme_tcp_icresp_pdu *icresp)
+{
+	char cbuf[CMSG_LEN(sizeof(char))];
+	struct msghdr msg;
+	struct kvec iov;
+	u8 ctype;
+	int i, ret;
+
+	for (i = 0; i < NVME_TCP_ICRESP_MAX_RECORDS; i++) {
+		memset(&msg, 0, sizeof(msg));
+		memset(cbuf, 0, sizeof(cbuf));
+		iov.iov_base = icresp;
+		iov.iov_len = sizeof(*icresp);
+		if (nvme_tcp_queue_tls(queue)) {
+			msg.msg_control = cbuf;
+			msg.msg_controllen = sizeof(cbuf);
+		}
+		msg.msg_flags = MSG_WAITALL;
+		ret = kernel_recvmsg(queue->sock, &msg, &iov, 1,
+				     iov.iov_len, msg.msg_flags);
+		if (ret < 0 || !nvme_tcp_queue_tls(queue))
+			return ret;
+
+		ctype = tls_get_record_type(queue->sock->sk,
+					    (struct cmsghdr *)cbuf);
+		if (ctype == TLS_RECORD_TYPE_DATA)
+			return ret;
+
+		ret = nvme_tcp_skip_tls_record(queue, &msg, ctype, ret);
+		if (ret < 0)
+			return ret;
+	}
+
+	pr_err("queue %d: too many TLS records before the ICResp\n",
+	       nvme_tcp_queue_id(queue));
+	return -EPROTO;
+}
+
 static int nvme_tcp_init_connection(struct nvme_tcp_queue *queue)
 {
 	struct nvme_tcp_icreq_pdu *icreq;
 	struct nvme_tcp_icresp_pdu *icresp;
-	char cbuf[CMSG_LEN(sizeof(char))] = {};
-	u8 ctype;
 	struct msghdr msg = {};
 	struct kvec iov;
 	bool ctrl_hdgst, ctrl_ddgst;
@@ -1591,16 +1723,7 @@ static int nvme_tcp_init_connection(struct nvme_tcp_queue *queue)
 		goto free_icresp;
 	}
 
-	memset(&msg, 0, sizeof(msg));
-	iov.iov_base = icresp;
-	iov.iov_len = sizeof(*icresp);
-	if (nvme_tcp_queue_tls(queue)) {
-		msg.msg_control = cbuf;
-		msg.msg_controllen = sizeof(cbuf);
-	}
-	msg.msg_flags = MSG_WAITALL;
-	ret = kernel_recvmsg(queue->sock, &msg, &iov, 1,
-			iov.iov_len, msg.msg_flags);
+	ret = nvme_tcp_recv_icresp(queue, icresp);
 	if (ret >= 0 && ret < sizeof(*icresp))
 		ret = -ECONNRESET;
 	if (ret < 0) {
@@ -1608,16 +1731,6 @@ static int nvme_tcp_init_connection(struct nvme_tcp_queue *queue)
 			nvme_tcp_queue_id(queue), ret);
 		goto free_icresp;
 	}
-	ret = -ENOTCONN;
-	if (nvme_tcp_queue_tls(queue)) {
-		ctype = tls_get_record_type(queue->sock->sk,
-					    (struct cmsghdr *)cbuf);
-		if (ctype != TLS_RECORD_TYPE_DATA) {
-			pr_err("queue %d: unhandled TLS record %d\n",
-			       nvme_tcp_queue_id(queue), ctype);
-			goto free_icresp;
-		}
-	}
 	ret = -EINVAL;
 	if (icresp->hdr.type != nvme_tcp_icresp) {
 		pr_err("queue %d: bad type returned %d\n",

-- 
2.53.0

Re: [PATCH 2/2] nvme-tcp: skip TLS control records while waiting for the ICResp

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] nvme-tcp: missing msg_control in nvme_tcp_skip_tls_record() causes -EIO
- [Medium] nvme-tcp: unhandled EOF in nvme_tcp_recv_icresp() on TLS queues
--

commit e6540f770ca0b1e419e220cc7a58ea6cad9fb1eb
Author: Martin Belanger <martin.belanger@dell.com>

nvme-tcp: skip TLS control records while waiting for the ICResp

This commit updates nvme-tcp connection initialization to skip non-data TLS
records (such as TLS 1.3 NewSessionTicket handshake records) and process
TLS alerts while waiting for the ICResp PDU.

> diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
> index a03476f0f69d5..2d361a9e87101 100644
> --- a/drivers/nvme/host/tcp.c
> +++ b/drivers/nvme/host/tcp.c
[ ... ]
> @@ -1548,12 +1554,138 @@ static void nvme_tcp_free_queue(struct nvme_ctrl *nctrl, int qid)
[ ... ]
> +static int nvme_tcp_skip_tls_record(struct nvme_tcp_queue *queue,
> +				    struct msghdr *msg, u8 ctype, int len)
> +{
> +	u8 buf[NVME_TCP_TLS_REC_CHUNK];
> +	int ret;
> +
> +	switch (ctype) {
> +	case TLS_RECORD_TYPE_HANDSHAKE:
> +		break;
> +	case TLS_RECORD_TYPE_ALERT:
> +		ret = nvme_tcp_recv_tls_alert(queue, msg, len);
> +		return ret ? : -ENOTCONN;
> +	default:
> +		pr_err("queue %d: unhandled TLS record %u\n",
> +		       nvme_tcp_queue_id(queue), ctype);
> +		return -ENOTCONN;
> +	}
> +
> +	/* the ICResp is in a later record, drain this one first */
> +	while (!(msg->msg_flags & MSG_EOR)) {
> +		struct kvec iov = {
> +			.iov_base = buf,
> +			.iov_len = sizeof(buf),
> +		};
> +		struct msghdr drain = {};
> +
> +		ret = kernel_recvmsg(queue->sock, &drain, &iov, 1, sizeof(buf),
> +				     0);
> +		if (ret <= 0) {
> +			pr_warn("queue %d: failed to drain TLS record: %d\n",
> +				nvme_tcp_queue_id(queue), ret);
> +			return ret ? : -ECONNRESET;
> +		}
> +		msg->msg_flags = drain.msg_flags;
> +	}

[Severity: High]
Will draining a handshake record larger than sizeof(*icresp) (128 bytes) in
drivers/nvme/host/tcp.c:nvme_tcp_skip_tls_record() always fail with -EIO?

When a TLS peer (such as an OpenSSL target sending a TLS 1.3
NewSessionTicket) sends a TLS_RECORD_TYPE_HANDSHAKE record larger than 128
bytes, the initial kernel_recvmsg() in nvme_tcp_recv_icresp() reads the
first 128 bytes and returns without setting MSG_EOR in msg.msg_flags.

In nvme_tcp_skip_tls_record(), the drain msghdr is zero-initialized with
drain.msg_control set to NULL and drain.msg_controllen set to 0 before
calling kernel_recvmsg().

In the kTLS receive path:

  kernel_recvmsg()
    -> tls_sw_recvmsg()
      -> process_rx_list()
        -> tls_record_content_type()
          -> put_cmsg()

net/core/scm.c:put_cmsg() checks msg->msg_control:

	if (!msg->msg_control || msg->msg_controllen < sizeof(struct cmsghdr)) {
		msg->msg_flags |= MSG_CTRUNC;
		return 0; /* XXX: return error? check spec. */
	}

And net/tls/tls_sw.c:tls_record_content_type() rejects non-data records
when MSG_CTRUNC is set:

	if (!*control) {
		*control = tlm->control;
		if (!*control)
			return -EBADMSG;

		err = put_cmsg(msg, SOL_TLS, TLS_GET_RECORD_TYPE,
			       sizeof(*control), control);
		if (*control != TLS_RECORD_TYPE_DATA) {
			if (err || msg->msg_flags & MSG_CTRUNC)
				return -EIO;
		}
	}

Because drain does not provide a control message buffer,
tls_record_content_type() returns -EIO without consuming the rest of the
handshake record, causing nvme_tcp_skip_tls_record() and
nvme_tcp_init_connection() to fail with -EIO.

Should drain.msg_control and drain.msg_controllen be populated with a
cmsg buffer on each iteration of the drain loop?

> +
> +	return 0;
> +}
> +
> +/*
> + * Receive the ICResp. Over TLS, skip the records that a peer may send
> + * ahead of it.
> + *
> + * Returns the length received, or a negative errno.
> + */
> +static int nvme_tcp_recv_icresp(struct nvme_tcp_queue *queue,
> +				struct nvme_tcp_icresp_pdu *icresp)
> +{
> +	char cbuf[CMSG_LEN(sizeof(char))];
> +	struct msghdr msg;
> +	struct kvec iov;
> +	u8 ctype;
> +	int i, ret;
> +
> +	for (i = 0; i < NVME_TCP_ICRESP_MAX_RECORDS; i++) {
> +		memset(&msg, 0, sizeof(msg));
> +		memset(cbuf, 0, sizeof(cbuf));
> +		iov.iov_base = icresp;
> +		iov.iov_len = sizeof(*icresp);
> +		if (nvme_tcp_queue_tls(queue)) {
> +			msg.msg_control = cbuf;
> +			msg.msg_controllen = sizeof(cbuf);
> +		}
> +		msg.msg_flags = MSG_WAITALL;
> +		ret = kernel_recvmsg(queue->sock, &msg, &iov, 1,
> +				     iov.iov_len, msg.msg_flags);
> +		if (ret < 0 || !nvme_tcp_queue_tls(queue))
> +			return ret;

[Severity: Medium]
What happens in drivers/nvme/host/tcp.c:nvme_tcp_recv_icresp() if the TLS
peer closes the connection cleanly before sending the ICResp and
kernel_recvmsg() returns 0?

When kernel_recvmsg() returns 0 on EOF over a TLS-enabled queue, the check
above (ret < 0 || !nvme_tcp_queue_tls(queue)) is false, so execution
continues to tls_get_record_type() with the zeroed cbuf.

Since cmsg->cmsg_level is 0, tls_get_record_type() returns 0, and
nvme_tcp_skip_tls_record() hits the default case:

	default:
		pr_err("queue %d: unhandled TLS record %u\n",
		       nvme_tcp_queue_id(queue), ctype);
		return -ENOTCONN;

This logs a spurious "unhandled TLS record 0" error and returns -ENOTCONN,
bypassing the EOF handling in nvme_tcp_init_connection():

	ret = nvme_tcp_recv_icresp(queue, icresp);
	if (ret >= 0 && ret < sizeof(*icresp))
		ret = -ECONNRESET;

Should the early return condition in nvme_tcp_recv_icresp() check ret <= 0
instead of ret < 0?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261026-nvme-tcp-ctrlrec-fixes-v1-0-ebf6ff7eff34@gmail.com?part=2

Re: [PATCH 2/2] nvme-tcp: skip TLS control records while waiting for the ICResp

[Hannes Reinecke]

On 10/26/26 7:09 PM, Martin Belanger via B4 Relay wrote:
> From: Martin Belanger <martin.belanger@dell.com>
> 
> A peer may send a NewSessionTicket as soon as the handshake completes,
> ahead of the ICResp. nvme_tcp_init_connection() fails the connection on
> any record that is not application data, so such a target cannot be
> reached at all. An OpenSSL-based target does this by default.
> 
> Discard such a record and read again, up to a fixed number of records.
> Report an alert, and fail the connection for any other record type.
> 
> The record type is now checked before the length. A control record
> shorter than the ICResp was reported as -ECONNRESET, which hid the real
> cause.
> 
> Fixes: 2837966ab2a8 ("nvme-tcp: control message handling for recvmsg()")
> Assisted-by: LLM
> Signed-off-by: Martin Belanger <martin.belanger@dell.com>
> ---
>   drivers/nvme/host/tcp.c | 157 +++++++++++++++++++++++++++++++++++++++++-------
>   1 file changed, 135 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
> index a03476f0f69d..2d361a9e8710 100644
> --- a/drivers/nvme/host/tcp.c
> +++ b/drivers/nvme/host/tcp.c
> @@ -55,6 +55,12 @@ MODULE_PARM_DESC(tls_handshake_timeout,
>   		 "nvme TLS handshake timeout in seconds (default 10)");
>   #endif
>   
> +/* read a TLS control record in chunks of this size */
> +#define NVME_TCP_TLS_REC_CHUNK 256
> +
> +/* TLS records read while waiting for the ICResp, the ICResp included */
> +#define NVME_TCP_ICRESP_MAX_RECORDS 8
> +
>   static int nvme_tcp_cpu_queues[NR_CPUS];
>   static DEFINE_SPINLOCK(nvme_tcp_cpu_queues_lock);
>   
> @@ -1548,12 +1554,138 @@ static void nvme_tcp_free_queue(struct nvme_ctrl *nctrl, int qid)
>   	mutex_destroy(&queue->pf_cache_lock);
>   }
>   
> +static int nvme_tcp_recv_tls_alert(struct nvme_tcp_queue *queue,
> +				   struct msghdr *msg, int len)
> +{
> +	struct sock *sk = queue->sock->sk;
> +	u8 level, description;
> +
> +	if (len < 2) {
> +		dev_err(queue->ctrl->ctrl.device,
> +			"queue %d: truncated TLS alert\n",
> +			nvme_tcp_queue_id(queue));
> +		return -ENOTCONN;
> +	}
> +
> +	/* tls_alert_recv() reads through the iterator */
> +	iov_iter_revert(&msg->msg_iter, len);
> +	tls_alert_recv(sk, msg, &level, &description);
> +
> +	if (level == TLS_ALERT_LEVEL_FATAL) {
> +		dev_err(queue->ctrl->ctrl.device,
> +			"queue %d: fatal TLS alert %u\n",
> +			nvme_tcp_queue_id(queue), description);
> +		return -ENOTCONN;
> +	}
> +
> +	/* a peer shutting down normally is not worth a warning */
> +	if (description == TLS_ALERT_DESC_CLOSE_NOTIFY)
> +		dev_dbg(queue->ctrl->ctrl.device,
> +			"queue %d: TLS close_notify\n",
> +			nvme_tcp_queue_id(queue));
> +	else
> +		dev_warn(queue->ctrl->ctrl.device,
> +			 "queue %d: TLS alert %u\n",
> +			 nvme_tcp_queue_id(queue), description);
> +
> +	return 0;
> +}
> +
> +/*
> + * Discard a TLS record that arrived in place of the ICResp. A peer may send
> + * a NewSessionTicket as soon as the handshake completes.
> + *
> + * Returns 0 when the record was discarded and the ICResp can be read, or a
> + * negative errno if the connection cannot continue.
> + */
> +static int nvme_tcp_skip_tls_record(struct nvme_tcp_queue *queue,
> +				    struct msghdr *msg, u8 ctype, int len)
> +{
> +	u8 buf[NVME_TCP_TLS_REC_CHUNK];
> +	int ret;
> +
> +	switch (ctype) {
> +	case TLS_RECORD_TYPE_HANDSHAKE:
> +		break;
> +	case TLS_RECORD_TYPE_ALERT:
> +		ret = nvme_tcp_recv_tls_alert(queue, msg, len);
> +		return ret ? : -ENOTCONN;
> +	default:
> +		pr_err("queue %d: unhandled TLS record %u\n",
> +		       nvme_tcp_queue_id(queue), ctype);
> +		return -ENOTCONN;
> +	}
> +
> +	/* the ICResp is in a later record, drain this one first */
> +	while (!(msg->msg_flags & MSG_EOR)) {
> +		struct kvec iov = {
> +			.iov_base = buf,
> +			.iov_len = sizeof(buf),
> +		};
> +		struct msghdr drain = {};
> +
> +		ret = kernel_recvmsg(queue->sock, &drain, &iov, 1, sizeof(buf),
> +				     0);
> +		if (ret <= 0) {
> +			pr_warn("queue %d: failed to drain TLS record: %d\n",
> +				nvme_tcp_queue_id(queue), ret);
> +			return ret ? : -ECONNRESET;
> +		}
> +		msg->msg_flags = drain.msg_flags;
> +	}
> +
> +	return 0;
> +}
> +
> +/*
> + * Receive the ICResp. Over TLS, skip the records that a peer may send
> + * ahead of it.
> + *
> + * Returns the length received, or a negative errno.
> + */
> +static int nvme_tcp_recv_icresp(struct nvme_tcp_queue *queue,
> +				struct nvme_tcp_icresp_pdu *icresp)
> +{
> +	char cbuf[CMSG_LEN(sizeof(char))];
> +	struct msghdr msg;
> +	struct kvec iov;
> +	u8 ctype;
> +	int i, ret;
> +
> +	for (i = 0; i < NVME_TCP_ICRESP_MAX_RECORDS; i++) {
> +		memset(&msg, 0, sizeof(msg));
> +		memset(cbuf, 0, sizeof(cbuf));
> +		iov.iov_base = icresp;
> +		iov.iov_len = sizeof(*icresp);
> +		if (nvme_tcp_queue_tls(queue)) {
> +			msg.msg_control = cbuf;
> +			msg.msg_controllen = sizeof(cbuf);
> +		}
> +		msg.msg_flags = MSG_WAITALL;
> +		ret = kernel_recvmsg(queue->sock, &msg, &iov, 1,
> +				     iov.iov_len, msg.msg_flags);
> +		if (ret < 0 || !nvme_tcp_queue_tls(queue))
> +			return ret;
> +
> +		ctype = tls_get_record_type(queue->sock->sk,
> +					    (struct cmsghdr *)cbuf);
> +		if (ctype == TLS_RECORD_TYPE_DATA)
> +			return ret;
> +
> +		ret = nvme_tcp_skip_tls_record(queue, &msg, ctype, ret);
> +		if (ret < 0)
> +			return ret;
> +	}
> +
> +	pr_err("queue %d: too many TLS records before the ICResp\n",
> +	       nvme_tcp_queue_id(queue));
> +	return -EPROTO;
> +}
> +
>   static int nvme_tcp_init_connection(struct nvme_tcp_queue *queue)
>   {
>   	struct nvme_tcp_icreq_pdu *icreq;
>   	struct nvme_tcp_icresp_pdu *icresp;
> -	char cbuf[CMSG_LEN(sizeof(char))] = {};
> -	u8 ctype;
>   	struct msghdr msg = {};
>   	struct kvec iov;
>   	bool ctrl_hdgst, ctrl_ddgst;
> @@ -1591,16 +1723,7 @@ static int nvme_tcp_init_connection(struct nvme_tcp_queue *queue)
>   		goto free_icresp;
>   	}
>   
> -	memset(&msg, 0, sizeof(msg));
> -	iov.iov_base = icresp;
> -	iov.iov_len = sizeof(*icresp);
> -	if (nvme_tcp_queue_tls(queue)) {
> -		msg.msg_control = cbuf;
> -		msg.msg_controllen = sizeof(cbuf);
> -	}
> -	msg.msg_flags = MSG_WAITALL;
> -	ret = kernel_recvmsg(queue->sock, &msg, &iov, 1,
> -			iov.iov_len, msg.msg_flags);
> +	ret = nvme_tcp_recv_icresp(queue, icresp);
>   	if (ret >= 0 && ret < sizeof(*icresp))
>   		ret = -ECONNRESET;
>   	if (ret < 0) {
> @@ -1608,16 +1731,6 @@ static int nvme_tcp_init_connection(struct nvme_tcp_queue *queue)
>   			nvme_tcp_queue_id(queue), ret);
>   		goto free_icresp;
>   	}
> -	ret = -ENOTCONN;
> -	if (nvme_tcp_queue_tls(queue)) {
> -		ctype = tls_get_record_type(queue->sock->sk,
> -					    (struct cmsghdr *)cbuf);
> -		if (ctype != TLS_RECORD_TYPE_DATA) {
> -			pr_err("queue %d: unhandled TLS record %d\n",
> -			       nvme_tcp_queue_id(queue), ctype);
> -			goto free_icresp;
> -		}
> -	}
>   	ret = -EINVAL;
>   	if (icresp->hdr.type != nvme_tcp_icresp) {
>   		pr_err("queue %d: bad type returned %d\n",
> 

Thanks for doing this. Turns out that the 'new' TLS RFC 9846 actually 
requires us to drop NewSessionTicket message as we don't do session
resumption.
Note to self: we could check to (ab-)use the NewSessionTicket mechanism
for I/O queues when creating the connection; might be interesting.
Anyway.

Reviewed-by: Hannes Reinecke <hare@kernel.org>

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

[PATCH 0/2] nvme,tls: minimal handling for TLS records

[Hannes Reinecke]

Hi all,

TLS 1.3 can send additional TLS records while the connection is established,
and typically one would evaluate these records with passing in a control message
buffer for recvmsg(). But nvme-tcp is using the read_sock() interface which
does not allow for handling of control messages.
So as the lazy way out I opted for resetting the queue on any non-data TLS records
as this would be the default action anyway for most non-data TLS records.
But turns out that this does not work as planned, as the TLS records are evaluated
(and errors generated) before ->read_sock() is called, so nvme-tcp would receive
the error but not start error recovery.

This patchset is the minimal fix to handle it, start error recovery when a non-data
TLS record is received and also return a distinct error code from tls to indicate
the situation.

The 'real' fix will of course involve actually reading the control message and take
action based on the type, but that is a rather involved operation which will be addressed
later. So for now this simple fix should be sufficient.

Martin Belanger (2):
  nvme-tcp: start error recovery when read_sock fails
  tls: return a distinct error for control records from read_sock

 drivers/nvme/host/tcp.c | 18 +++++++++++++++++-
 net/tls/tls_sw.c        |  4 ++--
 2 files changed, 19 insertions(+), 3 deletions(-)

-- 
2.51.0

[PATCH 1/2] nvme-tcp: start error recovery when read_sock fails

[Hannes Reinecke]

From: Martin Belanger <martin.belanger@dell.com>

nvme_tcp_recv_skb() disables the queue and starts error recovery on any
errors. But if the transport encounters an error before ->read_sock()
is called (and hence before nvme_tcp_recv_skb() is called) no further
action is taken. This causes nvme_tcp_io_work() to stall until a command
times out.

This patch checks for any error returned from ->read_sock(), and starts
error recovery when the queue is still enabled (ie if nvme_tcp_recv_skb()
has not detected the error).

Signed-off-by: Martin Belanger <martin.belanger@dell.com>
Signed-off-by: Hannes Reinecke <hare@kernel.org>
---
 drivers/nvme/host/tcp.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
index 7eec983c0ead..2f1ae3968678 100644
--- a/drivers/nvme/host/tcp.c
+++ b/drivers/nvme/host/tcp.c
@@ -1422,7 +1422,23 @@ static int nvme_tcp_try_recv(struct nvme_tcp_queue *queue)
 	queue->nr_cqe = 0;
 	consumed = sock->ops->read_sock(sk, &rd_desc, nvme_tcp_recv_skb);
 	release_sock(sk);
-	return consumed == -EAGAIN ? 0 : consumed;
+	if (consumed == -EAGAIN)
+		return 0;
+
+	/*
+	 * read_sock() might encounter an error before calling
+	 * nvme_tcp_recv_skb(), so we need to check if we need
+	 * to start error recovery here.
+	 */
+	if (unlikely(consumed < 0 && queue->rd_enabled)) {
+		dev_err(queue->ctrl->ctrl.device,
+			"queue %d: receive failed: %d\n",
+			nvme_tcp_queue_id(queue), consumed);
+		queue->rd_enabled = false;
+		nvme_tcp_error_recovery(&queue->ctrl->ctrl);
+	}
+
+	return consumed;
 }
 
 static void nvme_tcp_io_work(struct work_struct *w)
-- 
2.51.0