Re: [PATCH net v2 3/4] net: tls: fix use-after-free in tls_sw_sendmsg_locked after bpf verdict

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] Silent data loss, stream corruption, and race condition in the trim_sgl error path on wait_for_memory failure.
- [High] Removing the alloc_encrypted shortcut eliminates the verdict retry mechanism, causing connection hangs on ENOMEM.
--

commit 1c563620cb643329347417380b885f1f7483dc0f
Author: Jakub Kicinski <kuba@kernel.org>

net: tls: fix use-after-free in tls_sw_sendmsg_locked after bpf verdict

bpf_exec_tx_verdict() may modify the record and the plaintext/encrypted
sk_msg pointers, requiring them to be reloaded. The post-wait shortcut
could dereference a cached msg_en pointing to a freed record if the
verdict split the open rec before returning -ENOMEM. This commit drops
the shortcut to prevent a use-after-free.

> diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
> index 3bfdaf5e64f50..360f71fd78842 100644
> --- a/net/tls/tls_sw.c
> +++ b/net/tls/tls_sw.c

[ ... ]

> @@ -1255,9 +1254,6 @@ static int tls_sw_sendmsg_locked(struct sock *sk, struct msghdr *msg,
>  				tls_trim_both_msgs(sk, orig_size);

While this error path wasn't introduced by this patch, is it safe?

If bpf_exec_tx_verdict() returns -ENOMEM, execution jumps to wait_for_memory.
At this point, try_to_copy bytes have already been appended to the record,
msg_iter advanced, and copied incremented.

sk_stream_wait_memory() releases the socket lock and sleeps. If it wakes up and
returns an error, execution jumps to trim_sgl and calls
tls_trim_both_msgs(sk, orig_size) to truncate the record.

Does this discard the try_to_copy bytes without decrementing copied or
reverting msg_iter, returning success to userspace for deleted data?

Because the socket lock was dropped during the sleep, could a concurrent thread
have pushed the record and allocated a new one, meaning trimming this new
record using the stale orig_size from before the sleep corrupts the concurrent
thread's stream?

Also, if the verdict split the record before returning -ENOMEM, would orig_size
refer to the freed record, causing out-of-bounds trimming logic?

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

Does removing this shortcut eliminate the verdict retry mechanism?

If the verdict fails with -ENOMEM on the final data chunk (where
msg_data_left(msg) == 0) and memory subsequently becomes available, removing
this jump means execution falls through to the loop condition and immediately
terminates.

The record is fully constructed in ctx->open_rec but is never pushed to the
engine or transmitted. Does this leave the data stuck in the buffer
indefinitely until a subsequent sendmsg or socket close triggers a flush,
causing the protocol to hang?

>  	}
>  
>  send_end:

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260511174920.433155-1-kuba@kernel.org?part=3