Merge branch 'bpf-tcp-fix-bpf_sock_destroy-on-time_wait-and-listener-socks'

Jiayuan Chen says:

====================
bpf,tcp: Fix bpf_sock_destroy() on TIME_WAIT and listener socks

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.

v2 -> v3:
 - Patch 1: add Reviewed-by from Kuniyuki.
 - Patch 2: reword why cond_resched() has to go, it can reschedule or
   report a bogus quiescent state there.
 - Patch 2: keep the cond_resched() for the non-BPF path rather than
   removing it as Kuniyuki suggested. VOLUNTARY and NONE are still
   there on some arches, and on x86 in stable where this goes, so a
   big listener close() still relies on it. Can go once those modes
   are gone.
 - Selftest: don't leak the fd if accept() unexpectedly succeeds.
v2: https://lore.kernel.org/bpf/20260906074135.185212-1-jiayuan.chen@linux.dev/

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/
====================

Link: https://patch.msgid.link/20260910112107.148770-1-jiayuan.chen@linux.dev
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
This commit is contained in:
Alexei Starovoitov 2026-09-10 16:58:40 -07:00
commit 15071f2a12
4 changed files with 162 additions and 5 deletions

View File

@ -12913,8 +12913,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)
@ -12926,8 +12927,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);

View File

@ -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. */

View File

@ -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,122 @@ 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, accept_serv = -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);
accept_serv = accept(serv, NULL, NULL);
if (!ASSERT_LT(accept_serv, 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 (accept_serv != -1)
close(accept_serv);
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 +321,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"))

View File

@ -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)