[PATCH bpf v2 0/3] bpf,tcp: Fix bpf_sock_destroy() on TIME_WAIT and listener socks

[PATCH bpf v2 0/3] bpf,tcp: Fix bpf_sock_destroy() on TIME_WAIT and listener socks

[Jiayuan Chen]

Fix two bugs in bpf_sock_destroy(). One is an out-of-bounds read of
sk->sk_protocol on TIME_WAIT and NEW_SYN_RECV socks, since the field is
not in struct sock_common. The other is a might_sleep splat when
destroying a listener with children in its accept queue, the
cond_resched() in inet_csk_listen_stop() runs under the iterator's
rcu_read_lock(). Patch 3 adds a subtest for each.

v1 -> v2:
 - Patch 1: fix the return comment too.
 - Patch 2: new.
 - Selftest: server recv()s EOF before close so the FINs can't cross,
   comment style, keep the blank line before RUN_TESTS(), add the
   tcp_listen_pending subtest.
v1: https://lore.kernel.org/bpf/20260903125306.299943-1-jiayuan.chen@linux.dev/

Jiayuan Chen (3):
  bpf: Fix out-of-bounds read of sk_protocol in bpf_sock_destroy()
  tcp: Skip cond_resched() in inet_csk_listen_stop() under BPF context
  selftests/bpf: Test bpf_sock_destroy() on TIME_WAIT and listener socks

 net/core/filter.c                             |  13 +-
 net/ipv4/inet_connection_sock.c               |   3 +-
 .../selftests/bpf/prog_tests/sock_destroy.c   | 119 ++++++++++++++++++
 .../selftests/bpf/progs/sock_destroy_prog.c   |  30 +++++
 4 files changed, 160 insertions(+), 5 deletions(-)

-- 
2.43.0

[PATCH bpf v2 1/3] bpf: Fix out-of-bounds read of sk_protocol in bpf_sock_destroy()

[Jiayuan Chen]

sk_protocol lives in struct sock, not in struct sock_common. A timewait
or request sock handed to bpf_sock_destroy() by the tcp iterator is
neither, so reading sk->sk_protocol runs past the object:

==================================================================
BUG: KASAN: slab-out-of-bounds in bpf_sock_destroy+0xc7/0xe0
Read of size 2 at addr ffff8881047d11b4 by task test_progs/428

Tainted: [W]=WARN
Call Trace:
 <TASK>
 dump_stack_lvl+0x91/0xf0
 print_report+0xd1/0x630
 kasan_report+0xf3/0x130
 __asan_report_load2_noabort+0x14/0x30
 bpf_sock_destroy+0xc7/0xe0
 bpf_prog_c3dd61f9d9cd9f37_iter_tcp6_timewait+0x9f/0xb7
 bpf_iter_run_prog+0x538/0xde0
 bpf_iter_tcp_seq_show+0x26b/0x4b0
 bpf_seq_read+0x424/0x1210
 vfs_read+0x197/0xe40
 ksys_read+0x119/0x240
 __x64_sys_read+0x72/0xc0
 x64_sys_call+0x647/0x27e0
 do_syscall_64+0xe5/0x610
 entry_SYSCALL_64_after_hwframe+0x76/0x7e

Only check sk_protocol on full socks. tcp_abort() already knows how to
deal with TIME_WAIT and NEW_SYN_RECV socks. Also fix the comment, it
never matched the code.

Fixes: 4ddbcb886268 ("bpf: Add bpf_sock_destroy kfunc")
Reported-by: Xiang Mei (Microsoft) <xmei5@asu.edu>
Closes: https://lore.kernel.org/bpf/20260702224519.800135-1-xmei5@asu.edu/
Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev>
---
A reviewer asked to add ENOENT to the list of errors in the comment.
I'd rather not list what the handlers return, that can change any time,
so the comment now says "EOPNOTSUPP, or whatever the protocol specific
destroy handler returns".

A reviewer also worried that a sockmap iter could pass an unlocked sk here
and run tcp_abort() without the sock lock. It cannot: only the tcp and
udp iters mark their sk ctx arg as PTR_TRUSTED, and bpf_sock_destroy()
needs a trusted arg. A sockmap iter prog calling it is rejected by the
verifier with "R1 must be referenced or trusted", verified in qemu.

v1 -> v2: modify comment AND avoid flaky about selftest
v1: https://lore.kernel.org/bpf/20260903125306.299943-1-jiayuan.chen@linux.dev/
---
 net/core/filter.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/net/core/filter.c b/net/core/filter.c
index 61940e753552..a41cc60a401a 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -12912,8 +12912,9 @@ __bpf_kfunc_start_defs();
  * @sock: Pointer to socket to be destroyed
  *
  * Return:
- * On error, may return EPROTONOSUPPORT, EINVAL.
- * EPROTONOSUPPORT if protocol specific destroy handler is not supported.
+ * On error, may return EOPNOTSUPP, or whatever the protocol specific
+ * destroy handler returns.
+ * EOPNOTSUPP if protocol specific destroy handler is not supported.
  * 0 otherwise
  */
 __bpf_kfunc int bpf_sock_destroy(struct sock_common *sock)
@@ -12925,8 +12926,12 @@ __bpf_kfunc int bpf_sock_destroy(struct sock_common *sock)
 	 * Supporting protocols will need to acquire sock lock in the BPF context
 	 * prior to invoking this kfunc.
 	 */
-	if (!sk->sk_prot->diag_destroy || (sk->sk_protocol != IPPROTO_TCP &&
-					   sk->sk_protocol != IPPROTO_UDP))
+	if (!sk->sk_prot->diag_destroy)
+		return -EOPNOTSUPP;
+
+	if (sk_fullsock(sk) &&
+	    sk->sk_protocol != IPPROTO_TCP &&
+	    sk->sk_protocol != IPPROTO_UDP)
 		return -EOPNOTSUPP;
 
 	return sk->sk_prot->diag_destroy(sk, ECONNABORTED);
