Re: [PATCH bpf-next v7 5/5] selftests/bpf: Add ksock test for async callback guard

[email protected]
Newsgroups gmane.linux.network,gmane.linux.kernel.bpf
Message-ID <62274a8d6156015b945bc8e7b5b8c2962332b104e3484a5a2d0e0b0c95b531b1@mail.kernel.org>
> diff --git a/tools/testing/selftests/bpf/prog_tests/ksock_wq.c b/tools/testing/selftests/bpf/prog_tests/ksock_wq.c
> new file mode 100644
> index 0000000000000..d6dc20b8f95bb
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/ksock_wq.c
> @@ -0,0 +1,45 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2026 Isovalent */
> +
> +#include <unistd.h>
> +
> +#include "test_progs.h"
> +#include "ksock_wq.skel.h"
> +
> +#define CALLBACK_WAIT_RETRIES 1000
> +#define CALLBACK_WAIT_US 1000
> +
> +void test_ksock_wq(void)

This commit adds a new standalone test file for the workqueue guard check.
The same directory already has prog_tests/ksock.c (from commit a529f8c4d088b
two patches earlier in this series) which is the home of the ksock kfunc
tests. Notably, the immediately preceding commit in this series
(75ef545fdff15) added its negative test as a new function inside the
existing prog_tests/ksock.c rather than creating a new file. A subsystem
pattern flags this as potentially concerning: should test_ksock_wq() be
another function/subtest in prog_tests/ksock.c, consistent with how patch
4 of this same series added its case? The split means the ksock feature's
tests are now across two files with generated test names 'ksock_lsm',
'ksock_lsm_verifier' and 'ksock_wq' rather than grouped together.

> +{
> +	LIBBPF_OPTS(bpf_test_run_opts, opts);
> +	struct ksock_wq *skel;
> +	u32 callback_done;
> +	int err, i;
> +
> +	skel = ksock_wq__open_and_load();
> +	if (!ASSERT_OK_PTR(skel, "ksock_wq open and load"))
> +		return;
> +
> +	err = bpf_prog_test_run_opts(bpf_program__fd(skel->progs.ksock_wq_start),
> +				     &opts);
> +	if (!ASSERT_OK(err, "run ksock_wq_start"))
> +		goto out;
> +	if (!ASSERT_OK(opts.retval, "ksock_wq_start retval"))
> +		goto out;
> +
> +	for (i = 0; i < CALLBACK_WAIT_RETRIES; i++) {
> +		if (__atomic_load_n(&skel->bss->callback_done, __ATOMIC_ACQUIRE))
> +			break;
> +		usleep(CALLBACK_WAIT_US);
> +	}
> +	callback_done = __atomic_load_n(&skel->bss->callback_done,
> +					__ATOMIC_ACQUIRE);
> +	if (!ASSERT_EQ(callback_done, 1, "workqueue callback completed"))
> +		goto out;
> +
> +	ASSERT_EQ(skel->bss->create_err, -EOPNOTSUPP,
> +		  "workqueue create rejected");
> +
> +out:
> +	ksock_wq__destroy(skel);
> +}
> diff --git a/tools/testing/selftests/bpf/progs/ksock_wq.c b/tools/testing/selftests/bpf/progs/ksock_wq.c
> new file mode 100644
> index 0000000000000..16a1873d132ed
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/ksock_wq.c
> @@ -0,0 +1,62 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2026 Isovalent */
> +
> +#include "vmlinux.h"
> +#include <bpf/bpf_helpers.h>
> +#include "bpf_experimental.h"
> +#include "bpf_tracing_net.h"
> +#include "errno.h"
> +#include "ksock_common.h"
> +
> +struct ksock_wq_value {
> +	struct bpf_wq work;
> +};
> +
> +struct {
> +	__uint(type, BPF_MAP_TYPE_ARRAY);
> +	__uint(max_entries, 1);
> +	__type(key, u32);
> +	__type(value, struct ksock_wq_value);
> +} work_map SEC(".maps");
> +
> +int create_err;
> +u32 callback_done;
> +
> +static int ksock_wq_callback(void *map, int *key, void *value)
> +{
> +	struct bpf_ksock_create_opts opts = {
> +		.family = AF_INET,
> +		.type = SOCK_DGRAM,
> +		.protocol = IPPROTO_UDP,
> +	};
> +	struct bpf_ksock *ks;
> +	int err = 0;
> +
> +	ks = bpf_ksock_create(&opts, sizeof(opts), &err);
> +	if (ks)
> +		bpf_ksock_release(ks);
> +	create_err = err;
> +	__sync_fetch_and_add(&callback_done, 1);
> +	return 0;
> +}

[ ... ]

>     selftests/bpf: Add ksock test for async callback guard
>
>     Because the kfuncs are going through LSM hooks, allowing their use via
>     workqueue callbacks would expose the wrong credentials. This test
>     ensures the kfunc are preventing any use from these contexts.

This isn't a bug, but the changelog says the test "ensures the kfunc are
preventing any use from these contexts", which reads as covering the
whole kfunc set. The added program exercises exactly one kfunc from the
workqueue callback, bpf_ksock_create(). The other two kfuncs that carry
the same runtime guard, bpf_ksock_connect() and bpf_ksock_send(), are not
exercised from an async callback anywhere in the tree. Would it be worth
either narrowing the changelog to say the test covers bpf_ksock_create(),
or extending the callback to also call bpf_ksock_send() on a stashed kptr
so all three guarded kfuncs are pinned?


---
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/31694719067
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.