[PATCH net] tls: check the right open record in TLS_TX_MAX_PAYLOAD_LEN

[PATCH net] tls: check the right open record in TLS_TX_MAX_PAYLOAD_LEN

[Rishikesh Jethwani]

do_tls_setsockopt_tx_payload_len() refuses to change the record size
limit while a record is open, because both TX paths size the record they
are filling against tx_max_payload_len. It looks for the open record
through tls_sw_ctx_tx(), a plain cast of priv_ctx_tx to
struct tls_sw_context_tx.

On a socket whose TX side is HW offloaded (tx_conf == TLS_HW),
priv_ctx_tx is a struct tls_offload_context_tx, so the load lands in an
unrelated field. In the tested x86_64 configuration, open_rec sits at
offset 144, which aliases sg_tx_data[2].dma_address. TLS never DMA-maps
that table, so the check reads zero even while a record is open. This
alias depends on the structure layouts: other configurations can read a
populated field instead and reject a legitimate call with EBUSY.

The missed check matters:

  setsockopt(SOL_TLS, TLS_TX, ...)          TX offloaded to the NIC
  send(fd, buf, N, MSG_MORE)                leaves an open_record of N
  setsockopt(SOL_TLS, TLS_TX_MAX_PAYLOAD_LEN, M), M < N, accepted
  send(fd, buf, len, 0)

tls_push_data() recomputes max_open_record_len from the new limit, so
max_open_record_len - record->len underflows. The next chunk is appended
to the open record regardless of the limit. The record on the wire
exceeds the record_size_limit the peer advertised, which the peer must
answer with a fatal record_overflow alert, exactly what the option exists
to prevent.

Dispatch on tx_conf and look at tls_offload_context_tx::open_record for
TLS_HW. TLS_BASE has no TX context and nothing to check.

Found during review of the TLS core.

Tested on two nodes using mlx5 TLS TX offload. Sending 256 bytes with
MSG_MORE and then lowering the limit returns EBUSY with this patch and is
silently accepted without it. The same sequence on a loopback TLS_SW
socket returns EBUSY either way. With the limit set to 512 before
sending, records carry at most 512 bytes of payload. Without this patch,
lowering the limit after a MSG_MORE send produces a record exceeding the
requested payload limit.

Fixes: 82cb5be6ad64 ("net/tls: support setting the maximum payload size")
Cc: stable@vger.kernel.org
Signed-off-by: Rishikesh Jethwani <rjethwani@purestorage.com>
---
 net/tls/tls_main.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/net/tls/tls_main.c b/net/tls/tls_main.c