-- 
2.43.0

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

[Jiayuan Chen]

bpf_sock_destroy() runs from the tcp iterator, under rcu_read_lock(). If
the sock is a listener that still has children in its accept queue,
tcp_abort() ends up in inet_csk_listen_stop() and the cond_resched()
there trips the debug check:

BUG: sleeping function called from invalid context at net/ipv4/inet_connection_sock.c:1523
in_atomic(): 0, irqs_disabled(): 0, non_block: 0, pid: 628, name: test_progs
preempt_count: 0, expected: 0
RCU nest depth: 1, expected: 0
locks held by test_progs/628: 3, last CPU#3:
 #0: ffff8881158cee18 (&p->lock){+.+.}-{4:4}, at: bpf_seq_read+0x56/0x1210
 #1: ffff8881106bb858 (sk_lock-AF_INET6){+.+.}-{0:0}, at: bpf_iter_tcp_seq_show+0x32b/0x4b0
 #2: ffffffffb435af20 (rcu_read_lock){....}-{1:3}, at: bpf_iter_run_prog+0x46b/0xde0
CPU: 3 UID: 0 PID: 628 Comm: test_progs Tainted: G        W           7.2.0+ #65 PREEMPT
Tainted: [W]=WARN
Call Trace:
 <TASK>
 dump_stack_lvl+0xc1/0xf0
 dump_stack+0x10/0x20
 __might_resched+0x3d2/0x610
 inet_csk_listen_stop+0x7b/0xbf0
 tcp_abort+0x23b/0x3b0
 bpf_sock_destroy+0xfc/0x140
 bpf_prog_448133d24601754f_iter_tcp6_server+0x81/0x8a
 bpf_iter_run_prog+0x538/0xde0
 bpf_iter_tcp_seq_show+0x26b/0x4b0
 bpf_seq_read+0x424/0x1210
 vfs_read+0x197/0xe40
 ksys_read+0x119/0x240
 __x64_sys_read+0x72/0xc0
 x64_sys_call+0x647/0x27e0
 do_syscall_64+0xe5/0x610
 entry_SYSCALL_64_after_hwframe+0x76/0x7e
RIP: 0033:0x7fad39b28aca
RSP: 002b:00007ffc381c61c0 EFLAGS: 00000246 ORIG_RAX: 0000000000000000
RAX: ffffffffffffffda RBX: 00007ffc381c6a88 RCX: 00007fad39b28aca
RDX: 0000000000000032 RSI: 00007ffc381c6250 RDI: 0000000000000014
RBP: 00007ffc381c61e0 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000003
R13: 0000000000000000 R14: 000055f077c1bbb0 R15: 00007fad3a0f3000
 </TASK>

The commit that added the kfunc already guards lock_sock() in tcp_abort()
and udp_abort() with has_current_bpf_ctx(), but missed the listener path.
Do the same for the cond_resched(), it can't reschedule there anyway.

Fixes: 4ddbcb886268 ("bpf: Add bpf_sock_destroy kfunc")
Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev>
---
 net/ipv4/inet_connection_sock.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
index 6257459bcee2..6a30f1138454 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();
 	}
 	if (queue->fastopenq.rskq_rst_head) {
 		/* Free all the reqs queued in rskq_rst_head. */
-- 
2.43.0

[PATCH bpf v2 3/3] selftests/bpf: Test bpf_sock_destroy() on TIME_WAIT and listener socks

[Jiayuan Chen]

Add two subtests.

tcp_timewait: the client shuts down first and the server closes after
it, so the client sock ends up in TIME_WAIT. A tcp iterator then finds
the timewait sock by the cookie it inherited from the client sock and
destroys it. Iterate once more to make sure it is gone. Without the
first fix bpf_sock_destroy() reads past the timewait sock and KASAN
complains.

tcp_listen_pending: connect to a listener but never accept, so the child
sits in the accept queue, then destroy the listener. Without the second
fix the cond_resched() in inet_csk_listen_stop() trips the might_sleep
check under rcu_read_lock().

  ./test_progs -a sock_destroy

  #444/1   sock_destroy/tcp_client:OK
  #444/2   sock_destroy/tcp_server:OK
  #444/3   sock_destroy/tcp_listen_pending:OK
  #444/4   sock_destroy/tcp_timewait:OK
  #444/5   sock_destroy/udp_client:OK
  #444/6   sock_destroy/udp_server:OK
  #444/7   sock_destroy/trace_tcp_destroy_sock:OK
  #444     sock_destroy:OK
  Summary: 1/7 PASSED, 0 SKIPPED, 0/0 FAILED

Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev>
---
 .../selftests/bpf/prog_tests/sock_destroy.c   | 119 ++++++++++++++++++
 .../selftests/bpf/progs/sock_destroy_prog.c   |  30 +++++
 2 files changed, 149 insertions(+)

diff --git a/tools/testing/selftests/bpf/prog_tests/sock_destroy.c b/tools/testing/selftests/bpf/prog_tests/sock_destroy.c
index 9c11938fe597..c9f0885709c7 100644
--- a/tools/testing/selftests/bpf/prog_tests/sock_destroy.c
+++ b/tools/testing/selftests/bpf/prog_tests/sock_destroy.c
@@ -1,4 +1,5 @@
 // SPDX-License-Identifier: GPL-2.0
+#include <poll.h>
 #include <test_progs.h>
 #include <bpf/bpf_endian.h>
 
@@ -110,6 +111,120 @@ static void test_tcp_server(struct sock_destroy_prog *skel)
 		close(serv);
 }
 
