All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net] tls: check the right open record in TLS_TX_MAX_PAYLOAD_LEN
@ 2026-10-19 22:26 Rishikesh Jethwani
  2026-10-19 23:35 ` Wilfred Mallawa
  2026-10-20 10:58 ` Sabrina Dubroca
  0 siblings, 2 replies; 4+ messages in thread
From: Rishikesh Jethwani @ 2026-10-19 22:26 UTC (permalink / raw)
  To: netdev
  Cc: john.fastabend, kuba, sd, davem, edumazet, pabeni, horms,
	wilfred.mallawa, linux-kernel, stable, 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


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH net] tls: check the right open record in TLS_TX_MAX_PAYLOAD_LEN
  2026-10-19 22:26 [PATCH net] tls: check the right open record in TLS_TX_MAX_PAYLOAD_LEN Rishikesh Jethwani
@ 2026-10-19 23:35 ` Wilfred Mallawa
  2026-10-20 10:58 ` Sabrina Dubroca
  1 sibling, 0 replies; 4+ messages in thread
From: Wilfred Mallawa @ 2026-10-19 23:35 UTC (permalink / raw)
  To: rjethwani@purestorage.com, netdev@vger.kernel.org
  Cc: davem@davemloft.net, john.fastabend@gmail.com, sd@queasysnail.net,
	linux-kernel@vger.kernel.org, pabeni@redhat.com, horms@kernel.org,
	edumazet@google.com, kuba@kernel.org, stable@vger.kernel.org

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>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH net] tls: check the right open record in TLS_TX_MAX_PAYLOAD_LEN
  2026-10-19 22:26 [PATCH net] tls: check the right open record in TLS_TX_MAX_PAYLOAD_LEN Rishikesh Jethwani
  2026-10-19 23:35 ` Wilfred Mallawa
@ 2026-10-20 10:58 ` Sabrina Dubroca
  2026-10-20 11:18   ` Jiayuan Chen
  1 sibling, 1 reply; 4+ messages in thread
From: Sabrina Dubroca @ 2026-10-20 10:58 UTC (permalink / raw)
  To: Rishikesh Jethwani, Jiayuan Chen
  Cc: netdev, john.fastabend, kuba, davem, edumazet, pabeni, horms,
	wilfred.mallawa, linux-kernel, stable

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH net] tls: check the right open record in TLS_TX_MAX_PAYLOAD_LEN
  2026-10-20 10:58 ` Sabrina Dubroca
@ 2026-10-20 11:18   ` Jiayuan Chen
  0 siblings, 0 replies; 4+ messages in thread
From: Jiayuan Chen @ 2026-10-20 11:18 UTC (permalink / raw)
  To: Sabrina Dubroca, Rishikesh Jethwani
  Cc: netdev, john.fastabend, kuba, davem, edumazet, pabeni, horms,
	wilfred.mallawa, linux-kernel, stable

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?




^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-10-20 11:18 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-19 22:26 [PATCH net] tls: check the right open record in TLS_TX_MAX_PAYLOAD_LEN Rishikesh Jethwani
2026-10-19 23:35 ` Wilfred Mallawa
2026-10-20 10:58 ` Sabrina Dubroca
2026-10-20 11:18   ` Jiayuan Chen

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.