Re: [PATCH 3/5] selftests/sched_ext: Fail loudly when EXIT_KIND is missing from BTF

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] selftests/sched_ext: EXIT_KIND() hidden return bypasses cleanup and leaks spinning threads
--

commit 083f03de0fcd50d9c9ef1d52267ab98872def3d2
Author: Tao Cui <cuitao@kylinos.cn>

selftests/sched_ext: Fail loudly when EXIT_KIND is missing from BTF

The patch updates EXIT_KIND() in the sched_ext selftests to fail explicitly via
__COMPAT_read_enum() when a BTF enum is missing, avoiding silent failures where
SCX_EXIT_NONE is returned. It changes the polling helpers in kick and nohz_tick
to use UEI_EXITED() to retain their original semantics without requiring a BTF
lookup.

> diff --git a/tools/testing/selftests/sched_ext/kick.c b/tools/testing/selftests/sched_ext/kick.c
> index 7f01602c75b29..591c250d475db 100644
> --- a/tools/testing/selftests/sched_ext/kick.c
> +++ b/tools/testing/selftests/sched_ext/kick.c

[ ... ]

> @@ -421,6 +421,7 @@ static enum scx_test_status invalid_one(struct kick_ctx *ctx, u32 scenario)
>  	enum scx_test_status ret = SCX_TEST_FAIL;
>  	int cpu = ctx->target_cpu;
>  	int i;
> +	u64 error_kind;
>  
>  	victim = scx_test_spawn_gated_worker(cpu, false);
>  	if (victim.pid < 0)
> @@ -435,12 +436,13 @@ static enum scx_test_status invalid_one(struct kick_ctx *ctx, u32 scenario)
>  	bpf_program__set_autoload(skel->progs.kick_wait_callback, false);
>  	if (kick__load(skel))
>  		goto out;
> +	error_kind = EXIT_KIND(SCX_EXIT_ERROR);

[Severity: Medium]
Does this code leak the spawned gated worker and skeleton object on BTF lookup
failure?

The new EXIT_KIND() macro embeds SCX_FAIL(), which performs a hidden function
return. If the BTF lookup fails here, invalid_one() returns directly and
bypasses the goto out; cleanup path below.

This leaks the skel object and leaves the victim gated worker spinning
indefinitely in the background, which can corrupt the environment for
subsequent tests in the runner.

This pattern of leaking threads also appears in
tools/testing/selftests/sched_ext/cyclic_kick_wait.c:run() where EXIT_KIND() is
called after background pthreads have been started.

>  	ops_link = bpf_map__attach_struct_ops(skel->maps.kick_ops);
>  	if (!ops_link || !scx_test_start_gated_worker(&victim))
>  		goto out;
>  
>  	for (i = 0; i < WAIT_LOOPS; i++) {
> -		if (skel->data->uei.kind == EXIT_KIND(SCX_EXIT_ERROR)) {
> +		if (UEI_EXITED(skel, uei) == error_kind) {
>  			ret = SCX_TEST_PASS;
>  			break;
>  		}

[ ... ]

> diff --git a/tools/testing/selftests/sched_ext/scx_test.h b/tools/testing/selftests/sched_ext/scx_test.h
> index 90b8d6915bb7e..fe0e6dabe7a48 100644
> --- a/tools/testing/selftests/sched_ext/scx_test.h
> +++ b/tools/testing/selftests/sched_ext/scx_test.h

[ ... ]

> @@ -18,7 +19,21 @@ enum scx_test_status {
>  	SCX_TEST_FAIL,
>  };
>  
> -#define EXIT_KIND(__ent) __COMPAT_ENUM_OR_ZERO("scx_exit_kind", #__ent)
> +/*
> + * EXIT_KIND - Resolve an scx_exit_kind enum value from kernel BTF.
> + *
> + * __COMPAT_ENUM_OR_ZERO() silently maps a missing enum to 0, which for
> + * exit kinds is indistinguishable from SCX_EXIT_NONE and would make
> + * every EXIT_KIND() comparison pass vacuously. Fail the test loudly
> + * instead, mirroring SCX_ECODE_VAL() and SCX_KIND_VAL() below.
> + */
> +#define EXIT_KIND(__ent) ({							\
> +	u64 __val = 0;								\
> +										\
> +	if (!__COMPAT_read_enum("scx_exit_kind", #__ent, &__val))		\
> +		SCX_FAIL("scx_exit_kind.%s not found in kernel BTF", #__ent);	\

[Severity: Medium]
Could this hidden return cause resource leaks?

SCX_FAIL() expands to a direct return SCX_TEST_FAIL;. Because EXIT_KIND() is
used as an expression, this introduces a hidden control flow branch that
silently returns from the caller, bypassing explicit explicit cleanup paths.

> +	__val;									\
> +})
>  
>  struct scx_test {
>  	/**

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261009134104.296891-1-cui.tao@linux.dev?part=3