Re: [PATCH bpf v3 2/3] tcp: Skip cond_resched() in inet_csk_listen_stop() under BPF context

[Jiayuan Chen <jiayuan.chen@linux.dev>]

On 9/10/26 7:44 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>
> Pre-existing issues:
> - [High] bpf_sock_destroy() incorrectly relies on has_current_bpf_ctx() to bypass lock_sock(), leading to lockless socket state mutations and data corruption.
> --
>
> commit 5e3e5dd8730687f6f2e1d9b86f565fa03b6e3366
> Author: Jiayuan Chen <jiayuan.chen@linux.dev>
>
> tcp: Skip cond_resched() in inet_csk_listen_stop() under BPF context
>
> This commit skips cond_resched() in inet_csk_listen_stop() when running
> under a BPF context. It prevents a "sleeping function called from invalid
> context" BUG when bpf_sock_destroy() triggers tcp_abort() from a BPF
> iterator.
>
>> diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
>> index 6257459bcee24..6a30f11384547 100644
>> --- a/net/ipv4/inet_connection_sock.c
>> +++ b/net/ipv4/inet_connection_sock.c
> [ ... ]
>> @@ -1520,7 +1520,8 @@ void inet_csk_listen_stop(struct sock *sk)
>>   		local_bh_enable();
>>   		sock_put(child);
>>   
>> -		cond_resched();
>> +		if (!has_current_bpf_ctx())
>> +			cond_resched();
> [Severity: High]
> This is a pre-existing issue, but propagating the use of
> has_current_bpf_ctx() highlights a deeper locking bug in
> bpf_sock_destroy().
>
> Does relying on has_current_bpf_ctx() in tcp_abort() and udp_abort()
> lead to lockless socket state mutations and data corruption?
>
> When a BPF program of type BPF_TRACE_ITER obtains a socket pointer via a
> lookup helper like bpf_sk_lookup_tcp(), or operates in an iterator like
> iter/task_file that doesn't inherently lock the target socket, passing
> this socket to bpf_sock_destroy() calls tcp_abort() or udp_abort().
>
> In those functions, the socket lock is currently bypassed:
>
> net/ipv4/tcp.c:tcp_abort() {
>      ...
>      /* BPF context ensures sock locking. */
>      if (!has_current_bpf_ctx())
>          /* Don't race with userspace socket closes such as tcp_close. */
>          lock_sock(sk);
>      ...
> }
>
> Because has_current_bpf_ctx() returns true in BPF context, lock_sock(sk)
> is bypassed. Modifying socket state (like accept queues, timers, and
> flags) locklessly can lead to use-after-free and list/RB-tree corruption.
>
> Since the underlying bug stems from the commit that added
> bpf_sock_destroy(), should the lock acquisition be addressed structurally
> for those code paths instead of relying on has_current_bpf_ctx()?



Those paths don't exist...

Iter progs have no sk_lookup helpers, and sock_from_file() returns an

untrusted pointer, so the only socks that can reach bpf_sock_destroy() are

the tcp/udp iter ctx args, which are already locked by the iterator.

The verifier rejects anything else with "R1 must be referenced or 
trusted", see the note on patch 1.