+static void test_tcp_listen_pending(struct sock_destroy_prog *skel)
+{
+	int serv = -1, clien = -1, n, serv_port;
+	struct pollfd pfd = { .events = POLLIN };
+	char buf[1];
+
+	serv = start_server(AF_INET6, SOCK_STREAM, NULL, 0, 0);
+	if (!ASSERT_GE(serv, 0, "start_server"))
+		goto cleanup;
+	serv_port = get_socket_local_port(serv);
+	if (!ASSERT_GE(serv_port, 0, "get_sock_local_port"))
+		goto cleanup;
+	skel->bss->serv_port = (__be16)serv_port;
+
+	/*
+	 * Connect but never accept, so the child sits in the accept queue
+	 * of the listener. Wait until it's actually there.
+	 */
+	clien = connect_to_fd(serv, 0);
+	if (!ASSERT_GE(clien, 0, "connect_to_fd"))
+		goto cleanup;
+	pfd.fd = serv;
+	if (!ASSERT_EQ(poll(&pfd, 1, -1), 1, "poll listener"))
+		goto cleanup;
+
+	/* Run iterator program that destroys server sockets. */
+	start_iter_sockets(skel->progs.iter_tcp6_server);
+
+	n = accept(serv, NULL, NULL);
+	if (!ASSERT_LT(n, 0, "accept on destroyed listener"))
+		goto cleanup;
+	ASSERT_EQ(errno, EINVAL, "error code on destroyed listener");
+
+	/* The unaccepted child was reset along with the listener. */
+	n = recv(clien, buf, sizeof(buf), 0);
+	if (!ASSERT_LT(n, 0, "client recv on reset child"))
+		goto cleanup;
+	ASSERT_EQ(errno, ECONNRESET, "error code on reset child");
+
+cleanup:
+	if (clien != -1)
+		close(clien);
+	if (serv != -1)
+		close(serv);
+}
+
+static void test_tcp_timewait(struct sock_destroy_prog *skel)
+{
+	int serv = -1, clien = -1, accept_serv = -1, n;
+	struct timeval tv = {};
+	char buf[1];
+
+	serv = start_server(AF_INET6, SOCK_STREAM, NULL, 0, 0);
+	if (!ASSERT_GE(serv, 0, "start_server"))
+		goto cleanup;
+
+	clien = connect_to_fd(serv, 0);
+	if (!ASSERT_GE(clien, 0, "connect_to_fd"))
+		goto cleanup;
+
+	accept_serv = accept(serv, NULL, NULL);
+	if (!ASSERT_GE(accept_serv, 0, "serv accept"))
+		goto cleanup;
+
+	/*
+	 * Active close from the client, then close the server side. Once
+	 * recv() sees EOF the server FIN has been processed and the client
+	 * sock is in TIME_WAIT. Block without timeout so a loaded CI box
+	 * can't race us.
+	 */
+	if (!ASSERT_OK(setsockopt(clien, SOL_SOCKET, SO_RCVTIMEO, &tv,
+				  sizeof(tv)), "clear rcvtimeo"))
+		goto cleanup;
+	if (!ASSERT_OK(shutdown(clien, SHUT_WR), "client shutdown"))
+		goto cleanup;
+
+	/*
+	 * Make sure the server has seen the client FIN before it closes,
+	 * so the two FINs never cross.
+	 */
+	n = recv(accept_serv, buf, sizeof(buf), 0);
+	if (!ASSERT_EQ(n, 0, "server recv EOF"))
+		goto cleanup;
+
+	close(accept_serv);
+	accept_serv = -1;
+
+	/* block until return EOF */
+	n = recv(clien, buf, sizeof(buf), 0);
+	if (!ASSERT_EQ(n, 0, "client recv EOF"))
+		goto cleanup;
+
+	/* Run iterator program that destroys the timewait client sock. */
+	skel->bss->tw_found = 0;
+	start_iter_sockets(skel->progs.iter_tcp6_timewait);
+	if (!ASSERT_EQ(skel->bss->tw_found, 1, "timewait sock found"))
+		goto cleanup;
+
+	ASSERT_OK(skel->bss->tw_destroy_err, "destroy timewait sock");
+
+	/* The destroyed timewait sock must be gone. */
+	skel->bss->tw_found = 0;
+	start_iter_sockets(skel->progs.iter_tcp6_timewait);
+	ASSERT_EQ(skel->bss->tw_found, 0, "timewait sock destroyed");
+
+cleanup:
+	if (clien != -1)
+		close(clien);
+	if (accept_serv != -1)
+		close(accept_serv);
+	if (serv != -1)
+		close(serv);
+}
+
 static void test_udp_client(struct sock_destroy_prog *skel)
 {
 	int serv = -1, clien = -1, n = 0;
@@ -204,6 +319,10 @@ void test_sock_destroy(void)
 		test_tcp_client(skel);
 	if (test__start_subtest("tcp_server"))
 		test_tcp_server(skel);
+	if (test__start_subtest("tcp_listen_pending"))
+		test_tcp_listen_pending(skel);
+	if (test__start_subtest("tcp_timewait"))
+		test_tcp_timewait(skel);
 	if (test__start_subtest("udp_client"))
 		test_udp_client(skel);
 	if (test__start_subtest("udp_server"))
diff --git a/tools/testing/selftests/bpf/progs/sock_destroy_prog.c b/tools/testing/selftests/bpf/progs/sock_destroy_prog.c
index 9e0bf7a54cec..0a8887543218 100644
--- a/tools/testing/selftests/bpf/progs/sock_destroy_prog.c
+++ b/tools/testing/selftests/bpf/progs/sock_destroy_prog.c
@@ -7,6 +7,8 @@
 #include "bpf_tracing_net.h"
 
 __be16 serv_port = 0;
+int tw_found = 0;
+int tw_destroy_err = 0;
 
 int bpf_sock_destroy(struct sock_common *sk) __ksym;
 
@@ -100,6 +102,34 @@ int iter_tcp6_server(struct bpf_iter__tcp *ctx)
 	return 0;
 }
 
