net: tls: net: tls: fix a few random bugs

When an sk_msg scatterlist ring wraps (sg.end < sg.start), tls_push_record() chains the tail portion of the ring to the head using sg_chain(). An extra entry in the sg array is reserved for this:

struct sk_msg_sg {
[...]
/* The extra two elements:
* 1) used for chaining the front and sections when the list becomes
* partitioned (e.g. end < start). The crypto APIs require the
* chaining;
* 2) to chain tailer SG entries after the message.
*/
struct scatterlist data[MAX_MSG_FRAGS + 2];

The current code uses MAX_SKB_FRAGS + 1 as the ring size:

    sg_chain(&msg_pl->sg.data[msg_pl->sg.start],
             MAX_SKB_FRAGS - msg_pl->sg.start + 1,
             msg_pl->sg.data);

This places the chain pointer at

sg_chain(data[start], (MAX_SKB_FRAGS - msg_start + 1) .. = &data[start] + (MAX_SKB_FRAGS - msg_start + 1) - 1 = data[start + (MAX_SKB_FRAGS - start + 1) - 1] = data[MAX_SKB_FRAGS]

instead of the true last entry. This is likely due to a "race" of the commit under Fixes landing close to
commit 031097d9e079 ("bpf: sk_msg, zap ingress queue on psock down")

Convert to ARRAY_SIZE and drop the data[start] / - start (as suggested by Sabrina).

Reported-by 钱一铭 <[email protected]>

Fixes 9aaaa56845a0 ("bpf: Sockmap/tls, skmsg can have wrapped skmsg that needs extra chaining")

Signed-off-by Jakub Kicinski <[email protected]>

net/tls/tls_sw.c +3 -0

index 2608b0c01849..3bfdaf5e64f5 100644
@@ -789,21 +789,33 @@ static int tls_push_record(struct sock *sk, int flags,
 	i = msg_pl->sg.end;
 	sk_msg_iter_var_prev(i);
 
+	/* msg_pl->sg.data is a ring; data[MAX+1] is reserved for the wrap
+	 * link (frags won't use it). 'i' is now the last filled entry:
+	 *

Quoted 8 lines · Jakub Kicinski

net/tls/tls_sw.c +0 -0

index 2608b0c01849..3bfdaf5e64f5 100644
@@ -789,21 +789,33 @@ static int tls_push_record(struct sock *sk, int flags,
 	i = msg_pl->sg.end;
 	sk_msg_iter_var_prev(i);
 

net/tls/tls_sw.c static int tls_push_record(struct sock *sk, int flags, · line 804

Quoted 13 lines

net/tls/tls_sw.c +13 -0

+	/* msg_pl->sg.data is a ring; data[MAX+1] is reserved for the wrap
+	 * link (frags won't use it). 'i' is now the last filled entry:
+	 *
+	 *         i   end              start
+	 *         v    v                 v            [ rsv ]
+	 *  [ d ][ d ][   ][   ]...[   ][ d ][ d ][ d ][chain]
+	 *    ^   END                                     v
+	 *     `-----------------------------------------'
+	 *
+	 * Note that SGL does not allow chain-after-chain, so for TLS 1.3,
+	 * we must make sure we don't create the wrap entry and then chain
+	 * link to content_type immediately at index 0.
+	 */  

All this wrapping with extra "hidden" slots is so confusing...

Quoted 7 lines

net/tls/tls_sw.c +4 -0

+	if (i < msg_pl->sg.start)
+		sg_chain(msg_pl->sg.data, ARRAY_SIZE(msg_pl->sg.data),
+			 msg_pl->sg.data);
+
 	rec->content_type = record_type;
 	if (prot->version == TLS_1_3_VERSION) {
 		/* Add content type to end of message.  No padding added */

net/tls/tls_sw.c static int tls_push_record(struct sock *sk, int flags, · line 814

Quoted 5 lines

net/tls/tls_sw.c +1 -2

 		sg_set_buf(&rec->sg_content_type, &rec->content_type, 1);
 		sg_mark_end(&rec->sg_content_type);
-		sg_chain(msg_pl->sg.data, msg_pl->sg.end + 1,
-			 &rec->sg_content_type);
+		sg_chain(msg_pl->sg.data, i + 2, &rec->sg_content_type);  

Probably a silly question: why do we need to chain the content type? Could we just drop it directly into the right slot of msg_pl?

(I'm also a bit puzzled by the "last_filled + 2"/"end + 1", because that would leave an empty slot between the last_filled slot and the content_type? and maybe even overwrite the first actual message chunk if we had filled the message?)

Quoted 9 lines · On Tue, 12 May 2026 13:09:36 +0200 Sabrina Dubroca wrote:

Quoted 8 lines · Jakub Kicinski

net/tls/tls_sw.c +0 -0

index 2608b0c01849..3bfdaf5e64f5 100644
@@ -789,21 +789,33 @@ static int tls_push_record(struct sock *sk, int flags,
 	i = msg_pl->sg.end;
 	sk_msg_iter_var_prev(i);
 

Quoted 15 lines

net/tls/tls_sw.c static int tls_push_record(struct sock *sk, int flags, · line 804

Quoted 13 lines

net/tls/tls_sw.c +13 -0

+	/* msg_pl->sg.data is a ring; data[MAX+1] is reserved for the wrap
+	 * link (frags won't use it). 'i' is now the last filled entry:
+	 *
+	 *         i   end              start
+	 *         v    v                 v            [ rsv ]
+	 *  [ d ][ d ][   ][   ]...[   ][ d ][ d ][ d ][chain]
+	 *    ^   END                                     v
+	 *     `-----------------------------------------'
+	 *
+	 * Note that SGL does not allow chain-after-chain, so for TLS 1.3,
+	 * we must make sure we don't create the wrap entry and then chain
+	 * link to content_type immediately at index 0.
+	 */  

All this wrapping with extra "hidden" slots is so confusing...

Yes, always takes me an hour to swap the context back in. I was hoping the diagram would help.

Quoted 7 lines

Quoted 7 lines

net/tls/tls_sw.c +4 -0

+	if (i < msg_pl->sg.start)
+		sg_chain(msg_pl->sg.data, ARRAY_SIZE(msg_pl->sg.data),
+			 msg_pl->sg.data);
+
 	rec->content_type = record_type;
 	if (prot->version == TLS_1_3_VERSION) {
 		/* Add content type to end of message.  No padding added */

Quoted 8 lines

net/tls/tls_sw.c static int tls_push_record(struct sock *sk, int flags, · line 814

Quoted 5 lines

net/tls/tls_sw.c +1 -2

 		sg_set_buf(&rec->sg_content_type, &rec->content_type, 1);
 		sg_mark_end(&rec->sg_content_type);
-		sg_chain(msg_pl->sg.data, msg_pl->sg.end + 1,
-			 &rec->sg_content_type);
+		sg_chain(msg_pl->sg.data, i + 2, &rec->sg_content_type);  

Probably a silly question: why do we need to chain the content type? Could we just drop it directly into the right slot of msg_pl?

Good question, IDK either. Maybe anticipating the need for padding? Do you prefer that as the fix, or follow up in net-next is fine?

Quoted 4 lines

(I'm also a bit puzzled by the "last_filled + 2"/"end + 1", because that would leave an empty slot between the last_filled slot and the content_type? and maybe even overwrite the first actual message chunk if we had filled the message?)

Incredibly confusingly the value passed to sg_chain is the arrays size. So it's entry target + 1. IDK if that answer your question but it always confuses me. I even added it to the comment above but looks like I ended up removing it.

Quoted 5 lines

Quoted 2 lines

Probably a silly question: why do we need to chain the content type? Could we just drop it directly into the right slot of msg_pl?

Good question, IDK either. Maybe anticipating the need for padding? Do you prefer that as the fix, or follow up in net-next is fine?

I'd go with "make it a bit more straightforward" and remove the chaining. If you want to retain the hunk moving the other chaining (for wrapping) around, go ahead. Not sure it would still make sense.

bpf_exec_tx_verdict() may return having modified the record and the plaintext/encrypted sk_msg pointers. We must always reload those pointers after calling bpf_exec_tx_verdict().

On the wait_for_memory path after sk_stream_wait_memory() returns, the post-wait contains a shortcut:

	if (ctx->open_rec && msg_en->sg.size < required_size)
		goto alloc_encrypted;

which dereferences the cached msg_en, which can equally point at a freed record if the prior bpf_exec_tx_verdict() split the open rec before returning -ENOMEM. Drop the shortcut it seems to have only been an optimization to skip trivial intro of the loop.

Reported-by Sashiko <[email protected]>

Fixes 54a3ecaeeeae ("bpf: fix ktls panic with sockmap")

Signed-off-by Jakub Kicinski <[email protected]>

---
  net/tls/tls_sw.c | 4 ----
  1 file changed, 4 deletions(-)

net/tls/tls_sw.c +0 -4

index 3bfdaf5e64f5..360f71fd7884 100644
@@ -1112,7 +1112,6 @@ static int tls_sw_sendmsg_locked(struct sock *sk, struct msghdr *msg,
  		if (!sk_stream_memory_free(sk))
  			goto wait_for_sndbuf;
 
-alloc_encrypted:
  		ret = tls_alloc_encrypted_msg(sk, required_size);
  		if (ret) {
  			if (ret != -ENOSPC)

@@ -1255,9 +1254,6 @@ static int tls_sw_sendmsg_locked(struct sock *sk, struct msghdr *msg,
  				tls_trim_both_msgs(sk, orig_size);
  			goto send_end;
  		}
-
-		if (ctx->open_rec && msg_en->sg.size < required_size)
-			goto alloc_encrypted;
  	}
 
  send_end:

Quoted 2 lines

Reported-by Sashiko <[email protected]>

Fixes 54a3ecaeeeae ("bpf: fix ktls panic with sockmap")

Is the blamed commit correct? I don't think I touched the following code in this commit.

As explained in commit 54a3ecaeeeae ("bpf: fix ktls panic with sockmap") once we call BPF there's no way for us to rollback the iter and copy data, since BPF may have modified the message.
This is regardless of whether BPF set up cork or not.

Remove the attempt to roll back iter completely. This removes a UAF since BPF may have modified msg_pl and rec, so these pointers were stale.

Note that I'm entirely unsure what the expected behavior is here for BPF. Feels like this path must not be exercised by normal applications / existing deployments in the first place.

Fixes d3b18ad31f93 ("tls: add bpf support to sk_msg handling")

Signed-off-by Jakub Kicinski <[email protected]>

---
 net/tls/tls_sw.c | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

net/tls/tls_sw.c +0 -0

index 360f71fd7884..22b77840e35a 100644

Quoted 24 lines · Jakub Kicinski

As explained in commit 54a3ecaeeeae ("bpf: fix ktls panic with sockmap") once we call BPF there's no way for us to rollback the iter and copy data, since BPF may have modified the message.
This is regardless of whether BPF set up cork or not.

Remove the attempt to roll back iter completely. This removes a UAF since BPF may have modified msg_pl and rec, so these pointers were stale.

Note that I'm entirely unsure what the expected behavior is here for BPF. Feels like this path must not be exercised by normal applications / existing deployments in the first place.

Fixes d3b18ad31f93 ("tls: add bpf support to sk_msg handling")

Signed-off-by Jakub Kicinski <[email protected]>

---
 net/tls/tls_sw.c | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

net/tls/tls_sw.c +0 -0

index 360f71fd7884..22b77840e35a 100644

net/tls/tls_sw.c static int tls_sw_sendmsg_locked(struct sock *sk, struct msghdr *msg, · line 1168

Quoted 10 lines

net/tls/tls_sw.c +2 -5

@@ -1164,11 +1164,8 @@ static int tls_sw_sendmsg_locked(struct sock *sk, struct msghdr *msg,
 				else if (ret == -ENOMEM)
 					goto wait_for_memory;
 				else if (ctx->open_rec && ret == -ENOSPC) {
-					if (msg_pl->cork_bytes) {
-						ret = 0;
-						goto send_end;
-					}
-					goto rollback_iter;
+					ret = 0;
+					goto send_end;

The sashiko report here looks like a pre-existing issue that could be handled separately.

Still let me play safe and merge just the 2 first patch in the series.

Quoted 2 lines

The sashiko report here looks like a pre-existing issue that could be handled separately.

Quoted 1 line

Still let me play safe and merge just the 2 first patch in the series.

Kind of. We had agreed that #2 could be fixed in a somewhat better way.

@Jakub removal of the content_type chaining can be done in net-next as a clean up now I guess

Quoted 2 lines · On Thu, 14 May 2026 14:55:03 +0200 Sabrina Dubroca wrote:

Quoted 2 lines

The sashiko report here looks like a pre-existing issue that could be handled separately.

Or not, cause frankly TLS+sockmap feels a bit hopeless at this stage.

Quoted 7 lines

Quoted 1 line

Still let me play safe and merge just the 2 first patch in the series.

Kind of. We had agreed that #2 could be fixed in a somewhat better way.

@Jakub removal of the content_type chaining can be done in net-next as a clean up now I guess

Oh, TBH I also thought you are okay with the patch as posted. I'll follow up in net-next shortly (once Linus pulls).

Quoted 3 lines · Jakub Kicinski

On Thu, 14 May 2026 14:55:03 +0200 Sabrina Dubroca wrote:
Or not, cause frankly TLS+sockmap feels a bit hopeless at this stage.

:/
How long do you want to give Daniel/John (or others) to chime in on the cover letter? (it's been only 2 days, but it feels like much longer)

General question - do we know about any kTLS with sockmap-redirects users out there? I've been asking around at conferences for a couple years now and haven't heard about any consumers of this feature.

If there are no known users, maybe we can have kTLS+sockmap glue code behind a build-time config option and put it on a path to deprecation?

WDYT?