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

Maximilian Immanuel Brandtner <[email protected]>
Newsgroups org.kernel.vger.netdev
Message-ID <[email protected]>
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 <[email protected]>
---
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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.