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

On Thu, Aug 13, 2026 at 12:13:31PM +0000, [email protected] wrote:
> > 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.

Yes indeed this one was added in a separate file, not sure it's
important.

> 
> > +{
> > +	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?

Yeah, eventually they all call bpf_ksock_has_user_task_context() so the
runtime guard tested is the same.

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