[PATCH v2 0/3] nvme-tcp: consume TLS records without application data on a live queue

[PATCH v2 0/3] nvme-tcp: consume TLS records without application data on a live queue

[Martin Belanger via B4 Relay]

After the TLS handshake, a peer may send records that carry no
application data: a NewSessionTicket, a KeyUpdate or an alert.
tls_sw_read_sock() cannot pass such a record to nvme_tcp_recv_skb().
It returns -EINVAL and leaves the record at the head of the receive
queue. Since commit 288cfa187208 ("nvme-tcp: start error recovery when
read_sock fails"), nvme-tcp then starts error recovery, and the
controller reconnects.

TLS 1.3 allows a server to send a NewSessionTicket at any time after
the handshake (RFC 8446, section 4.6.1). Alerts can also arrive at any
time.

Patch 1 aligns the control message buffer that receives the record
type.

Patch 2 moves the TLS record read loop into a helper. No functional
change.

Patch 3 reads the record with recvmsg() when read_sock() returns
-EINVAL, and handles it by type. Handshake messages other than
KeyUpdate are discarded and alerts are logged. A fatal alert, a
KeyUpdate or another record type fails the queue.

Patch 3 detects a KeyUpdate only when it is the first handshake
message of the record. A KeyUpdate that follows another message in the
same record is discarded with it. The next record then fails to
decrypt, and the queue fails as it does today.

All patches are tagged for stable, because 288cfa187208 is in 6.12.117,
6.18.63 and 7.3.4.

Tool use
========

I developed this series with the help of an LLM coding assistant. It
drafted code, tests and changelogs, and helped with analysis of the
kTLS and sunrpc sources. I reviewed, edited and tested all of it, and
I am responsible for the content.

Testing
=======

Host and target in one VM (virtme-ng), on 7.4-rc2. A test-only change
to net/tls makes the target send records on a live I/O queue during
I/O. Each case writes data and reads it back with O_DIRECT.

  Records sent by the target   7.4-rc2     With this series
  ---------------------------  ----------  --------------------------
  NewSessionTicket, 64 bytes   reconnect   queue stays live
  2 x 300, 2 x 1000, 6 x 64    reconnect   queue stays live
  close_notify alert           reconnect   queue stays live
  warning alert (90)           reconnect   queue stays live, logged
  fatal alert (40)             reconnect   reconnect, logged
  KeyUpdate                    reconnect   reconnect, logged

Data was correct in all cases. Also built with W=1, with and without
CONFIG_NVME_TCP_TLS.

Poll queues are not tested. Polled I/O hangs in my test VM, also on
7.4-rc2 without this series.

Signed-off-by: Martin Belanger <martin.belanger@dell.com>
---
Changes in v2:
- New patch 1: use a union with struct cmsghdr for the record type
  buffer, as sunrpc does. (Sashiko)
- Patch 3: on a poll queue, nvme_tcp_poll() and nvme_tcp_io_work()
  could both read the same record. A queue flag lets only one of them
  read it. (Sashiko)
- Patch 3: drop a log line that repeated the error of
  nvme_tcp_try_recv().
- Link to v1: https://patch.msgid.link/20261109-nvme-tcp-live-ctrlrec-v1-0-47ba61670e65@gmail.com

---
Martin Belanger (3):
      nvme-tcp: use an aligned buffer for the TLS record type
      nvme-tcp: move the TLS record read loop into a helper
      nvme-tcp: consume TLS records without application data

 drivers/nvme/host/tcp.c | 175 +++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 144 insertions(+), 31 deletions(-)
---
base-commit: 7475ea8b58616b33c04afb976cf0fbaad9dd63fb
change-id: 20261109-nvme-tcp-live-ctrlrec-4539b9390301

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

[PATCH v2 0/3] nvme-tcp: consume TLS records without application data on a live queue

[Martin Belanger]

After the TLS handshake, a peer may send records that carry no
application data: a NewSessionTicket, a KeyUpdate or an alert.
tls_sw_read_sock() cannot pass such a record to nvme_tcp_recv_skb().
It returns -EINVAL and leaves the record at the head of the receive
queue. Since commit 288cfa187208 ("nvme-tcp: start error recovery when
read_sock fails"), nvme-tcp then starts error recovery, and the
controller reconnects.

TLS 1.3 allows a server to send a NewSessionTicket at any time after
the handshake (RFC 8446, section 4.6.1). Alerts can also arrive at any
time.

Patch 1 aligns the control message buffer that receives the record
type.

Patch 2 moves the TLS record read loop into a helper. No functional
change.

Patch 3 reads the record with recvmsg() when read_sock() returns
-EINVAL, and handles it by type. Handshake messages other than
KeyUpdate are discarded and alerts are logged. A fatal alert, a
KeyUpdate or another record type fails the queue.

Patch 3 detects a KeyUpdate only when it is the first handshake
message of the record. A KeyUpdate that follows another message in the
same record is discarded with it. The next record then fails to
decrypt, and the queue fails as it does today.

All patches are tagged for stable, because 288cfa187208 is in 6.12.117,
6.18.63 and 7.3.4.

Tool use
========

I developed this series with the help of an LLM coding assistant. It
drafted code, tests and changelogs, and helped with analysis of the
kTLS and sunrpc sources. I reviewed, edited and tested all of it, and
I am responsible for the content.

Testing
=======

Host and target in one VM (virtme-ng), on 7.4-rc2. A test-only change
to net/tls makes the target send records on a live I/O queue during
I/O. Each case writes data and reads it back with O_DIRECT.

  Records sent by the target   7.4-rc2     With this series
  ---------------------------  ----------  --------------------------
  NewSessionTicket, 64 bytes   reconnect   queue stays live
  2 x 300, 2 x 1000, 6 x 64    reconnect   queue stays live
  close_notify alert           reconnect   queue stays live
  warning alert (90)           reconnect   queue stays live, logged
  fatal alert (40)             reconnect   reconnect, logged
  KeyUpdate                    reconnect   reconnect, logged

Data was correct in all cases. Also built with W=1, with and without
CONFIG_NVME_TCP_TLS.

Poll queues are not tested. Polled I/O hangs in my test VM, also on
7.4-rc2 without this series.

Signed-off-by: Martin Belanger <martin.belanger@dell.com>
---
Changes in v2:
- New patch 1: use a union with struct cmsghdr for the record type
  buffer, as sunrpc does. (Sashiko)
- Patch 3: on a poll queue, nvme_tcp_poll() and nvme_tcp_io_work()
  could both read the same record. A queue flag lets only one of them
  read it. (Sashiko)
- Patch 3: drop a log line that repeated the error of
  nvme_tcp_try_recv().
- Link to v1: https://patch.msgid.link/20261109-nvme-tcp-live-ctrlrec-v1-0-47ba61670e65@gmail.com

---
Martin Belanger (3):
      nvme-tcp: use an aligned buffer for the TLS record type
      nvme-tcp: move the TLS record read loop into a helper
      nvme-tcp: consume TLS records without application data

 drivers/nvme/host/tcp.c | 175 +++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 144 insertions(+), 31 deletions(-)
---
base-commit: 7475ea8b58616b33c04afb976cf0fbaad9dd63fb
change-id: 20261109-nvme-tcp-live-ctrlrec-4539b9390301

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

[PATCH v2 1/3] nvme-tcp: use an aligned buffer for the TLS record type

[Martin Belanger via B4 Relay]

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

The buffers that receive the TLS record type are char arrays.
put_cmsg() and tls_get_record_type() access them as struct cmsghdr,
which needs a stricter alignment than a char array has.

Use a union with struct cmsghdr, sized with CMSG_SPACE(), as
svc_tcp_recv_cmsg() and xs_tls_read_record() in sunrpc do.

Fixes: 2837966ab2a8 ("nvme-tcp: control message handling for recvmsg()")
Cc: stable@vger.kernel.org
Signed-off-by: Martin Belanger <martin.belanger@dell.com>
---
 drivers/nvme/host/tcp.c | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
index 7ecc6c007f8d..749d77b4e172 100644
--- a/drivers/nvme/host/tcp.c
+++ b/drivers/nvme/host/tcp.c
@@ -77,6 +77,12 @@ MODULE_PARM_DESC(tls_handshake_timeout,
 /* TLS records read while waiting for the ICResp, the ICResp included */
 #define NVME_TCP_ICRESP_MAX_RECORDS 8
 
+/* control message buffer for the TLS record type */
+union nvme_tcp_tls_cmsg {
+	struct cmsghdr	cmsg;
+	u8		buf[CMSG_SPACE(sizeof(u8))];
+};
+
 static int nvme_tcp_cpu_queues[NR_CPUS];
 static DEFINE_SPINLOCK(nvme_tcp_cpu_queues_lock);
 
@@ -1653,11 +1659,11 @@ static int nvme_tcp_skip_tls_record(struct nvme_tcp_queue *queue,
 			.iov_base = buf,
 			.iov_len = sizeof(buf),
 		};
-		char cbuf[CMSG_LEN(sizeof(char))] = {};
+		union nvme_tcp_tls_cmsg u = {};
 		/* kTLS fails a control record read without a cmsg buffer */
 		struct msghdr drain = {
-			.msg_control = cbuf,
-			.msg_controllen = sizeof(cbuf),
+			.msg_control = &u,
+			.msg_controllen = sizeof(u),
 		};
 
 		ret = kernel_recvmsg(queue->sock, &drain, &iov, 1, sizeof(buf),
@@ -1682,7 +1688,7 @@ static int nvme_tcp_skip_tls_record(struct nvme_tcp_queue *queue,
 static int nvme_tcp_recv_icresp(struct nvme_tcp_queue *queue,
 				struct nvme_tcp_icresp_pdu *icresp)
 {
-	char cbuf[CMSG_LEN(sizeof(char))];
+	union nvme_tcp_tls_cmsg u;
 	struct msghdr msg;
 	struct kvec iov;
 	u8 ctype;
@@ -1690,12 +1696,12 @@ static int nvme_tcp_recv_icresp(struct nvme_tcp_queue *queue,
 
 	for (i = 0; i < NVME_TCP_ICRESP_MAX_RECORDS; i++) {
 		memset(&msg, 0, sizeof(msg));
-		memset(cbuf, 0, sizeof(cbuf));
+		memset(&u, 0, sizeof(u));
 		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_control = &u;
+			msg.msg_controllen = sizeof(u);
 		}
 		msg.msg_flags = MSG_WAITALL;
 		ret = kernel_recvmsg(queue->sock, &msg, &iov, 1,
@@ -1703,8 +1709,7 @@ static int nvme_tcp_recv_icresp(struct nvme_tcp_queue *queue,
 		if (ret <= 0 || !nvme_tcp_queue_tls(queue))
 			return ret;
 
-		ctype = tls_get_record_type(queue->sock->sk,
-					    (struct cmsghdr *)cbuf);
+		ctype = tls_get_record_type(queue->sock->sk, &u.cmsg);
 		if (ctype == TLS_RECORD_TYPE_DATA)
 			return ret;
 

-- 
2.53.0

[PATCH v2 1/3] nvme-tcp: use an aligned buffer for the TLS record type

[Martin Belanger]

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

The buffers that receive the TLS record type are char arrays.
put_cmsg() and tls_get_record_type() access them as struct cmsghdr,
which needs a stricter alignment than a char array has.

Use a union with struct cmsghdr, sized with CMSG_SPACE(), as
svc_tcp_recv_cmsg() and xs_tls_read_record() in sunrpc do.

Fixes: 2837966ab2a8 ("nvme-tcp: control message handling for recvmsg()")
Cc: stable@vger.kernel.org
Signed-off-by: Martin Belanger <martin.belanger@dell.com>
---
 drivers/nvme/host/tcp.c | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
index 7ecc6c007f8d..749d77b4e172 100644
--- a/drivers/nvme/host/tcp.c
+++ b/drivers/nvme/host/tcp.c
@@ -77,6 +77,12 @@ MODULE_PARM_DESC(tls_handshake_timeout,
 /* TLS records read while waiting for the ICResp, the ICResp included */
 #define NVME_TCP_ICRESP_MAX_RECORDS 8
 
+/* control message buffer for the TLS record type */
+union nvme_tcp_tls_cmsg {
+	struct cmsghdr	cmsg;
+	u8		buf[CMSG_SPACE(sizeof(u8))];
+};
+
 static int nvme_tcp_cpu_queues[NR_CPUS];
 static DEFINE_SPINLOCK(nvme_tcp_cpu_queues_lock);
 
@@ -1653,11 +1659,11 @@ static int nvme_tcp_skip_tls_record(struct nvme_tcp_queue *queue,
 			.iov_base = buf,
 			.iov_len = sizeof(buf),
 		};
-		char cbuf[CMSG_LEN(sizeof(char))] = {};
+		union nvme_tcp_tls_cmsg u = {};
 		/* kTLS fails a control record read without a cmsg buffer */
 		struct msghdr drain = {
-			.msg_control = cbuf,
-			.msg_controllen = sizeof(cbuf),
+			.msg_control = &u,
+			.msg_controllen = sizeof(u),
 		};
 
 		ret = kernel_recvmsg(queue->sock, &drain, &iov, 1, sizeof(buf),
@@ -1682,7 +1688,7 @@ static int nvme_tcp_skip_tls_record(struct nvme_tcp_queue *queue,
 static int nvme_tcp_recv_icresp(struct nvme_tcp_queue *queue,
 				struct nvme_tcp_icresp_pdu *icresp)
 {
-	char cbuf[CMSG_LEN(sizeof(char))];
+	union nvme_tcp_tls_cmsg u;
 	struct msghdr msg;
 	struct kvec iov;
 	u8 ctype;
@@ -1690,12 +1696,12 @@ static int nvme_tcp_recv_icresp(struct nvme_tcp_queue *queue,
 
 	for (i = 0; i < NVME_TCP_ICRESP_MAX_RECORDS; i++) {
 		memset(&msg, 0, sizeof(msg));
-		memset(cbuf, 0, sizeof(cbuf));
+		memset(&u, 0, sizeof(u));
 		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_control = &u;
+			msg.msg_controllen = sizeof(u);
 		}
 		msg.msg_flags = MSG_WAITALL;
 		ret = kernel_recvmsg(queue->sock, &msg, &iov, 1,
@@ -1703,8 +1709,7 @@ static int nvme_tcp_recv_icresp(struct nvme_tcp_queue *queue,
 		if (ret <= 0 || !nvme_tcp_queue_tls(queue))
 			return ret;
 
-		ctype = tls_get_record_type(queue->sock->sk,
-					    (struct cmsghdr *)cbuf);
+		ctype = tls_get_record_type(queue->sock->sk, &u.cmsg);
 		if (ctype == TLS_RECORD_TYPE_DATA)
 			return ret;
 

-- 
2.53.0

[PATCH v2 2/3] nvme-tcp: move the TLS record read loop into a helper

[Martin Belanger via B4 Relay]

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

Move the read loop out of nvme_tcp_skip_tls_record() into a new
helper, nvme_tcp_drain_tls_record(). The next patch uses the helper
to read TLS records that arrive on a live queue.

No functional change.

Cc: stable@vger.kernel.org
Signed-off-by: Martin Belanger <martin.belanger@dell.com>
---
 drivers/nvme/host/tcp.c | 60 +++++++++++++++++++++++++++++++------------------
 1 file changed, 38 insertions(+), 22 deletions(-)

diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
index 749d77b4e172..c3bf345c4e92 100644
--- a/drivers/nvme/host/tcp.c
+++ b/drivers/nvme/host/tcp.c
@@ -1629,31 +1629,16 @@ static int nvme_tcp_recv_tls_alert(struct nvme_tcp_queue *queue,
 }
 
 /*
- * Discard a TLS record that arrived in place of the ICResp. A peer may send
- * a NewSessionTicket as soon as the handshake completes.
+ * Read the rest of a TLS record whose first part was read into @msg.
  *
- * Returns 0 when the record was discarded and the ICResp can be read, or a
- * negative errno if the connection cannot continue.
+ * Returns the bytes read, or a negative errno.
  */
-static int nvme_tcp_skip_tls_record(struct nvme_tcp_queue *queue,
-				    struct msghdr *msg, u8 ctype, int len)
+static int nvme_tcp_drain_tls_record(struct nvme_tcp_queue *queue,
+				     struct msghdr *msg, int flags)
 {
 	u8 buf[NVME_TCP_TLS_REC_CHUNK];
-	int ret;
+	int ret, drained = 0;
 
-	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,
@@ -1667,16 +1652,47 @@ static int nvme_tcp_skip_tls_record(struct nvme_tcp_queue *queue,
 		};
 
 		ret = kernel_recvmsg(queue->sock, &drain, &iov, 1, sizeof(buf),
-				     0);
+				     flags);
 		if (ret <= 0) {
 			pr_warn("queue %d: failed to drain TLS record: %d\n",
 				nvme_tcp_queue_id(queue), ret);
 			return ret ? : -ECONNRESET;
 		}
+		drained += ret;
 		msg->msg_flags = drain.msg_flags;
 	}
 
-	return 0;
+	return drained;
+}
+
+/*
+ * 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)
+{
+	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 */
+	ret = nvme_tcp_drain_tls_record(queue, msg, 0);
+
+	return ret < 0 ? ret : 0;
 }
 
 /*

-- 
2.53.0

[PATCH v2 2/3] nvme-tcp: move the TLS record read loop into a helper

[Martin Belanger]

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

Move the read loop out of nvme_tcp_skip_tls_record() into a new
helper, nvme_tcp_drain_tls_record(). The next patch uses the helper
to read TLS records that arrive on a live queue.

No functional change.

Cc: stable@vger.kernel.org
Signed-off-by: Martin Belanger <martin.belanger@dell.com>
---
 drivers/nvme/host/tcp.c | 60 +++++++++++++++++++++++++++++++------------------
 1 file changed, 38 insertions(+), 22 deletions(-)

diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
index 749d77b4e172..c3bf345c4e92 100644
--- a/drivers/nvme/host/tcp.c
+++ b/drivers/nvme/host/tcp.c
@@ -1629,31 +1629,16 @@ static int nvme_tcp_recv_tls_alert(struct nvme_tcp_queue *queue,
 }
 
 /*
- * Discard a TLS record that arrived in place of the ICResp. A peer may send
- * a NewSessionTicket as soon as the handshake completes.
+ * Read the rest of a TLS record whose first part was read into @msg.
  *
- * Returns 0 when the record was discarded and the ICResp can be read, or a
- * negative errno if the connection cannot continue.
+ * Returns the bytes read, or a negative errno.
  */
-static int nvme_tcp_skip_tls_record(struct nvme_tcp_queue *queue,
-				    struct msghdr *msg, u8 ctype, int len)
+static int nvme_tcp_drain_tls_record(struct nvme_tcp_queue *queue,
+				     struct msghdr *msg, int flags)
 {
 	u8 buf[NVME_TCP_TLS_REC_CHUNK];
-	int ret;
+	int ret, drained = 0;
 
-	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,
@@ -1667,16 +1652,47 @@ static int nvme_tcp_skip_tls_record(struct nvme_tcp_queue *queue,
 		};
 
 		ret = kernel_recvmsg(queue->sock, &drain, &iov, 1, sizeof(buf),
-				     0);
+				     flags);
 		if (ret <= 0) {
 			pr_warn("queue %d: failed to drain TLS record: %d\n",
 				nvme_tcp_queue_id(queue), ret);
 			return ret ? : -ECONNRESET;
 		}
+		drained += ret;
 		msg->msg_flags = drain.msg_flags;
 	}
 
-	return 0;
+	return drained;
+}
+
+/*
+ * 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)
+{
+	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 */
+	ret = nvme_tcp_drain_tls_record(queue, msg, 0);
+
+	return ret < 0 ? ret : 0;
 }
 
 /*

-- 
2.53.0

[PATCH v2 3/3] nvme-tcp: consume TLS records without application data

[Martin Belanger via B4 Relay]

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

A peer may send a TLS record without application data at any time
after the handshake: a NewSessionTicket, a KeyUpdate or an alert.
tls_sw_read_sock() cannot pass such a record to nvme_tcp_recv_skb().
It returns -EINVAL and leaves the record at the head of the receive
queue. nvme_tcp_try_recv() then starts error recovery, and the
controller reconnects.

TLS 1.3 allows a server to send a NewSessionTicket at any time after
the handshake (RFC 8446, section 4.6.1). Alerts can also arrive at any
time.

On -EINVAL, read the record with recvmsg(), which reports the record
type. Discard handshake messages other than KeyUpdate. Log alerts, and
fail the queue on a fatal alert. Fail the queue on a KeyUpdate, which
nvme-tcp cannot answer, and on any other record type. recvmsg() takes
the socket lock, so release it around the read.

On a poll queue, nvme_tcp_poll() and nvme_tcp_io_work() can both see
the record while the socket lock is released. A queue flag makes sure
that only one of them reads it.

svc_tcp_recvfrom() in sunrpc handles -EINVAL from read_sock() the same
way.

Fixes: be8e82caa685 ("nvme-tcp: enable TLS handshake upcall")
Cc: stable@vger.kernel.org
Signed-off-by: Martin Belanger <martin.belanger@dell.com>
---
 drivers/nvme/host/tcp.c | 96 +++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 94 insertions(+), 2 deletions(-)

diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
index c3bf345c4e92..cc6a46f118b2 100644
--- a/drivers/nvme/host/tcp.c
+++ b/drivers/nvme/host/tcp.c
@@ -123,6 +123,7 @@ enum nvme_tcp_queue_flags {
 	NVME_TCP_Q_LIVE		= 1,
 	NVME_TCP_Q_POLLING	= 2,
 	NVME_TCP_Q_IO_CPU_SET	= 3,
+	NVME_TCP_Q_TLS_RECORD	= 4,
 };
 
 #define NVME_TCP_OPT_NR_QUEUES(opts) ((opts)->nr_io_queues + \
@@ -1450,6 +1451,8 @@ static int nvme_tcp_try_send(struct nvme_tcp_queue *queue)
 	return ret;
 }
 
+static int nvme_tcp_recv_tls_record(struct nvme_tcp_queue *queue);
+
 static int nvme_tcp_try_recv(struct nvme_tcp_queue *queue)
 {
 	struct socket *sock = queue->sock;
@@ -1462,10 +1465,31 @@ static int nvme_tcp_try_recv(struct nvme_tcp_queue *queue)
 	lock_sock(sk);
 	queue->nr_cqe = 0;
 	consumed = sock->ops->read_sock(sk, &rd_desc, nvme_tcp_recv_skb);
+	/*
+	 * kTLS cannot pass a TLS record without application data to
+	 * nvme_tcp_recv_skb(). It returns -EINVAL and leaves the record at
+	 * the head of the receive queue. recvmsg() takes the socket lock,
+	 * so read the record without it.
+	 */
+	if (unlikely(consumed == -EINVAL && nvme_tcp_queue_tls(queue) &&
+		     queue->rd_enabled)) {
+		/*
+		 * On a poll queue, nvme_tcp_poll() and nvme_tcp_io_work() can
+		 * both get here for the same record. Only one of them reads it.
+		 */
+		if (test_and_set_bit(NVME_TCP_Q_TLS_RECORD, &queue->flags)) {
+			consumed = 0;
+		} else {
+			release_sock(sk);
+			consumed = nvme_tcp_recv_tls_record(queue);
+			lock_sock(sk);
+			clear_bit(NVME_TCP_Q_TLS_RECORD, &queue->flags);
+		}
+	}
 	/*
 	 * nvme_tcp_recv_skb() handles its own errors, so a queue that is
-	 * still enabled failed inside read_sock(). Handle it before
-	 * release_sock() processes the backlog.
+	 * still enabled failed inside read_sock() or while reading a TLS
+	 * record. Handle it before release_sock() processes the backlog.
 	 */
 	if (unlikely(consumed < 0 && consumed != -EAGAIN &&
 		     queue->rd_enabled)) {
@@ -1665,6 +1689,74 @@ static int nvme_tcp_drain_tls_record(struct nvme_tcp_queue *queue,
 	return drained;
 }
 
+/*
+ * Read the TLS record that read_sock() could not pass to
+ * nvme_tcp_recv_skb(). The caller must not hold the socket lock.
+ *
+ * A KeyUpdate needs new keys that nothing here can supply. Other handshake
+ * messages, such as a NewSessionTicket, are discarded.
+ *
+ * Returns the bytes consumed, or a negative errno if the connection cannot
+ * continue.
+ */
+static int nvme_tcp_recv_tls_record(struct nvme_tcp_queue *queue)
+{
+	u8 buf[NVME_TCP_TLS_REC_CHUNK];
+	union nvme_tcp_tls_cmsg u = {};
+	struct kvec iov = {
+		.iov_base = buf,
+		.iov_len = sizeof(buf),
+	};
+	struct msghdr msg = {
+		.msg_control = &u,
+		.msg_controllen = sizeof(u),
+	};
+	int len, ret;
+	u8 ctype;
+
+	len = kernel_recvmsg(queue->sock, &msg, &iov, 1, sizeof(buf),
+			     MSG_DONTWAIT);
+	if (len <= 0)
+		goto fail;
+
+	ctype = tls_get_record_type(queue->sock->sk, &u.cmsg);
+	switch (ctype) {
+	case TLS_RECORD_TYPE_HANDSHAKE:
+		if (buf[0] == TLS_HANDSHAKE_KEYUPDATE) {
+			dev_err(queue->ctrl->ctrl.device,
+				"queue %d: peer sent a TLS KeyUpdate, which is not supported\n",
+				nvme_tcp_queue_id(queue));
+			return -EKEYEXPIRED;
+		}
+		break;
+	case TLS_RECORD_TYPE_ALERT:
+		ret = nvme_tcp_recv_tls_alert(queue, &msg, len);
+		if (ret)
+			return ret;
+		break;
+	default:
+		dev_err(queue->ctrl->ctrl.device,
+			"queue %d: unexpected TLS record %u\n",
+			nvme_tcp_queue_id(queue), ctype);
+		return -EPROTO;
+	}
+
+	ret = nvme_tcp_drain_tls_record(queue, &msg, MSG_DONTWAIT);
+	if (ret < 0) {
+		len = ret;
+		goto fail;
+	}
+
+	return len + ret;
+
+fail:
+	/* read_sock() left a whole record, so none of it can be missing */
+	if (len == -EAGAIN)
+		return -EPROTO;
+
+	return len ? : -ECONNRESET;
+}
+
 /*
  * Discard a TLS record that arrived in place of the ICResp. A peer may send
  * a NewSessionTicket as soon as the handshake completes.

-- 
2.53.0

[PATCH v2 3/3] nvme-tcp: consume TLS records without application data

[Martin Belanger]

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

A peer may send a TLS record without application data at any time
after the handshake: a NewSessionTicket, a KeyUpdate or an alert.
tls_sw_read_sock() cannot pass such a record to nvme_tcp_recv_skb().
It returns -EINVAL and leaves the record at the head of the receive
queue. nvme_tcp_try_recv() then starts error recovery, and the
controller reconnects.

TLS 1.3 allows a server to send a NewSessionTicket at any time after
the handshake (RFC 8446, section 4.6.1). Alerts can also arrive at any
time.

On -EINVAL, read the record with recvmsg(), which reports the record
type. Discard handshake messages other than KeyUpdate. Log alerts, and
fail the queue on a fatal alert. Fail the queue on a KeyUpdate, which
nvme-tcp cannot answer, and on any other record type. recvmsg() takes
the socket lock, so release it around the read.

On a poll queue, nvme_tcp_poll() and nvme_tcp_io_work() can both see
the record while the socket lock is released. A queue flag makes sure
that only one of them reads it.

svc_tcp_recvfrom() in sunrpc handles -EINVAL from read_sock() the same
way.

Fixes: be8e82caa685 ("nvme-tcp: enable TLS handshake upcall")
Cc: stable@vger.kernel.org
Signed-off-by: Martin Belanger <martin.belanger@dell.com>
---
 drivers/nvme/host/tcp.c | 96 +++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 94 insertions(+), 2 deletions(-)

diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
index c3bf345c4e92..cc6a46f118b2 100644
--- a/drivers/nvme/host/tcp.c
+++ b/drivers/nvme/host/tcp.c
@@ -123,6 +123,7 @@ enum nvme_tcp_queue_flags {
 	NVME_TCP_Q_LIVE		= 1,
 	NVME_TCP_Q_POLLING	= 2,
 	NVME_TCP_Q_IO_CPU_SET	= 3,
+	NVME_TCP_Q_TLS_RECORD	= 4,
 };
 
 #define NVME_TCP_OPT_NR_QUEUES(opts) ((opts)->nr_io_queues + \
@@ -1450,6 +1451,8 @@ static int nvme_tcp_try_send(struct nvme_tcp_queue *queue)
 	return ret;
 }
 
+static int nvme_tcp_recv_tls_record(struct nvme_tcp_queue *queue);
+
 static int nvme_tcp_try_recv(struct nvme_tcp_queue *queue)
 {
 	struct socket *sock = queue->sock;
@@ -1462,10 +1465,31 @@ static int nvme_tcp_try_recv(struct nvme_tcp_queue *queue)
 	lock_sock(sk);
 	queue->nr_cqe = 0;
 	consumed = sock->ops->read_sock(sk, &rd_desc, nvme_tcp_recv_skb);
+	/*
+	 * kTLS cannot pass a TLS record without application data to
+	 * nvme_tcp_recv_skb(). It returns -EINVAL and leaves the record at
+	 * the head of the receive queue. recvmsg() takes the socket lock,
+	 * so read the record without it.
+	 */
+	if (unlikely(consumed == -EINVAL && nvme_tcp_queue_tls(queue) &&
+		     queue->rd_enabled)) {
+		/*
+		 * On a poll queue, nvme_tcp_poll() and nvme_tcp_io_work() can
+		 * both get here for the same record. Only one of them reads it.
+		 */
+		if (test_and_set_bit(NVME_TCP_Q_TLS_RECORD, &queue->flags)) {
+			consumed = 0;
+		} else {
+			release_sock(sk);
+			consumed = nvme_tcp_recv_tls_record(queue);
+			lock_sock(sk);
+			clear_bit(NVME_TCP_Q_TLS_RECORD, &queue->flags);
+		}
+	}
 	/*
 	 * nvme_tcp_recv_skb() handles its own errors, so a queue that is
-	 * still enabled failed inside read_sock(). Handle it before
-	 * release_sock() processes the backlog.
+	 * still enabled failed inside read_sock() or while reading a TLS
+	 * record. Handle it before release_sock() processes the backlog.
 	 */
 	if (unlikely(consumed < 0 && consumed != -EAGAIN &&
 		     queue->rd_enabled)) {
@@ -1665,6 +1689,74 @@ static int nvme_tcp_drain_tls_record(struct nvme_tcp_queue *queue,
 	return drained;
 }
 
+/*
+ * Read the TLS record that read_sock() could not pass to
+ * nvme_tcp_recv_skb(). The caller must not hold the socket lock.
+ *
+ * A KeyUpdate needs new keys that nothing here can supply. Other handshake
+ * messages, such as a NewSessionTicket, are discarded.
+ *
+ * Returns the bytes consumed, or a negative errno if the connection cannot
+ * continue.
+ */
+static int nvme_tcp_recv_tls_record(struct nvme_tcp_queue *queue)
+{
+	u8 buf[NVME_TCP_TLS_REC_CHUNK];
+	union nvme_tcp_tls_cmsg u = {};
+	struct kvec iov = {
+		.iov_base = buf,
+		.iov_len = sizeof(buf),
+	};
+	struct msghdr msg = {
+		.msg_control = &u,
+		.msg_controllen = sizeof(u),
+	};
+	int len, ret;
+	u8 ctype;
+
+	len = kernel_recvmsg(queue->sock, &msg, &iov, 1, sizeof(buf),
+			     MSG_DONTWAIT);
+	if (len <= 0)
+		goto fail;
+
+	ctype = tls_get_record_type(queue->sock->sk, &u.cmsg);
+	switch (ctype) {
+	case TLS_RECORD_TYPE_HANDSHAKE:
+		if (buf[0] == TLS_HANDSHAKE_KEYUPDATE) {
+			dev_err(queue->ctrl->ctrl.device,
+				"queue %d: peer sent a TLS KeyUpdate, which is not supported\n",
+				nvme_tcp_queue_id(queue));
+			return -EKEYEXPIRED;
+		}
+		break;
+	case TLS_RECORD_TYPE_ALERT:
+		ret = nvme_tcp_recv_tls_alert(queue, &msg, len);
+		if (ret)
+			return ret;
+		break;
+	default:
+		dev_err(queue->ctrl->ctrl.device,
+			"queue %d: unexpected TLS record %u\n",
+			nvme_tcp_queue_id(queue), ctype);
+		return -EPROTO;
+	}
+
+	ret = nvme_tcp_drain_tls_record(queue, &msg, MSG_DONTWAIT);
+	if (ret < 0) {
+		len = ret;
+		goto fail;
+	}
+
+	return len + ret;
+
+fail:
+	/* read_sock() left a whole record, so none of it can be missing */
+	if (len == -EAGAIN)
+		return -EPROTO;
+
+	return len ? : -ECONNRESET;
+}
+
 /*
  * Discard a TLS record that arrived in place of the ICResp. A peer may send
  * a NewSessionTicket as soon as the handshake completes.

-- 
2.53.0

Re: [PATCH v2 3/3] nvme-tcp: consume TLS records without application data

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] nvme-tcp: incomplete receive drain and NVME_TCP_Q_TLS_RECORD
  race in nvme_tcp_try_recv()
--

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

nvme-tcp: consume TLS records without application data

This commit handles -EINVAL returns from sock->ops->read_sock() on TLS
queues by reading non-application-data TLS records via kernel_recvmsg(),
discarding post-handshake records such as NewSessionTicket while failing
the queue on KeyUpdate or fatal alerts.

> diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
> index c3bf345c4e921..cc6a46f118b25 100644
> --- a/drivers/nvme/host/tcp.c
> +++ b/drivers/nvme/host/tcp.c
[ ... ]
> @@ -1462,10 +1465,31 @@ static int nvme_tcp_try_recv(struct nvme_tcp_queue *queue)
>  	lock_sock(sk);
>  	queue->nr_cqe = 0;
>  	consumed = sock->ops->read_sock(sk, &rd_desc, nvme_tcp_recv_skb);
> +	/*
> +	 * kTLS cannot pass a TLS record without application data to
> +	 * nvme_tcp_recv_skb(). It returns -EINVAL and leaves the record at
> +	 * the head of the receive queue. recvmsg() takes the socket lock,
> +	 * so read the record without it.
> +	 */
> +	if (unlikely(consumed == -EINVAL && nvme_tcp_queue_tls(queue) &&
> +		     queue->rd_enabled)) {
> +		/*
> +		 * On a poll queue, nvme_tcp_poll() and nvme_tcp_io_work() can
> +		 * both get here for the same record. Only one of them reads it.
> +		 */
> +		if (test_and_set_bit(NVME_TCP_Q_TLS_RECORD, &queue->flags)) {
> +			consumed = 0;
> +		} else {
> +			release_sock(sk);
> +			consumed = nvme_tcp_recv_tls_record(queue);
> +			lock_sock(sk);
> +			clear_bit(NVME_TCP_Q_TLS_RECORD, &queue->flags);
> +		}
> +	}

[Severity: Medium]
Can nvme_tcp_try_recv() leave subsequent TLS records or NVMe PDUs stranded
on a poll queue after consuming a control record?

When tls_sw_read_sock() hits a non-data record (record 1),
tls_rx_rec_done() calls tls_strp_check_rcv(), which parses the next queued
record (record 2, such as an R2T PDU, completion, or a second
NewSessionTicket) and sets strp->msg_ready.  Before returning -EINVAL,
tls_rx_reader_release() sets strp->msg_announced and calls
nvme_tcp_data_ready(sk).

If nvme_tcp_poll() and nvme_tcp_io_work() race on a TLS poll queue:

CPU0 (nvme_tcp_poll)              CPU1 (nvme_tcp_io_work)
set_bit(NVME_TCP_Q_POLLING, ...)
nvme_tcp_try_recv()
  lock_sock(sk)
  read_sock() -> -EINVAL
    /* announces record 2;
     * NVME_TCP_Q_POLLING skips
     * queueing io_work */
  test_and_set_bit(TLS_RECORD)
  release_sock(sk)
                                  nvme_tcp_try_recv()
                                    lock_sock(sk)
                                    read_sock() -> -EINVAL
                                    test_and_set_bit(TLS_RECORD)
                                    consumed = 0
                                    release_sock(sk)
                                  /* result == 0 -> pending == false,
                                   * io_work exits */
  nvme_tcp_recv_tls_record()
    /* consumes only record 1 */
  lock_sock(sk)
  clear_bit(TLS_RECORD)
  release_sock(sk)
clear_bit(NVME_TCP_Q_POLLING, ...)
/* returns nr_cqe == 0 without
 * retrying read_sock() */

Even without the race, nvme_tcp_poll() calls nvme_tcp_try_recv() once with
NVME_TCP_Q_POLLING set, so consuming record 1 returns nr_cqe == 0 without
re-running read_sock() for record 2.  Likewise, kernel_recvmsg() inside
nvme_tcp_recv_tls_record() drops the socket lock after consuming record 1
while NVME_TCP_Q_TLS_RECORD is still set; if record 2 is a second
NewSessionTicket, CPU1 can enter read_sock() in that window, decrypt
record 2 onto rx_list, see NVME_TCP_Q_TLS_RECORD still set, and return 0.

Because strp->msg_ready and strp->msg_announced are already set, later TCP
data will not trigger nvme_tcp_data_ready(), stalling the queue until I/O
times out.  Looping back to read_sock() after nvme_tcp_recv_tls_record()
succeeds would drain any remaining records under the same receive call.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261111-nvme-tcp-live-ctrlrec-v2-0-6295202b84de@gmail.com?part=3

Re: [PATCH v2 3/3] nvme-tcp: consume TLS records without application data

[Martin Belanger]

On 11/11/26 17:54, sashiko-bot@kernel.org wrote:
> [Severity: Medium]
> Can nvme_tcp_try_recv() leave subsequent TLS records or NVMe PDUs stranded
> on a poll queue after consuming a control record?
[...]
> Because strp->msg_ready and strp->msg_announced are already set, later TCP
> data will not trigger nvme_tcp_data_ready(), stalling the queue until I/O
> times out.  Looping back to read_sock() after nvme_tcp_recv_tls_record()
> succeeds would drain any remaining records under the same receive call.

On a queue without polling, only io_work reads. After the control
record, nvme_tcp_try_recv() returns a positive value, so
nvme_tcp_io_work() sets pending and calls read_sock() again, which
reads record 2. The announcement of record 2 also queues io_work,
because NVME_TCP_Q_POLLING is not set.

On a poll queue, the announcement is suppressed and nvme_tcp_poll()
returns 0, as you describe. But a poll queue only carries polled
requests, and their submitter (io_uring IOPOLL, bio_poll()) keeps
calling nvme_tcp_poll() until the request completes. The next call
reads record 2. The same applies when io_work sees
NVME_TCP_Q_TLS_RECORD set and returns 0. A record that is not yet
complete (tls_rx_rec_wait() returns 0) is handled the same way
today.

Calling read_sock() again in nvme_tcp_try_recv() would save one poll
call. It would also need a limit for a peer that keeps sending
tickets. I do not plan to change this.

Martin

Re: [PATCH v2 1/3] nvme-tcp: use an aligned buffer for the TLS record type

[Hannes Reinecke]

On 11/11/26 9:42 PM, Martin Belanger via B4 Relay wrote:
> From: Martin Belanger <martin.belanger@dell.com>
> 
> The buffers that receive the TLS record type are char arrays.
> put_cmsg() and tls_get_record_type() access them as struct cmsghdr,
> which needs a stricter alignment than a char array has.
> 
> Use a union with struct cmsghdr, sized with CMSG_SPACE(), as
> svc_tcp_recv_cmsg() and xs_tls_read_record() in sunrpc do.
> 
> Fixes: 2837966ab2a8 ("nvme-tcp: control message handling for recvmsg()")
> Cc: stable@vger.kernel.org
> Signed-off-by: Martin Belanger <martin.belanger@dell.com>
> ---
>   drivers/nvme/host/tcp.c | 23 ++++++++++++++---------
>   1 file changed, 14 insertions(+), 9 deletions(-)
> 
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

Re: [PATCH v2 2/3] nvme-tcp: move the TLS record read loop into a helper

[Hannes Reinecke]

On 11/11/26 9:42 PM, Martin Belanger via B4 Relay wrote:
> From: Martin Belanger <martin.belanger@dell.com>
> 
> Move the read loop out of nvme_tcp_skip_tls_record() into a new
> helper, nvme_tcp_drain_tls_record(). The next patch uses the helper
> to read TLS records that arrive on a live queue.
> 
> No functional change.
> 
> Cc: stable@vger.kernel.org
> Signed-off-by: Martin Belanger <martin.belanger@dell.com>
> ---
>   drivers/nvme/host/tcp.c | 60 +++++++++++++++++++++++++++++++------------------
>   1 file changed, 38 insertions(+), 22 deletions(-)
> 
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

Re: [PATCH v2 3/3] nvme-tcp: consume TLS records without application data

[Hannes Reinecke]

On 11/11/26 9:42 PM, Martin Belanger via B4 Relay wrote:
> From: Martin Belanger <martin.belanger@dell.com>
> 
> A peer may send a TLS record without application data at any time
> after the handshake: a NewSessionTicket, a KeyUpdate or an alert.
> tls_sw_read_sock() cannot pass such a record to nvme_tcp_recv_skb().
> It returns -EINVAL and leaves the record at the head of the receive
> queue. nvme_tcp_try_recv() then starts error recovery, and the
> controller reconnects.
> 
> TLS 1.3 allows a server to send a NewSessionTicket at any time after
> the handshake (RFC 8446, section 4.6.1). Alerts can also arrive at any
> time.
> 
> On -EINVAL, read the record with recvmsg(), which reports the record
> type. Discard handshake messages other than KeyUpdate. Log alerts, and
> fail the queue on a fatal alert. Fail the queue on a KeyUpdate, which
> nvme-tcp cannot answer, and on any other record type. recvmsg() takes
> the socket lock, so release it around the read.
> 
> On a poll queue, nvme_tcp_poll() and nvme_tcp_io_work() can both see
> the record while the socket lock is released. A queue flag makes sure
> that only one of them reads it.
> 
> svc_tcp_recvfrom() in sunrpc handles -EINVAL from read_sock() the same
> way.
> 
> Fixes: be8e82caa685 ("nvme-tcp: enable TLS handshake upcall")
> Cc: stable@vger.kernel.org
> Signed-off-by: Martin Belanger <martin.belanger@dell.com>
> ---
>   drivers/nvme/host/tcp.c | 96 +++++++++++++++++++++++++++++++++++++++++++++++--
>   1 file changed, 94 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
> index c3bf345c4e92..cc6a46f118b2 100644
> --- a/drivers/nvme/host/tcp.c
> +++ b/drivers/nvme/host/tcp.c
> @@ -123,6 +123,7 @@ enum nvme_tcp_queue_flags {
>   	NVME_TCP_Q_LIVE		= 1,
>   	NVME_TCP_Q_POLLING	= 2,
>   	NVME_TCP_Q_IO_CPU_SET	= 3,
> +	NVME_TCP_Q_TLS_RECORD	= 4,
>   };
>   
>   #define NVME_TCP_OPT_NR_QUEUES(opts) ((opts)->nr_io_queues + \
> @@ -1450,6 +1451,8 @@ static int nvme_tcp_try_send(struct nvme_tcp_queue *queue)
>   	return ret;
>   }
>   
> +static int nvme_tcp_recv_tls_record(struct nvme_tcp_queue *queue);
> +
>   static int nvme_tcp_try_recv(struct nvme_tcp_queue *queue)
>   {
>   	struct socket *sock = queue->sock;
> @@ -1462,10 +1465,31 @@ static int nvme_tcp_try_recv(struct nvme_tcp_queue *queue)
>   	lock_sock(sk);
>   	queue->nr_cqe = 0;
>   	consumed = sock->ops->read_sock(sk, &rd_desc, nvme_tcp_recv_skb);
> +	/*
> +	 * kTLS cannot pass a TLS record without application data to
> +	 * nvme_tcp_recv_skb(). It returns -EINVAL and leaves the record at
> +	 * the head of the receive queue. recvmsg() takes the socket lock,
> +	 * so read the record without it.
> +	 */
> +	if (unlikely(consumed == -EINVAL && nvme_tcp_queue_tls(queue) &&
> +		     queue->rd_enabled)) {
> +		/*
> +		 * On a poll queue, nvme_tcp_poll() and nvme_tcp_io_work() can
> +		 * both get here for the same record. Only one of them reads it.
> +		 */
> +		if (test_and_set_bit(NVME_TCP_Q_TLS_RECORD, &queue->flags)) {
> +			consumed = 0;
> +		} else {
> +			release_sock(sk);
> +			consumed = nvme_tcp_recv_tls_record(queue);
> +			lock_sock(sk);
> +			clear_bit(NVME_TCP_Q_TLS_RECORD, &queue->flags);
> +		}
> +	}
>   	/*
>   	 * nvme_tcp_recv_skb() handles its own errors, so a queue that is
> -	 * still enabled failed inside read_sock(). Handle it before
> -	 * release_sock() processes the backlog.
> +	 * still enabled failed inside read_sock() or while reading a TLS
> +	 * record. Handle it before release_sock() processes the backlog.
>   	 */
>   	if (unlikely(consumed < 0 && consumed != -EAGAIN &&
>   		     queue->rd_enabled)) {
> @@ -1665,6 +1689,74 @@ static int nvme_tcp_drain_tls_record(struct nvme_tcp_queue *queue,
>   	return drained;
>   }
>   
> +/*
> + * Read the TLS record that read_sock() could not pass to
> + * nvme_tcp_recv_skb(). The caller must not hold the socket lock.
> + *
> + * A KeyUpdate needs new keys that nothing here can supply. Other handshake
> + * messages, such as a NewSessionTicket, are discarded.
> + *
> + * Returns the bytes consumed, or a negative errno if the connection cannot
> + * continue.
> + */
> +static int nvme_tcp_recv_tls_record(struct nvme_tcp_queue *queue)
> +{
> +	u8 buf[NVME_TCP_TLS_REC_CHUNK];
> +	union nvme_tcp_tls_cmsg u = {};
> +	struct kvec iov = {
> +		.iov_base = buf,
> +		.iov_len = sizeof(buf),
> +	};
> +	struct msghdr msg = {
> +		.msg_control = &u,
> +		.msg_controllen = sizeof(u),
> +	};
> +	int len, ret;
> +	u8 ctype;
> +
> +	len = kernel_recvmsg(queue->sock, &msg, &iov, 1, sizeof(buf),
> +			     MSG_DONTWAIT);
> +	if (len <= 0)
> +		goto fail;
> +
> +	ctype = tls_get_record_type(queue->sock->sk, &u.cmsg);
> +	switch (ctype) {
> +	case TLS_RECORD_TYPE_HANDSHAKE:
> +		if (buf[0] == TLS_HANDSHAKE_KEYUPDATE) {
> +			dev_err(queue->ctrl->ctrl.device,
> +				"queue %d: peer sent a TLS KeyUpdate, which is not supported\n",

A bit awkward; maybe 'TLS KeyUpdate is not supported'?

> +				nvme_tcp_queue_id(queue));
> +			return -EKEYEXPIRED;
> +		}
> +		break;
> +	case TLS_RECORD_TYPE_ALERT:
> +		ret = nvme_tcp_recv_tls_alert(queue, &msg, len);
> +		if (ret)
> +			return ret;
> +		break;
> +	default:
> +		dev_err(queue->ctrl->ctrl.device,
> +			"queue %d: unexpected TLS record %u\n",
> +			nvme_tcp_queue_id(queue), ctype);
> +		return -EPROTO;
> +	}
> +
> +	ret = nvme_tcp_drain_tls_record(queue, &msg, MSG_DONTWAIT);

Hmm. Doesn't this assume that the KeyUpdate is always the first record?
And we do have similar logic in nvme_tcp_skip_tls_record(); can't we
merge both (ie just call nvme_tcp_drain_tls_record(), and drop
this switch statement?

> +	if (ret < 0) {
> +		len = ret;
> +		goto fail;
> +	}
> +
> +	return len + ret;
> +
> +fail:
> +	/* read_sock() left a whole record, so none of it can be missing */
> +	if (len == -EAGAIN)
> +		return -EPROTO;
> +
> +	return len ? : -ECONNRESET;
> +}
> +
>   /*
>    * Discard a TLS record that arrived in place of the ICResp. A peer may send
>    * a NewSessionTicket as soon as the handshake completes.
> 

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 v2 3/3] nvme-tcp: consume TLS records without application data

[Martin Belanger]

On 11/12/26 07:32, Hannes Reinecke wrote:
> diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
[...]
> +             if (buf[0] == TLS_HANDSHAKE_KEYUPDATE) {
> +                     dev_err(queue->ctrl->ctrl.device,
> +                             "queue %d: peer sent a TLS KeyUpdate,
which is not supported\n",
>
> A bit awkward; maybe 'TLS KeyUpdate is not supported'?

Yes. v3 will use "queue %d: TLS KeyUpdate is not supported".

[...]
> +     ret = nvme_tcp_drain_tls_record(queue, &msg, MSG_DONTWAIT);
>
> Hmm. Doesn't this assume that the KeyUpdate is always the first record?

It assumes that the KeyUpdate is the first handshake message in its
record. RFC 8446 also allows a KeyUpdate after another handshake message
in the same record. kTLS has the same gap: tls_check_pending_rekey()
reads only the first byte of the record. v7 of the kTLS KeyUpdate series
will check every handshake message in the record, so kTLS will detect
this case.

nvme-tcp needs no change for it, because it cannot answer a KeyUpdate in
any case. If kTLS misses the KeyUpdate, the peer's next record uses the
new key, decryption fails, and the queue goes into error recovery. The
queue fails in both cases. Only the log message differs.

> And we do have similar logic in nvme_tcp_skip_tls_record(); can't we
> merge both (ie just call nvme_tcp_drain_tls_record(), and drop
> this switch statement?

v3 will merge them. nvme_tcp_skip_tls_record() goes away, and the ICResp
path and the live path call one helper.

I would like to keep the switch, for the log messages. Draining alone
works: kTLS sets key_update_pending when it decrypts the KeyUpdate, so
the next read_sock() returns -EKEYEXPIRED. After a fatal alert, the peer
closes the connection. But then the log only shows "receive failed". The
switch logs the KeyUpdate and the alert description. The alert is often
the only hint why a target dropped the connection.

One change for the ICResp path: a warning alert ahead of the ICResp no
longer fails the connection. If the peer closes the connection after the
alert, the next read fails.

Thanks,
Martin

Re: [PATCH v2 1/3] nvme-tcp: use an aligned buffer for the TLS record type

[David Laight]

On Wed, 11 Nov 2026 15:42:39 -0500
Martin Belanger via B4 Relay <devnull+martin.nvme.dev.gmail.com@kernel.org> wrote:

> From: Martin Belanger <martin.belanger@dell.com>
> 
> The buffers that receive the TLS record type are char arrays.
> put_cmsg() and tls_get_record_type() access them as struct cmsghdr,
> which needs a stricter alignment than a char array has.
> 
> Use a union with struct cmsghdr, sized with CMSG_SPACE(), as
> svc_tcp_recv_cmsg() and xs_tls_read_record() in sunrpc do.
> 
> Fixes: 2837966ab2a8 ("nvme-tcp: control message handling for recvmsg()")
> Cc: stable@vger.kernel.org
> Signed-off-by: Martin Belanger <martin.belanger@dell.com>
> ---
>  drivers/nvme/host/tcp.c | 23 ++++++++++++++---------
>  1 file changed, 14 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
> index 7ecc6c007f8d..749d77b4e172 100644
> --- a/drivers/nvme/host/tcp.c
> +++ b/drivers/nvme/host/tcp.c
> @@ -77,6 +77,12 @@ MODULE_PARM_DESC(tls_handshake_timeout,
>  /* TLS records read while waiting for the ICResp, the ICResp included */
>  #define NVME_TCP_ICRESP_MAX_RECORDS 8
>  
> +/* control message buffer for the TLS record type */
> +union nvme_tcp_tls_cmsg {
> +	struct cmsghdr	cmsg;
> +	u8		buf[CMSG_SPACE(sizeof(u8))];
> +};
> +
>  static int nvme_tcp_cpu_queues[NR_CPUS];
>  static DEFINE_SPINLOCK(nvme_tcp_cpu_queues_lock);
>  
> @@ -1653,11 +1659,11 @@ static int nvme_tcp_skip_tls_record(struct nvme_tcp_queue *queue,
>  			.iov_base = buf,
>  			.iov_len = sizeof(buf),
>  		};
> -		char cbuf[CMSG_LEN(sizeof(char))] = {};
> +		union nvme_tcp_tls_cmsg u = {};

Is that right?
You are changing something that was CMSG_LEN(1) to one of CMSG_SPACE(1).

I've a feeling it would be right if it was a struct, not a union.

Did you test it?
Or even compile it and compare the two object files.

David

>  		/* kTLS fails a control record read without a cmsg buffer */
>  		struct msghdr drain = {
> -			.msg_control = cbuf,
> -			.msg_controllen = sizeof(cbuf),
> +			.msg_control = &u,
> +			.msg_controllen = sizeof(u),
>  		};
>  
>  		ret = kernel_recvmsg(queue->sock, &drain, &iov, 1, sizeof(buf),
> @@ -1682,7 +1688,7 @@ static int nvme_tcp_skip_tls_record(struct nvme_tcp_queue *queue,
>  static int nvme_tcp_recv_icresp(struct nvme_tcp_queue *queue,
>  				struct nvme_tcp_icresp_pdu *icresp)
>  {
> -	char cbuf[CMSG_LEN(sizeof(char))];
> +	union nvme_tcp_tls_cmsg u;
>  	struct msghdr msg;
>  	struct kvec iov;
>  	u8 ctype;
> @@ -1690,12 +1696,12 @@ static int nvme_tcp_recv_icresp(struct nvme_tcp_queue *queue,
>  
>  	for (i = 0; i < NVME_TCP_ICRESP_MAX_RECORDS; i++) {
>  		memset(&msg, 0, sizeof(msg));
> -		memset(cbuf, 0, sizeof(cbuf));
> +		memset(&u, 0, sizeof(u));
>  		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_control = &u;
> +			msg.msg_controllen = sizeof(u);
>  		}
>  		msg.msg_flags = MSG_WAITALL;
>  		ret = kernel_recvmsg(queue->sock, &msg, &iov, 1,
> @@ -1703,8 +1709,7 @@ static int nvme_tcp_recv_icresp(struct nvme_tcp_queue *queue,
>  		if (ret <= 0 || !nvme_tcp_queue_tls(queue))
>  			return ret;
>  
> -		ctype = tls_get_record_type(queue->sock->sk,
> -					    (struct cmsghdr *)cbuf);
> +		ctype = tls_get_record_type(queue->sock->sk, &u.cmsg);
>  		if (ctype == TLS_RECORD_TYPE_DATA)
>  			return ret;
>  
>