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

[sashiko-bot@kernel.org]

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()?

>  	}
>  	if (queue->fastopenq.rskq_rst_head) {
>  		/* Free all the reqs queued in rskq_rst_head. */

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260910112107.148770-1-jiayuan.chen@linux.dev?part=2