linux-kernel - Re: [PATCH bpf-next v2 2/2] selftests/bpf: Add test to cover ktls with bpf_msg_pop

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: <20250611050833.lhyymoung6rpo5zo@gmail.com>
Date: Tue, 10 Jun 2025 22:08:33 -0700
From: John Fastabend <john.fastabend@...il.com>
To: Jiayuan Chen <jiayuan.chen@...ux.dev>
Cc: bpf@...r.kernel.org, Boris Pismenny <borisp@...dia.com>,
	Jakub Kicinski <kuba@...nel.org>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>,
	Simon Horman <horms@...nel.org>,
	Alexei Starovoitov <ast@...nel.org>,
	Daniel Borkmann <daniel@...earbox.net>,
	Andrii Nakryiko <andrii@...nel.org>,
	Martin KaFai Lau <martin.lau@...ux.dev>,
	Eduard Zingerman <eddyz87@...il.com>, Song Liu <song@...nel.org>,
	Yonghong Song <yonghong.song@...ux.dev>,
	KP Singh <kpsingh@...nel.org>, Stanislav Fomichev <sdf@...ichev.me>,
	Hao Luo <haoluo@...gle.com>, Jiri Olsa <jolsa@...nel.org>,
	Mykola Lysenko <mykolal@...com>, Shuah Khan <shuah@...nel.org>,
	Ihor Solodrai <isolodrai@...a.com>, netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-kselftest@...r.kernel.org
Subject: Re: [PATCH bpf-next v2 2/2] selftests/bpf: Add test to cover ktls
 with bpf_msg_pop_data

On 2025-06-09 10:08:53, Jiayuan Chen wrote:
> The selftest can reproduce an issue where using bpf_msg_pop_data() in
> ktls causes errors on the receiving end.
> 
> Signed-off-by: Jiayuan Chen <jiayuan.chen@...ux.dev>
> ---

Reviewed-by: John Fastabend <john.fastabend@...il.com>

>  .../selftests/bpf/prog_tests/sockmap_ktls.c   | 91 +++++++++++++++++++
>  .../selftests/bpf/progs/test_sockmap_ktls.c   |  4 +
>  2 files changed, 95 insertions(+)
> 
> diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_ktls.c b/tools/testing/selftests/bpf/prog_tests/sockmap_ktls.c
> index b6c471da5c28..b87e7f39e15a 100644
> --- a/tools/testing/selftests/bpf/prog_tests/sockmap_ktls.c
> +++ b/tools/testing/selftests/bpf/prog_tests/sockmap_ktls.c
> @@ -314,6 +314,95 @@ static void test_sockmap_ktls_tx_no_buf(int family, int sotype, bool push)
>  	test_sockmap_ktls__destroy(skel);
>  }
>  
> +static void test_sockmap_ktls_tx_pop(int family, int sotype)
> +{
> +	char msg[37] = "0123456789abcdefghijklmnopqrstuvwxyz\0";
> +	int c = 0, p = 0, one = 1, sent, recvd;
> +	struct test_sockmap_ktls *skel;
> +	int prog_fd, map_fd;
> +	char rcv[50] = {0};
> +	int err;
> +	int i, m, r;
> +
> +	skel = test_sockmap_ktls__open_and_load();
> +	if (!ASSERT_TRUE(skel, "open ktls skel"))
> +		return;
> +
> +	err = create_pair(family, sotype, &c, &p);
> +	if (!ASSERT_OK(err, "create_pair()"))
> +		goto out;
> +
> +	prog_fd = bpf_program__fd(skel->progs.prog_sk_policy);
> +	map_fd = bpf_map__fd(skel->maps.sock_map);
> +
> +	err = bpf_prog_attach(prog_fd, map_fd, BPF_SK_MSG_VERDICT, 0);
> +	if (!ASSERT_OK(err, "bpf_prog_attach sk msg"))
> +		goto out;
> +
> +	err = bpf_map_update_elem(map_fd, &one, &c, BPF_NOEXIST);
> +	if (!ASSERT_OK(err, "bpf_map_update_elem(c)"))
> +		goto out;
> +
> +	err = init_ktls_pairs(c, p);
> +	if (!ASSERT_OK(err, "init_ktls_pairs(c, p)"))
> +		goto out;
> +
> +	struct {
> +		int	pop_start;
> +		int	pop_len;
> +	} pop_policy[] = {
> +		/* trim the start */
> +		{0, 2},
> +		{0, 10},
> +		{1, 2},
> +		{1, 10},
> +		/* trim the end */
> +		{35, 2},
> +		/* New entries should be added before this line */
> +		{-1, -1},
> +	};
> +
> +	i = 0;
> +	while (pop_policy[i].pop_start >= 0) {
> +		skel->bss->pop_start = pop_policy[i].pop_start;
> +		skel->bss->pop_end =  pop_policy[i].pop_len;
> +
> +		sent = send(c, msg, sizeof(msg), 0);
> +		if (!ASSERT_EQ(sent, sizeof(msg), "send(msg)"))
> +			goto out;

Its possible this could actually not send 38B (sent < 38), but then again
it is only 38B so I guess it should never fail? Anyways we have this
case in a few places already I think and its not tripping CI so lets go
for it.

Thanks,
John

> +
> +		recvd = recv_timeout(p, rcv, sizeof(rcv), MSG_DONTWAIT, 1);
> +		if (!ASSERT_EQ(recvd, sizeof(msg) - pop_policy[i].pop_len, "pop len mismatch"))
> +			goto out;