index fbb274287aa5..8f11b8629492 100644
--- a/net/tls/tls_main.c
+++ b/net/tls/tls_main.c
@@ -837,12 +837,23 @@ static int do_tls_setsockopt_tx_payload_len(struct sock *sk, sockptr_t optval,
 					    unsigned int optlen)
 {
 	struct tls_context *ctx = tls_get_ctx(sk);
-	struct tls_sw_context_tx *sw_ctx = tls_sw_ctx_tx(ctx);
 	u16 value;
 	bool tls_13 = ctx->prot_info.version == TLS_1_3_VERSION;
 
-	if (sw_ctx && sw_ctx->open_rec)
-		return -EBUSY;
+	/* Both TX paths size the record they are filling against
+	 * tx_max_payload_len, so refuse to change it under an open record.
+	 * priv_ctx_tx is a different struct for each TX configuration.
+	 */
+	switch (ctx->tx_conf) {
+	case TLS_SW:
+		if (tls_sw_ctx_tx(ctx)->open_rec)
+			return -EBUSY;
+		break;
+	case TLS_HW:
+		if (tls_offload_ctx_tx(ctx)->open_record)
+			return -EBUSY;
+		break;
+	}
 
 	if (sockptr_is_null(optval) || optlen != sizeof(value))
 		return -EINVAL;
-- 
2.50.1

Re: [PATCH net] tls: check the right open record in TLS_TX_MAX_PAYLOAD_LEN

[Wilfred Mallawa]

On Mon, 2026-10-19 at 16:26 -0600, Rishikesh Jethwani wrote:
> 
>  net/tls/tls_main.c | 17 ++++++++++++++---
>  1 file changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/net/tls/tls_main.c b/net/tls/tls_main.c
> index fbb274287aa5..8f11b8629492 100644
> --- a/net/tls/tls_main.c
> +++ b/net/tls/tls_main.c
> @@ -837,12 +837,23 @@ static int
> do_tls_setsockopt_tx_payload_len(struct sock *sk, sockptr_t optval,
>  					    unsigned int optlen)
>  {
>  	struct tls_context *ctx = tls_get_ctx(sk);
> -	struct tls_sw_context_tx *sw_ctx = tls_sw_ctx_tx(ctx);
>  	u16 value;
>  	bool tls_13 = ctx->prot_info.version == TLS_1_3_VERSION;
>  
> -	if (sw_ctx && sw_ctx->open_rec)
> -		return -EBUSY;
> +	/* Both TX paths size the record they are filling against
> +	 * tx_max_payload_len, so refuse to change it under an open
> record.
> +	 * priv_ctx_tx is a different struct for each TX
> configuration.
> +	 */
> +	switch (ctx->tx_conf) {
> +	case TLS_SW:
> +		if (tls_sw_ctx_tx(ctx)->open_rec)
> +			return -EBUSY;
> +		break;
> +	case TLS_HW:
> +		if (tls_offload_ctx_tx(ctx)->open_record)
> +			return -EBUSY;
> +		break;
> +	}
>  

Ah good catch!

Reviewed-by: Wilfred Mallawa <wilfred.mallawa@wdc.com>

Re: [PATCH net] tls: check the right open record in TLS_TX_MAX_PAYLOAD_LEN

[Sabrina Dubroca]

2026-10-19, 16:26:25 -0600, Rishikesh Jethwani wrote:
> do_tls_setsockopt_tx_payload_len() refuses to change the record size
> limit while a record is open, because both TX paths size the record they
> are filling against tx_max_payload_len. It looks for the open record
> through tls_sw_ctx_tx(), a plain cast of priv_ctx_tx to
> struct tls_sw_context_tx.

There was already a patch for this
https://lore.kernel.org/all/20260901072927.128688-1-jiayuan.chen@linux.dev/
with extra locking to address sashiko comments.

Jiayuan, were you going to send a v3 to address Paolo's comment? Or
maybe resend your v2, since it got marked as "changes requested" in
patchwork?

-- 
Sabrina

Re: [PATCH net] tls: check the right open record in TLS_TX_MAX_PAYLOAD_LEN

[Jiayuan Chen]

Hi Sabrina,

Thanks for your reminder. I totally forgot that thread.

I will send a v3 and update the commit message to address Paolo's comment.



On 10/20/26 6:58 PM, Sabrina Dubroca wrote:
> 2026-10-19, 16:26:25 -0600, Rishikesh Jethwani wrote:
>> do_tls_setsockopt_tx_payload_len() refuses to change the record size
>> limit while a record is open, because both TX paths size the record they
>> are filling against tx_max_payload_len. It looks for the open record
>> through tls_sw_ctx_tx(), a plain cast of priv_ctx_tx to
>> struct tls_sw_context_tx.


I guess sashiko will complain the same issue:

https://lore.kernel.org/netdev/20260901031931.1636980-1-kuba@kernel.org/


> There was already a patch for this
> https://lore.kernel.org/all/20260901072927.128688-1-jiayuan.chen@linux.dev/
> with extra locking to address sashiko comments.
>
> Jiayuan, were you going to send a v3 to address Paolo's comment? Or
> maybe resend your v2, since it got marked as "changes requested" in
> patchwork?

Re: [PATCH net] tls: check the right open record in TLS_TX_MAX_PAYLOAD_LEN

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The TLS_HW check added to do_tls_setsockopt_tx_payload_len()
  (net/tls/tls_main.c) only rejects the change when…

--

tls: check the right open record in TLS_TX_MAX_PAYLOAD_LEN

This patch changes do_tls_setsockopt_tx_payload_len() to look up the open
record based on ctx->tx_conf. For TLS_HW it now checks
tls_offload_context_tx::open_record, instead of casting priv_ctx_tx to
struct tls_sw_context_tx. The aim is to stop HW offloaded TX from building
records larger than a limit that setsockopt has accepted.

> diff --git a/net/tls/tls_main.c b/net/tls/tls_main.c
> index fbb274287aa5..8f11b8629492 100644
> --- a/net/tls/tls_main.c
> +++ b/net/tls/tls_main.c
> @@ -837,12 +837,23 @@ static int do_tls_setsockopt_tx_payload_len(struct sock *sk, sockptr_t optval,
>  					    unsigned int optlen)
>  {
>  	struct tls_context *ctx = tls_get_ctx(sk);
> -	struct tls_sw_context_tx *sw_ctx = tls_sw_ctx_tx(ctx);
>  	u16 value;
>  	bool tls_13 = ctx->prot_info.version == TLS_1_3_VERSION;
>  
> -	if (sw_ctx && sw_ctx->open_rec)
> -		return -EBUSY;
> +	/* Both TX paths size the record they are filling against
> +	 * tx_max_payload_len, so refuse to change it under an open record.
> +	 * priv_ctx_tx is a different struct for each TX configuration.
> +	 */
> +	switch (ctx->tx_conf) {
> +	case TLS_SW:
> +		if (tls_sw_ctx_tx(ctx)->open_rec)
> +			return -EBUSY;
> +		break;
> +	case TLS_HW:
> +		if (tls_offload_ctx_tx(ctx)->open_record)
> +			return -EBUSY;
> +		break;
> +	}

[Severity: Medium]
Can a concurrent HW send still produce records larger than the new limit
after this check passes?

The setsockopt path takes only the socket lock and not ctx->tx_lock:

net/tls/tls_main.c:do_tls_setsockopt() {
    ...
	case TLS_TX_MAX_PAYLOAD_LEN:
		lock_sock(sk);
		rc = do_tls_setsockopt_tx_payload_len(sk, optval, optlen);
		release_sock(sk);
    ...
}

On the device side, tls_device_sendmsg() holds tx_lock for the whole
send. However, tls_push_data() reads the limit only once, before its loop:

net/tls/tls_device.c:tls_push_data() {
    ...
	max_open_record_len = tls_ctx->tx_max_payload_len +
			      prot->prepend_size;
	do {
		rc = tls_do_allocation(sk, ctx, pfrag, prot->prepend_size);
		if (unlikely(rc)) {
			rc = sk_stream_wait_memory(sk, &timeo);
    ...
		copy = min_t(size_t, size, max_open_record_len - record->len);
    ...
		if (done || record->len >= max_open_record_len ||
    ...
}

Inside the loop, the socket lock can be dropped while open_record is NULL.
This can happen in two places:

  tls_push_record()
    offload_ctx->open_record = NULL;
    tls_push_sg()->tcp_sendmsg_locked()->sk_stream_wait_memory()
      sk_wait_event()->release_sock()

and also when tls_do_allocation() fails with no open record and
tls_push_data() calls sk_stream_wait_memory() directly.

If a second thread calls setsockopt(SOL_TLS, TLS_TX_MAX_PAYLOAD_LEN) with a
smaller value during either window, the new TLS_HW check sees open_record
as NULL and the call returns 0. The blocked send then continues to build
records against the old, larger max_open_record_len.

The SW path looks different here. tls_sw_sendmsg_locked() re-reads the
limit after open_rec is allocated on every iteration:

	record_room = tls_ctx->tx_max_payload_len - msg_pl->sg.size;

Given that, is the new comment accurate for TLS_HW? It says "Both TX paths
size the record they are filling against tx_max_payload_len", but the
device path uses a cached copy.

Would it make sense to recompute max_open_record_len from
tls_ctx->tx_max_payload_len each time tls_push_data() starts a new record?
Another option would be to take tx_lock in the TLS_TX_MAX_PAYLOAD_LEN
setsockopt path before lock_sock().

>  
>  	if (sockptr_is_null(optval) || optlen != sizeof(value))
>  		return -EINVAL;

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261019222626.745408-1-rjethwani%40purestorage.com