Re: [PATCH v2 3/6] 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() early return leaks bpf_link and breaks test suite
--

commit dc88ff2863ae13f6a94ae129f38a8ba739ac1964
Author: Tao Cui <cuitao@kylinos.cn>
    
selftests/sched_ext: Fail loudly when EXIT_KIND is missing from BTF

EXIT_KIND() resolved scx_exit_kind values through
__COMPAT_ENUM_OR_ZERO(), which maps a missing enumerator to 0 (==
SCX_EXIT_NONE). struct scx_exit_info carries the enum itself, so the
BTF of any loaded scheduler always has the type, and every enumerator
the tests resolve today exists on all sched_ext kernels: the fallback
got the value right, and the single comparison that could pass
vacuously was the kick.c error poll.

> diff --git a/tools/testing/selftests/sched_ext/scx_test.h b/tools/testing/selftests/sched_ext/scx_test.h
> index 90b8d6915bb7e..319e67f19b6dd 100644
> --- a/tools/testing/selftests/sched_ext/scx_test.h
> +++ b/tools/testing/selftests/sched_ext/scx_test.h
[ ... ]
> @@ -119,13 +117,25 @@ void scx_test_register(struct scx_test *test);
>  	(s64)__val;								\
>  })
>  
> +/*
> + * SCX_KIND_VAL - Resolve an scx_exit_kind enumerator from kernel BTF.
> + *
> + * Unlike __COMPAT_ENUM_OR_ZERO(), a missing enumerator fails the test
> + * loudly instead of silently resolving to 0 (== SCX_EXIT_NONE), which
> + * would make the comparison vacuous.
> + */
>  #define SCX_KIND_VAL(__kind) ({							\
> -        u64 __val = 0;								\
> -	bool __found = false;							\
> +	u64 __val = 0;								\
>  										\
> -	__found = __COMPAT_read_enum("scx_exit_kind", #__kind, &__val);		\
> -	SCX_ASSERT(__found);							\
> +	if (!__COMPAT_read_enum("scx_exit_kind", #__kind, &__val))		\
> +		SCX_FAIL("scx_exit_kind.%s not found in kernel BTF", #__kind);	\
>  	__val;									\
>  })
>  
> +/*
> + * EXIT_KIND - Alias of SCX_KIND_VAL() for resolving scx_exit_kind
> + * enumerator values, kept for readability at the EXIT_KIND() call sites.
> + */
> +#define EXIT_KIND(__ent) SCX_KIND_VAL(__ent)
> +
>  #endif  // # __SCX_TEST_H__

[Severity: Medium]
When EXIT_KIND() fails to resolve the enum, SCX_FAIL() triggers a hidden
return SCX_TEST_FAIL. Does this early return leak resources and break the
test suite?

For example, in tools/testing/selftests/sched_ext/exit.c:run():

		link = bpf_map__attach_struct_ops(skel->maps.exit_ops);
		if (!link) {
			SCX_ERR("Failed to attach scheduler");
			exit__destroy(skel);
			return SCX_TEST_FAIL;
		}

		/* Assumes uei.kind is written last */
		while (skel->data->uei.kind == EXIT_KIND(SCX_EXIT_NONE))
			sched_yield();

		SCX_EQ(skel->data->uei.kind, EXIT_KIND(SCX_EXIT_UNREG_BPF));
		SCX_EQ(skel->data->uei.exit_code, tc);
		sprintf(buf, "%d", tc);
		SCX_ASSERT(!strcmp(skel->data->uei.msg, buf));
		bpf_link__destroy(link);
		exit__destroy(skel);

If EXIT_KIND() fails inside the SCX_EQ() assertion or the while loop, it
returns early and bypasses bpf_link__destroy() and exit__destroy(), leaking
both the BPF link and skeleton. 

Since runner.c executes tests serially in a single process, could the open
BPF link keep the scheduler active in the kernel, causing all subsequent tests
to fail with -EBUSY?

This issue also appears to affect multiple other tests, such as
ddsp_bogus_dsq_fail.c, prog_run.c, and enq_select_cpu.c, where EXIT_KIND() is
used inside assertions while a struct bpf_link *link is held. While follow-up
patches in this series refactor loops to use SCX_TIMED_WAIT_UEI, they do not
seem to resolve these specific assertion leaks.

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