netdev - Re: [PATCH v1 bpf-next 10/10] selftests/bpf: Add tests for bucket resume logic in established sockets

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ae95a774-2218-4ddc-b2e0-d7bac2b731fd@linux.dev>
Date: Tue, 27 May 2025 17:51:37 -0700
From: Martin KaFai Lau <martin.lau@...ux.dev>
To: Jordan Rife <jordan@...fe.io>
Cc: Daniel Borkmann <daniel@...earbox.net>,
 Willem de Bruijn <willemdebruijn.kernel@...il.com>,
 Kuniyuki Iwashima <kuniyu@...zon.com>,
 Alexei Starovoitov <alexei.starovoitov@...il.com>, netdev@...r.kernel.org,
 bpf@...r.kernel.org
Subject: Re: [PATCH v1 bpf-next 10/10] selftests/bpf: Add tests for bucket
 resume logic in established sockets

On 5/20/25 7:50 AM, Jordan Rife wrote:
> +static bool close_and_wait(int fd, struct bpf_link *link)
> +{
> +	static const int us_per_ms = 1000;
> +	__u64 cookie = socket_cookie(fd);
> +	struct iter_out out;
> +	bool exists = true;
> +	int iter_fd, nread;
> +	int waits = 20; /* 2 seconds */
> +
> +	close(fd);
> +
> +	/* Wait for socket to disappear from the ehash table. */
> +	while (waits--) {
> +		iter_fd = bpf_iter_create(bpf_link__fd(link));
> +		if (!ASSERT_OK_FD(iter_fd, "bpf_iter_create"))
> +			return false;
> +
> +		/* Is it still there? */
> +		do {
> +			nread = read(iter_fd, &out, sizeof(out));
> +			if (!ASSERT_GE(nread, 0, "nread")) {
> +				close(iter_fd);
> +				return false;
> +			}
> +			exists = nread && cookie == out.cookie;
> +		} while (!exists && nread);
> +
> +		close(iter_fd);
> +
> +		if (!exists)
> +			break;
> +
> +		usleep(100 * us_per_ms);

Instead of retrying with the bpf_iter_tcp to confirm the sk is gone from the 
ehash table, I think the bpf_sock_destroy() can help here.

> +	}
> +
> +	return !exists;
> +}
> +
>   static int get_seen_count(int fd, struct sock_count counts[], int n)
>   {
>   	__u64 cookie = socket_cookie(fd);
> @@ -241,6 +279,43 @@ static void remove_seen(int family, int sock_type, const char *addr, __u16 port,
>   			       counts_len);
>   }
>   
> +static void remove_seen_established(int family, int sock_type, const char *addr,
> +				    __u16 port, int *listen_socks,
> +				    int listen_socks_len, int *established_socks,
> +				    int established_socks_len,
> +				    struct sock_count *counts, int counts_len,
> +				    struct bpf_link *link, int iter_fd)
> +{
> +	int close_idx;
> +
> +	/* Iterate through all listening sockets. */
> +	read_n(iter_fd, listen_socks_len, counts, counts_len);
> +
> +	/* Make sure we saw all listening sockets exactly once. */
> +	check_n_were_seen_once(listen_socks, listen_socks_len, listen_socks_len,
> +			       counts, counts_len);
> +
> +	/* Leave one established socket. */
> +	read_n(iter_fd, established_socks_len - 1, counts, counts_len);

hmm... In the "SEC("iter/tcp") int iter_tcp_soreuse(...)" bpf prog, there is a 
"sk->sk_state != TCP_LISTEN" check and the established sk should have been 
skipped. Does it have an existing bug? I suspect it is missing a "()" around
"sk->sk_family == AF_INET6 ? !ipv6_addr_loopback(...) : ...".