Re: [PATCH net v2 4/4] net: tls: remove bad rollback and UAF on ENOSPC

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] Silent data loss and stale state usage (UAF) in the -ENOMEM error path.
- [High] BPF policy bypass due to uncleared psock->eval state on -ENOMEM.
--

commit 9867dad26c6eb929c35af6ad7b485aac95c01aef
Author: Jakub Kicinski <kuba@kernel.org>

net: tls: remove bad rollback and UAF on ENOSPC

This commit removes the attempt to roll back the iterator when BPF
encounters an ENOSPC error. Since BPF may have modified the message,
variables like orig_size and other pointers become stale, so rolling
them back leads to use-after-free or corruption.

> diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
> index 360f71fd78842..22b77840e35ae 100644
> --- a/net/tls/tls_sw.c
> +++ b/net/tls/tls_sw.c
> @@ -1164,11 +1164,8 @@ static int tls_sw_sendmsg_locked(struct sock *sk, struct msghdr *msg,
>  				else if (ret == -ENOMEM)
>  					goto wait_for_memory;

The commit message notes that BPF may have modified the message, making
pointers and state stale. Does this same stale state issue still exist in
the -ENOMEM path?

If bpf_exec_tx_verdict() executes BPF (which may push or pop data) and then
fails with -ENOMEM, execution jumps to wait_for_memory:

wait_for_memory:
        ret = sk_stream_wait_memory(sk, &timeo);
        if (ret) {
trim_sgl:
            if (ctx->open_rec)
                tls_trim_both_msgs(sk, orig_size);
            goto send_end;
        }

If sk_stream_wait_memory() fails, the code calls tls_trim_both_msgs(sk,
orig_size). Can trimming to the pre-BPF orig_size corrupt the scatterlist
if BPF changed the size?

Additionally, copied was already incremented by try_to_copy. Since the
function returns copied, might it falsely indicate to userspace that the
discarded bytes were successfully sent, causing silent data corruption?

What happens if sk_stream_wait_memory() succeeds instead? The loop in
tls_sw_sendmsg_locked() restarts, appending more data from userspace to
the open record.

If tls_push_record() inside the bpf_exec_tx_verdict() path returns -ENOMEM,
does it return early without resetting psock->eval to __SK_NONE?

If so, when bpf_exec_tx_verdict() is called again, could it see psock->eval
evaluating to __SK_PASS and skip the BPF evaluation entirely, allowing
the newly appended data to be sent without verification?

>  				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;
>  				} else if (ret != -EAGAIN)
>  					goto send_end;
>  			}

[ ... ]

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