+SEC("iter/tcp")
+int iter_tcp6_timewait(struct bpf_iter__tcp *ctx)
+{
+	struct sock_common *sk_common = ctx->sk_common;
+	__u64 *val;
+	int key = 0;
+
+	if (!sk_common)
+		return 0;
+
+	if (sk_common->skc_family != AF_INET6)
+		return 0;
+
+	if (!bpf_skc_to_tcp_timewait_sock(sk_common))
+		return 0;
+
+	val = bpf_map_lookup_elem(&tcp_conn_sockets, &key);
+	if (!val)
+		return 0;
+	/* The timewait sock inherits the cookie of the closed client sock. */
+	if (bpf_get_socket_cookie(sk_common) != *val)
+		return 0;
+
+	tw_found++;
+	tw_destroy_err = bpf_sock_destroy(sk_common);
+
+	return 0;
+}
 
 SEC("iter/udp")
 int iter_udp6_client(struct bpf_iter__udp *ctx)
-- 
2.43.0

Re: [PATCH bpf v2 3/3] selftests/bpf: Test bpf_sock_destroy() on TIME_WAIT and listener socks

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The file descriptor returned by `accept()` is leaked on the test failure path in `test_tcp_listen_pending()`.
--

commit a0eed787a7f06953fc57cb1d362195f000753ca9
Author: Jiayuan Chen <jiayuan.chen@linux.dev>

selftests/bpf: Test bpf_sock_destroy() on TIME_WAIT and listener socks

This commit adds two subtests to the sock_destroy BPF selftest. The tests
ensure that bpf_sock_destroy() correctly handles sockets in TIME_WAIT and
listener sockets with pending connections in the accept queue.

> diff --git a/tools/testing/selftests/bpf/prog_tests/sock_destroy.c b/tools/testing/selftests/bpf/prog_tests/sock_destroy.c
> index 9c11938fe597d..c9f0885709c7e 100644
> --- a/tools/testing/selftests/bpf/prog_tests/sock_destroy.c
> +++ b/tools/testing/selftests/bpf/prog_tests/sock_destroy.c

[ ... ]

> @@ -110,6 +111,120 @@ static void test_tcp_server(struct sock_destroy_prog *skel)
>  		close(serv);
>  }
>  
> +static void test_tcp_listen_pending(struct sock_destroy_prog *skel)
> +{
> +	int serv = -1, clien = -1, n, serv_port;
> +	struct pollfd pfd = { .events = POLLIN };
> +	char buf[1];
> +
> +	serv = start_server(AF_INET6, SOCK_STREAM, NULL, 0, 0);

[ ... ]

> +	/* Run iterator program that destroys server sockets. */
> +	start_iter_sockets(skel->progs.iter_tcp6_server);
> +
> +	n = accept(serv, NULL, NULL);
> +	if (!ASSERT_LT(n, 0, "accept on destroyed listener"))
> +		goto cleanup;

[Severity: Medium]
Does this code leak the file descriptor on the error path?

If accept(serv, NULL, NULL) unexpectedly succeeds, it returns a valid
file descriptor in n (where n >= 0). The ASSERT_LT(n, 0) check then
fails and execution jumps to the cleanup block.

Because n is not closed in cleanup, the accepted socket is leaked.

> +	ASSERT_EQ(errno, EINVAL, "error code on destroyed listener");
> +
> +	/* The unaccepted child was reset along with the listener. */
> +	n = recv(clien, buf, sizeof(buf), 0);
> +	if (!ASSERT_LT(n, 0, "client recv on reset child"))
> +		goto cleanup;
> +	ASSERT_EQ(errno, ECONNRESET, "error code on reset child");
> +
> +cleanup:
> +	if (clien != -1)
> +		close(clien);
> +	if (serv != -1)
> +		close(serv);
> +}

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

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Medium] Unconditionally skipping `cond_resched()` in `inet_csk_listen_stop()` during BPF execution forces unbounded accept queue processing to run atomically, causing CPU soft lockups and RCU stalls.

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but the TCP BPF iterator acquires `lock_sock()` on unaccepted child sockets, causing destructive data races if the parent listener is closed concurrently.
--

commit ab882b1f2e506040f7bcef30415806e9b24c59a4
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 called under a
BPF context. This prevents a "sleeping function called from invalid context"
bug when an iterator BPF program calls bpf_sock_destroy() on a listener
socket.

> 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

[Severity: High]
This isn't a bug introduced by this patch, but does the TCP BPF iterator
acquire lock_sock() on unaccepted child sockets, causing destructive data
races if the parent listener is closed concurrently?

If a TCP BPF iterator is running in bpf_iter_tcp_seq_show() and visits a
child socket in the ehash table that has not yet been accepted by userspace:

net/ipv4/tcp_ipv4.c:bpf_iter_tcp_seq_show() {
    ...
    if (sk_fullsock(sk))
        lock_sock(sk);
    ...
}

And concurrently, the parent listener is closed, invoking
inet_csk_listen_stop():

net/ipv4/inet_connection_sock.c:inet_csk_listen_stop() {
    ...
    bh_lock_sock(child);
    WARN_ON(sock_owned_by_user(child));
    ...
}

