[PATCH net] tls: fix RX desync on overlapping skbs

[PATCH net] tls: fix RX desync on overlapping skbs

[Maximilian Immanuel Brandtner]

On a reordering path (e.g. IPsec crypto offload + GRO) the TCP receive
queue can hold adjacent skbs whose sequence ranges overlap. The tls
fast-path reads the record header with skb_copy_bits() by byte offset,
which assumes skb ranges don't overlap.
When a record boundary lands within the last few bytes of an skb, the
5-byte header straddles into the overlapping next skb; the duplicate bytes
are read as the header, yielding a bogus length (-EMSGSIZE) or version
(-EINVAL) and aborting the connection.

tls_strp_check_queue_ok() already detects such overlaps and diverts to the
seq-addressed copy path, but it only ran after the header was parsed, and
it validated stm.offset + stm.full_len -- and full_len is still 0 for a new
record, so it could not cover the header bytes. Parameterize it by length
and also validate the header region (stm.offset + TLS_HEADER_SIZE) before
parsing.

Fixes: 84c61fe1a75b ("tls: rx: do not use the standard strparser")
Signed-off-by: Maximilian Immanuel Brandtner <maxbr@linux.ibm.com>
---
Tested on Linux kernel 7.2 rc6
---
 net/tls/tls_strp.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/net/tls/tls_strp.c b/net/tls/tls_strp.c
index 61b10c697ecc..64d5dea90540 100644
--- a/net/tls/tls_strp.c
+++ b/net/tls/tls_strp.c
@@ -430,9 +430,9 @@ static int tls_strp_read_copy(struct tls_strparser *strp, bool qshort)
 	return 0;
 }
 
-static bool tls_strp_check_queue_ok(struct tls_strparser *strp)
+static bool tls_strp_check_queue_ok(struct tls_strparser *strp,
+				    unsigned int len)
 {
-	unsigned int len = strp->stm.offset + strp->stm.full_len;
 	struct sk_buff *first, *skb;
 	u32 seq;
 
@@ -525,6 +525,12 @@ static int tls_strp_read_sock(struct tls_strparser *strp)
 
 	tls_strp_load_anchor_with_queue(strp, inq);
 	if (!strp->stm.full_len) {
+		if (inq < TLS_HEADER_SIZE)
+			return tls_strp_read_copy(strp, true);
+
+		if (!tls_strp_check_queue_ok(strp, strp->stm.offset + TLS_HEADER_SIZE))
+			return tls_strp_read_copy(strp, false);
+
 		sz = tls_rx_msg_size(strp, strp->anchor);
 		if (sz < 0)
 			return sz;
@@ -535,7 +541,7 @@ static int tls_strp_read_sock(struct tls_strparser *strp)
 			return tls_strp_read_copy(strp, true);
 	}
 
-	if (!tls_strp_check_queue_ok(strp))
+	if (!tls_strp_check_queue_ok(strp, strp->stm.offset + strp->stm.full_len))
 		return tls_strp_read_copy(strp, false);
 
 	WRITE_ONCE(strp->msg_ready, 1);
-- 
2.55.0

Re: [PATCH net] tls: fix RX desync on overlapping skbs

[Jakub Kicinski]

On Fri,  7 Aug 2026 19:31:05 +0200 Maximilian Immanuel Brandtner wrote:
> On a reordering path (e.g. IPsec crypto offload + GRO) the TCP receive

It'd be better to move this "IPsec crypto offload + GRO" to the end,
it's not super relevant to the problem. Mention that you observe this
in real life and that "IPsec crypto offload + GRO" is the likely
culprit. Mentioning the NIC may also be useful to other users.

> queue can hold adjacent skbs whose sequence ranges overlap. The tls
> fast-path reads the record header with skb_copy_bits() by byte offset,
> which assumes skb ranges don't overlap.
> When a record boundary lands within the last few bytes of an skb, the
> 5-byte header straddles into the overlapping next skb; the duplicate bytes
> are read as the header, yielding a bogus length (-EMSGSIZE) or version
> (-EINVAL) and aborting the connection.
> 
> tls_strp_check_queue_ok() already detects such overlaps and diverts to the
> seq-addressed copy path, but it only ran after the header was parsed,

The rest of this paragraph below looks like slop, don't describe what
the patch does. We can hopefully see that from the diff.

> and
> it validated stm.offset + stm.full_len -- and full_len is still 0 for a new
> record, so it could not cover the header bytes. Parameterize it by length
> and also validate the header region (stm.offset + TLS_HEADER_SIZE) before
> parsing.
> 
> Fixes: 84c61fe1a75b ("tls: rx: do not use the standard strparser")
> Signed-off-by: Maximilian Immanuel Brandtner <maxbr@linux.ibm.com>
> ---
> Tested on Linux kernel 7.2 rc6
> ---
>  net/tls/tls_strp.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/net/tls/tls_strp.c b/net/tls/tls_strp.c
> index 61b10c697ecc..64d5dea90540 100644
> --- a/net/tls/tls_strp.c
> +++ b/net/tls/tls_strp.c
> @@ -430,9 +430,9 @@ static int tls_strp_read_copy(struct tls_strparser *strp, bool qshort)
>  	return 0;
>  }
>  
> -static bool tls_strp_check_queue_ok(struct tls_strparser *strp)
> +static bool tls_strp_check_queue_ok(struct tls_strparser *strp,
> +				    unsigned int len)
>  {
> -	unsigned int len = strp->stm.offset + strp->stm.full_len;

keep the stm.offset addition here, it's doesn't belong in the caller

>  	struct sk_buff *first, *skb;
>  	u32 seq;
>  
> @@ -525,6 +525,12 @@ static int tls_strp_read_sock(struct tls_strparser *strp)
>  
>  	tls_strp_load_anchor_with_queue(strp, inq);
>  	if (!strp->stm.full_len) {
> +		if (inq < TLS_HEADER_SIZE)
> +			return tls_strp_read_copy(strp, true);
> +

no empty line between the two checks

> +		if (!tls_strp_check_queue_ok(strp, strp->stm.offset + TLS_HEADER_SIZE))
> +			return tls_strp_read_copy(strp, false);