* [PATCH v2 1/3] nvme-tcp: use an aligned buffer for the TLS record type
2026-11-11 20:42 [PATCH v2 0/3] nvme-tcp: consume TLS records without application data on a live queue Martin Belanger via B4 Relay
@ 2026-11-11 20:42 ` Martin Belanger via B4 Relay
2026-11-11 20:42 ` [PATCH v2 2/3] nvme-tcp: move the TLS record read loop into a helper Martin Belanger via B4 Relay
2026-11-11 20:42 ` [PATCH v2 3/3] nvme-tcp: consume TLS records without application data Martin Belanger via B4 Relay
2 siblings, 0 replies; 6+ messages in thread
From: Martin Belanger via B4 Relay @ 2026-11-11 20:42 UTC (permalink / raw)
To: Keith Busch, Jens Axboe, Christoph Hellwig, Sagi Grimberg,
Hannes Reinecke
Cc: Martin Belanger, linux-nvme, linux-kernel, Martin Belanger,
stable
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
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v2 2/3] nvme-tcp: move the TLS record read loop into a helper
2026-11-11 20:42 [PATCH v2 0/3] nvme-tcp: consume TLS records without application data on a live queue Martin Belanger via B4 Relay
2026-11-11 20:42 ` [PATCH v2 1/3] nvme-tcp: use an aligned buffer for the TLS record type Martin Belanger via B4 Relay
@ 2026-11-11 20:42 ` Martin Belanger via B4 Relay
2026-11-11 20:42 ` [PATCH v2 3/3] nvme-tcp: consume TLS records without application data Martin Belanger via B4 Relay
2 siblings, 0 replies; 6+ messages in thread
From: Martin Belanger via B4 Relay @ 2026-11-11 20:42 UTC (permalink / raw)
To: Keith Busch, Jens Axboe, Christoph Hellwig, Sagi Grimberg,
Hannes Reinecke
Cc: Martin Belanger, linux-nvme, linux-kernel, Martin Belanger,
stable
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
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v2 3/3] nvme-tcp: consume TLS records without application data
2026-11-11 20:42 [PATCH v2 0/3] nvme-tcp: consume TLS records without application data on a live queue Martin Belanger via B4 Relay
2026-11-11 20:42 ` [PATCH v2 1/3] nvme-tcp: use an aligned buffer for the TLS record type Martin Belanger via B4 Relay
2026-11-11 20:42 ` [PATCH v2 2/3] nvme-tcp: move the TLS record read loop into a helper Martin Belanger via B4 Relay
@ 2026-11-11 20:42 ` Martin Belanger via B4 Relay
2026-11-11 22:54 ` sashiko-bot
2 siblings, 1 reply; 6+ messages in thread
From: Martin Belanger via B4 Relay @ 2026-11-11 20:42 UTC (permalink / raw)
To: Keith Busch, Jens Axboe, Christoph Hellwig, Sagi Grimberg,
Hannes Reinecke
Cc: Martin Belanger, linux-nvme, linux-kernel, Martin Belanger,
stable
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
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2 3/3] nvme-tcp: consume TLS records without application data
2026-11-11 20:42 ` [PATCH v2 3/3] nvme-tcp: consume TLS records without application data Martin Belanger via B4 Relay
@ 2026-11-11 22:54 ` sashiko-bot
2026-11-12 11:23 ` Martin Belanger
0 siblings, 1 reply; 6+ messages in thread
From: sashiko-bot @ 2026-11-11 22:54 UTC (permalink / raw)
To: Martin Belanger; +Cc: linux-nvme
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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2 3/3] nvme-tcp: consume TLS records without application data
2026-11-11 22:54 ` sashiko-bot
@ 2026-11-12 11:23 ` Martin Belanger
0 siblings, 0 replies; 6+ messages in thread
From: Martin Belanger @ 2026-11-12 11:23 UTC (permalink / raw)
To: sashiko-reviews; +Cc: linux-nvme
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
^ permalink raw reply [flat|nested] 6+ messages in thread