inet_csk_listen_stop() only takes the spinlock (bh_lock_sock()), triggers
the WARN_ON, and completely ignores the mutex, destructively modifying the
socket while the BPF program actively uses it.

> @@ -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: Medium]
Can unconditionally skipping cond_resched() here force unbounded accept queue
processing to run atomically?

Because the loop dismantles the accept queue (which can contain up to
somaxconn sockets), executing it entirely within a non-preemptible RCU
read-side section could cause CPU soft lockups and RCU stalls.

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

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

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 3684 bytes --]

> tcp: Skip cond_resched() in inet_csk_listen_stop() under BPF context
>
> bpf_sock_destroy() runs from the tcp iterator, under rcu_read_lock(). If
> the sock is a listener that still has children in its accept queue,
> tcp_abort() ends up in inet_csk_listen_stop() and the cond_resched()
> there trips the debug check:
>
> BUG: sleeping function called from invalid context at net/ipv4/inet_connection_sock.c:1523
> in_atomic(): 0, irqs_disabled(): 0, non_block: 0, pid: 628, name: test_progs
> preempt_count: 0, expected: 0
> RCU nest depth: 1, expected: 0
> locks held by test_progs/628: 3, last CPU#3:
>  #0: ffff8881158cee18 (&p->lock){+.+.}-{4:4}, at: bpf_seq_read+0x56/0x1210
>  #1: ffff8881106bb858 (sk_lock-AF_INET6){+.+.}-{0:0}, at: bpf_iter_tcp_seq_show+0x32b/0x4b0
>  #2: ffffffffb435af20 (rcu_read_lock){....}-{1:3}, at: bpf_iter_run_prog+0x46b/0xde0
> CPU: 3 UID: 0 PID: 628 Comm: test_progs Tainted: G        W           7.2.0+ #65 PREEMPT
> Tainted: [W]=WARN
> Call Trace:
>  <TASK>
>  dump_stack_lvl+0xc1/0xf0
>  dump_stack+0x10/0x20
>  __might_resched+0x3d2/0x610
>  inet_csk_listen_stop+0x7b/0xbf0
>  tcp_abort+0x23b/0x3b0
>  bpf_sock_destroy+0xfc/0x140
>  bpf_prog_448133d24601754f_iter_tcp6_server+0x81/0x8a
>  bpf_iter_run_prog+0x538/0xde0
>  bpf_iter_tcp_seq_show+0x26b/0x4b0
>  bpf_seq_read+0x424/0x1210
>  vfs_read+0x197/0xe40
>  ksys_read+0x119/0x240
>  __x64_sys_read+0x72/0xc0
>  x64_sys_call+0x647/0x27e0
>  do_syscall_64+0xe5/0x610
>  entry_SYSCALL_64_after_hwframe+0x76/0x7e
> RIP: 0033:0x7fad39b28aca
> RSP: 002b:00007ffc381c61c0 EFLAGS: 00000246 ORIG_RAX: 0000000000000000
> RAX: ffffffffffffffda RBX: 00007ffc381c6a88 RCX: 00007fad39b28aca
> RDX: 0000000000000032 RSI: 00007ffc381c6250 RDI: 0000000000000014
> RBP: 00007ffc381c61e0 R08: 0000000000000000 R09: 0000000000000000
> R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000003
> R13: 0000000000000000 R14: 000055f077c1bbb0 R15: 00007fad3a0f3000
>  </TASK>
>
> The commit that added the kfunc already guards lock_sock() in tcp_abort()
> and udp_abort() with has_current_bpf_ctx(), but missed the listener path.
> Do the same for the cond_resched(), it can't reschedule there anyway.
>
> Fixes: 4ddbcb886268 ("bpf: Add bpf_sock_destroy kfunc")
> Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev>

Is the justification "it can't reschedule there anyway" accurate?

With CONFIG_PREEMPT_DYNAMIC=y booted with preempt=none or preempt=voluntary,
cond_resched() expands to __cond_resched() which can actually reschedule.
The splat in the commit message confirms preempt_count is 0 while RCU nest
depth is 1. With preempt_count==0, should_resched(0) can be true and
__cond_resched() will call preempt_schedule_common() for a real reschedule.

Additionally, in configurations with CONFIG_PREEMPT_RCU=n where
rcu_read_lock() is preempt_disable(), __cond_resched() falls through to
rcu_all_qs() which calls rcu_qs() to report a quiescent state from inside
an RCU read-side critical section. That's a correctness problem beyond just
the debug check.

So the call can either reschedule (PREEMPT_DYNAMIC none/voluntary) or report
a bogus quiescent state (non-preemptible RCU). Could the justification be
reworded to explain that the loop runs inside the iterator's RCU read-side
critical section and must not reschedule or report a quiescent state there?
The code change itself is correct and matches the existing pattern in
tcp_abort() and udp_abort().


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/34020353095

Re: [PATCH bpf v2 1/3] bpf: Fix out-of-bounds read of sk_protocol in bpf_sock_destroy()

[Kuniyuki Iwashima]

On Sun, Sep 6, 2026 at 12:42 AM Jiayuan Chen <jiayuan.chen@linux.dev> wrote:
>
> sk_protocol lives in struct sock, not in struct sock_common. A timewait
> or request sock handed to bpf_sock_destroy() by the tcp iterator is
> neither, so reading sk->sk_protocol runs past the object:
>
> ==================================================================
> BUG: KASAN: slab-out-of-bounds in bpf_sock_destroy+0xc7/0xe0
> Read of size 2 at addr ffff8881047d11b4 by task test_progs/428
>
> Tainted: [W]=WARN
> Call Trace:
>  <TASK>
>  dump_stack_lvl+0x91/0xf0
>  print_report+0xd1/0x630
>  kasan_report+0xf3/0x130
>  __asan_report_load2_noabort+0x14/0x30
>  bpf_sock_destroy+0xc7/0xe0
>  bpf_prog_c3dd61f9d9cd9f37_iter_tcp6_timewait+0x9f/0xb7
>  bpf_iter_run_prog+0x538/0xde0
>  bpf_iter_tcp_seq_show+0x26b/0x4b0
>  bpf_seq_read+0x424/0x1210
>  vfs_read+0x197/0xe40
>  ksys_read+0x119/0x240
>  __x64_sys_read+0x72/0xc0
>  x64_sys_call+0x647/0x27e0
>  do_syscall_64+0xe5/0x610
>  entry_SYSCALL_64_after_hwframe+0x76/0x7e
>
> Only check sk_protocol on full socks. tcp_abort() already knows how to
> deal with TIME_WAIT and NEW_SYN_RECV socks. Also fix the comment, it
> never matched the code.
>
> Fixes: 4ddbcb886268 ("bpf: Add bpf_sock_destroy kfunc")
> Reported-by: Xiang Mei (Microsoft) <xmei5@asu.edu>
> Closes: https://lore.kernel.org/bpf/20260702224519.800135-1-xmei5@asu.edu/
> Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev>

Reviewed-by: Kuniyuki Iwashima <kuniyu@google.com>

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

[Kuniyuki Iwashima]

On Sun, Sep 6, 2026 at 1:23 AM <bot+bpf-ci@kernel.org> wrote:
>
> > tcp: Skip cond_resched() in inet_csk_listen_stop() under BPF context
> >
> > bpf_sock_destroy() runs from the tcp iterator, under rcu_read_lock(). If
> > the sock is a listener that still has children in its accept queue,
> > tcp_abort() ends up in inet_csk_listen_stop() and the cond_resched()
> > there trips the debug check:
> >
> > BUG: sleeping function called from invalid context at net/ipv4/inet_connection_sock.c:1523
> > in_atomic(): 0, irqs_disabled(): 0, non_block: 0, pid: 628, name: test_progs
> > preempt_count: 0, expected: 0
> > RCU nest depth: 1, expected: 0
> > locks held by test_progs/628: 3, last CPU#3:
> >  #0: ffff8881158cee18 (&p->lock){+.+.}-{4:4}, at: bpf_seq_read+0x56/0x1210
> >  #1: ffff8881106bb858 (sk_lock-AF_INET6){+.+.}-{0:0}, at: bpf_iter_tcp_seq_show+0x32b/0x4b0
> >  #2: ffffffffb435af20 (rcu_read_lock){....}-{1:3}, at: bpf_iter_run_prog+0x46b/0xde0
> > CPU: 3 UID: 0 PID: 628 Comm: test_progs Tainted: G        W           7.2.0+ #65 PREEMPT
> > Tainted: [W]=WARN
> > Call Trace:
> >  <TASK>
> >  dump_stack_lvl+0xc1/0xf0
> >  dump_stack+0x10/0x20
> >  __might_resched+0x3d2/0x610
> >  inet_csk_listen_stop+0x7b/0xbf0
> >  tcp_abort+0x23b/0x3b0
> >  bpf_sock_destroy+0xfc/0x140
> >  bpf_prog_448133d24601754f_iter_tcp6_server+0x81/0x8a
> >  bpf_iter_run_prog+0x538/0xde0
> >  bpf_iter_tcp_seq_show+0x26b/0x4b0
> >  bpf_seq_read+0x424/0x1210
> >  vfs_read+0x197/0xe40
> >  ksys_read+0x119/0x240
> >  __x64_sys_read+0x72/0xc0
> >  x64_sys_call+0x647/0x27e0
> >  do_syscall_64+0xe5/0x610
> >  entry_SYSCALL_64_after_hwframe+0x76/0x7e
> > RIP: 0033:0x7fad39b28aca
> > RSP: 002b:00007ffc381c61c0 EFLAGS: 00000246 ORIG_RAX: 0000000000000000
> > RAX: ffffffffffffffda RBX: 00007ffc381c6a88 RCX: 00007fad39b28aca
> > RDX: 0000000000000032 RSI: 00007ffc381c6250 RDI: 0000000000000014
> > RBP: 00007ffc381c61e0 R08: 0000000000000000 R09: 0000000000000000
> > R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000003
> > R13: 0000000000000000 R14: 000055f077c1bbb0 R15: 00007fad3a0f3000
> >  </TASK>
> >
> > The commit that added the kfunc already guards lock_sock() in tcp_abort()
> > and udp_abort() with has_current_bpf_ctx(), but missed the listener path.
> > Do the same for the cond_resched(), it can't reschedule there anyway.
> >
> > Fixes: 4ddbcb886268 ("bpf: Add bpf_sock_destroy kfunc")
> > Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev>
>
> Is the justification "it can't reschedule there anyway" accurate?
>
> With CONFIG_PREEMPT_DYNAMIC=y booted with preempt=none or preempt=voluntary,
> cond_resched() expands to __cond_resched() which can actually reschedule.
> The splat in the commit message confirms preempt_count is 0 while RCU nest
> depth is 1. With preempt_count==0, should_resched(0) can be true and
> __cond_resched() will call preempt_schedule_common() for a real reschedule.
>
> Additionally, in configurations with CONFIG_PREEMPT_RCU=n where
> rcu_read_lock() is preempt_disable(), __cond_resched() falls through to
> rcu_all_qs() which calls rcu_qs() to report a quiescent state from inside
> an RCU read-side critical section. That's a correctness problem beyond just
> the debug check.
>
> So the call can either reschedule (PREEMPT_DYNAMIC none/voluntary) or report
> a bogus quiescent state (non-preemptible RCU). Could the justification be
> reworded to explain that the loop runs inside the iterator's RCU read-side
> critical section and must not reschedule or report a quiescent state there?
> The code change itself is correct and matches the existing pattern in
> tcp_abort() and udp_abort().

or maybe simply remove cond_resched(), hoping 7dadeaa6e851 would
resolve the scheduling issue.


>
>
> ---
> AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
> See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
>
> CI run summary: https://github.com/kernel-patches/bpf/actions/runs/34020353095

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

[Jiayuan Chen]

On 9/8/26 7:34 AM, Kuniyuki Iwashima wrote:
> On Sun, Sep 6, 2026 at 1:23 AM <bot+bpf-ci@kernel.org> wrote:
>>> tcp: Skip cond_resched() in inet_csk_listen_stop() under BPF context
>>>
>>> bpf_sock_destroy() runs from the tcp iterator, under rcu_read_lock(). If
>>> the sock is a listener that still has children in its accept queue,
>>> tcp_abort() ends up in inet_csk_listen_stop() and the cond_resched()
>>> there trips the debug check:
>>>
>>> BUG: sleeping function called from invalid context at net/ipv4/inet_connection_sock.c:1523
>>> in_atomic(): 0, irqs_disabled(): 0, non_block: 0, pid: 628, name: test_progs
>>> preempt_count: 0, expected: 0
>>> RCU nest depth: 1, expected: 0
>>> locks held by test_progs/628: 3, last CPU#3:
>>>   #0: ffff8881158cee18 (&p->lock){+.+.}-{4:4}, at: bpf_seq_read+0x56/0x1210
>>>   #1: ffff8881106bb858 (sk_lock-AF_INET6){+.+.}-{0:0}, at: bpf_iter_tcp_seq_show+0x32b/0x4b0
>>>   #2: ffffffffb435af20 (rcu_read_lock){....}-{1:3}, at: bpf_iter_run_prog+0x46b/0xde0
>>> CPU: 3 UID: 0 PID: 628 Comm: test_progs Tainted: G        W           7.2.0+ #65 PREEMPT
>>> Tainted: [W]=WARN
>>> Call Trace:
>>>   <TASK>
>>>   dump_stack_lvl+0xc1/0xf0
>>>   dump_stack+0x10/0x20
>>>   __might_resched+0x3d2/0x610
>>>   inet_csk_listen_stop+0x7b/0xbf0
>>>   tcp_abort+0x23b/0x3b0
>>>   bpf_sock_destroy+0xfc/0x140
>>>   bpf_prog_448133d24601754f_iter_tcp6_server+0x81/0x8a
>>>   bpf_iter_run_prog+0x538/0xde0
>>>   bpf_iter_tcp_seq_show+0x26b/0x4b0
>>>   bpf_seq_read+0x424/0x1210
>>>   vfs_read+0x197/0xe40
>>>   ksys_read+0x119/0x240
>>>   __x64_sys_read+0x72/0xc0
>>>   x64_sys_call+0x647/0x27e0
>>>   do_syscall_64+0xe5/0x610
>>>   entry_SYSCALL_64_after_hwframe+0x76/0x7e
>>> RIP: 0033:0x7fad39b28aca
>>> RSP: 002b:00007ffc381c61c0 EFLAGS: 00000246 ORIG_RAX: 0000000000000000
>>> RAX: ffffffffffffffda RBX: 00007ffc381c6a88 RCX: 00007fad39b28aca
>>> RDX: 0000000000000032 RSI: 00007ffc381c6250 RDI: 0000000000000014
>>> RBP: 00007ffc381c61e0 R08: 0000000000000000 R09: 0000000000000000
>>> R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000003
>>> R13: 0000000000000000 R14: 000055f077c1bbb0 R15: 00007fad3a0f3000
>>>   </TASK>
>>>
>>> The commit that added the kfunc already guards lock_sock() in tcp_abort()
>>> and udp_abort() with has_current_bpf_ctx(), but missed the listener path.
>>> Do the same for the cond_resched(), it can't reschedule there anyway.
>>>
>>> Fixes: 4ddbcb886268 ("bpf: Add bpf_sock_destroy kfunc")
>>> Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev>
>> Is the justification "it can't reschedule there anyway" accurate?
>>
>> With CONFIG_PREEMPT_DYNAMIC=y booted with preempt=none or preempt=voluntary,
>> cond_resched() expands to __cond_resched() which can actually reschedule.
>> The splat in the commit message confirms preempt_count is 0 while RCU nest
>> depth is 1. With preempt_count==0, should_resched(0) can be true and
>> __cond_resched() will call preempt_schedule_common() for a real reschedule.
>>
>> Additionally, in configurations with CONFIG_PREEMPT_RCU=n where
>> rcu_read_lock() is preempt_disable(), __cond_resched() falls through to
>> rcu_all_qs() which calls rcu_qs() to report a quiescent state from inside
>> an RCU read-side critical section. That's a correctness problem beyond just
>> the debug check.
>>
>> So the call can either reschedule (PREEMPT_DYNAMIC none/voluntary) or report
>> a bogus quiescent state (non-preemptible RCU). Could the justification be
>> reworded to explain that the loop runs inside the iterator's RCU read-side
>> critical section and must not reschedule or report a quiescent state there?
>> The code change itself is correct and matches the existing pattern in
>> tcp_abort() and udp_abort().
> or maybe simply remove cond_resched(), hoping 7dadeaa6e851 would
> resolve the scheduling issue.
>

Good suggestion. cond_resched has become old practice under 
CONFIG_PREEMPT_LAZY

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

[Jiayuan Chen]

On 9/8/26 4:07 PM, Jiayuan Chen wrote:
>
> On 9/8/26 7:34 AM, Kuniyuki Iwashima wrote:
>> On Sun, Sep 6, 2026 at 1:23 AM <bot+bpf-ci@kernel.org> wrote:
>>>> tcp: Skip cond_resched() in inet_csk_listen_stop() under BPF context
>>>>
>>>> bpf_sock_destroy() runs from the tcp iterator, under 
>>>> rcu_read_lock(). If
>>>> the sock is a listener that still has children in its accept queue,
>>>> tcp_abort() ends up in inet_csk_listen_stop() and the cond_resched()
>>>> there trips the debug check:
>>>>
>>>> BUG: sleeping function called from invalid context at 
>>>> net/ipv4/inet_connection_sock.c:1523
>>>> in_atomic(): 0, irqs_disabled(): 0, non_block: 0, pid: 628, name: 
>>>> test_progs
>>>> preempt_count: 0, expected: 0
>>>> RCU nest depth: 1, expected: 0
>>>> locks held by test_progs/628: 3, last CPU#3:
>>>>   #0: ffff8881158cee18 (&p->lock){+.+.}-{4:4}, at: 
>>>> bpf_seq_read+0x56/0x1210
>>>>   #1: ffff8881106bb858 (sk_lock-AF_INET6){+.+.}-{0:0}, at: 
>>>> bpf_iter_tcp_seq_show+0x32b/0x4b0
>>>>   #2: ffffffffb435af20 (rcu_read_lock){....}-{1:3}, at: 
>>>> bpf_iter_run_prog+0x46b/0xde0
>>>> CPU: 3 UID: 0 PID: 628 Comm: test_progs Tainted: G W           
>>>> 7.2.0+ #65 PREEMPT
>>>> Tainted: [W]=WARN
>>>> Call Trace:
>>>>   <TASK>
>>>>   dump_stack_lvl+0xc1/0xf0
>>>>   dump_stack+0x10/0x20
>>>>   __might_resched+0x3d2/0x610
>>>>   inet_csk_listen_stop+0x7b/0xbf0
>>>>   tcp_abort+0x23b/0x3b0
>>>>   bpf_sock_destroy+0xfc/0x140
>>>>   bpf_prog_448133d24601754f_iter_tcp6_server+0x81/0x8a
>>>>   bpf_iter_run_prog+0x538/0xde0
>>>>   bpf_iter_tcp_seq_show+0x26b/0x4b0
>>>>   bpf_seq_read+0x424/0x1210
>>>>   vfs_read+0x197/0xe40
>>>>   ksys_read+0x119/0x240
>>>>   __x64_sys_read+0x72/0xc0
>>>>   x64_sys_call+0x647/0x27e0
>>>>   do_syscall_64+0xe5/0x610
>>>>   entry_SYSCALL_64_after_hwframe+0x76/0x7e
>>>> RIP: 0033:0x7fad39b28aca
>>>> RSP: 002b:00007ffc381c61c0 EFLAGS: 00000246 ORIG_RAX: 0000000000000000
>>>> RAX: ffffffffffffffda RBX: 00007ffc381c6a88 RCX: 00007fad39b28aca
>>>> RDX: 0000000000000032 RSI: 00007ffc381c6250 RDI: 0000000000000014
>>>> RBP: 00007ffc381c61e0 R08: 0000000000000000 R09: 0000000000000000
>>>> R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000003
>>>> R13: 0000000000000000 R14: 000055f077c1bbb0 R15: 00007fad3a0f3000
>>>>   </TASK>
>>>>
>>>> The commit that added the kfunc already guards lock_sock() in 
>>>> tcp_abort()
>>>> and udp_abort() with has_current_bpf_ctx(), but missed the listener 
>>>> path.
>>>> Do the same for the cond_resched(), it can't reschedule there anyway.
>>>>
>>>> Fixes: 4ddbcb886268 ("bpf: Add bpf_sock_destroy kfunc")
>>>> Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev>
>>> Is the justification "it can't reschedule there anyway" accurate?
>>>
>>> With CONFIG_PREEMPT_DYNAMIC=y booted with preempt=none or 
>>> preempt=voluntary,
>>> cond_resched() expands to __cond_resched() which can actually 
>>> reschedule.
>>> The splat in the commit message confirms preempt_count is 0 while 
>>> RCU nest
>>> depth is 1. With preempt_count==0, should_resched(0) can be true and
>>> __cond_resched() will call preempt_schedule_common() for a real 
>>> reschedule.
>>>
>>> Additionally, in configurations with CONFIG_PREEMPT_RCU=n where
>>> rcu_read_lock() is preempt_disable(), __cond_resched() falls through to
>>> rcu_all_qs() which calls rcu_qs() to report a quiescent state from 
>>> inside
>>> an RCU read-side critical section. That's a correctness problem 
>>> beyond just
>>> the debug check.
>>>
>>> So the call can either reschedule (PREEMPT_DYNAMIC none/voluntary) 
>>> or report
>>> a bogus quiescent state (non-preemptible RCU). Could the 
>>> justification be
>>> reworded to explain that the loop runs inside the iterator's RCU 
>>> read-side
>>> critical section and must not reschedule or report a quiescent state 
>>> there?
>>> The code change itself is correct and matches the existing pattern in
>>> tcp_abort() and udp_abort().
>> or maybe simply remove cond_resched(), hoping 7dadeaa6e851 would
>> resolve the scheduling issue.
>>
>
> Good suggestion. cond_resched has become old practice under 
> CONFIG_PREEMPT_LAZY


After reconsideration, I think it's not a good idea to remove it if we 
treat it as a fix and the fix will be backported to LTS.

Or we just drop both cond_resched and Fixes tag.