Re: [PATCH bpf-next 5/7] selftests/bpf: Test local IRQ kfunc inlining

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] selftests/bpf: parallel race on bpf_jit_harden in test_irq_jit()
- [Medium] selftests/bpf: missing is_jit_enabled() skip check in test_irq_jit()
--

commit c1826dafea259f7db660c28b5f90f57c88d53332
Author: Emil Tsalapatis <emil@etsalapatis.com>

selftests/bpf: Test local IRQ kfunc inlining

This commit adds BPF selftests for local IRQ save and restore kfunc inlining,
checking that userspace cannot submit the kernel-private opcodes directly and
verifying the translated and JITed instructions with bpf_jit_harden enabled.

> diff --git a/tools/testing/selftests/bpf/prog_tests/irq_jit.c b/tools/testing/selftests/bpf/prog_tests/irq_jit.c
> new file mode 100644
> index 0000000000000..3d5dc3c77edbc
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/irq_jit.c

[ ... ]

> +void test_irq_jit(void)
> +{
> +	struct irq_jit *skel;
> +	const char *harden = "/proc/sys/net/core/bpf_jit_harden";
> +	char old_harden[32];
> +	int err;

[Severity: Medium]
Should test_irq_jit() be declared as serial_test_irq_jit() instead?

Because /proc/sys/net/core/bpf_jit_harden modifies the global bpf_jit_harden
sysctl rather than per-netns state, running test_irq_jit() in parallel mode
(test_progs -j) enables constant blinding and disables BPF kallsyms globally
while other worker processes are running.

Concurrent tests that assert on unblinded xlated instruction offsets (such as
check_one_to_one_mapping(), check_simple(), and check_deletions() in
tools/testing/selftests/bpf/prog_tests/bpf_insn_array.c) or look up JITed BPF
symbols in /proc/kallsyms (such as test_stacktrace_ips) can fail spuriously
while bpf_jit_harden is set to 2.

In addition, test_irq_jit() races with other parallel tests that read and
write bpf_jit_harden (such as toggle_jit_harden() in
tools/testing/selftests/bpf/prog_tests/subprogs.c and check_blindness() in
tools/testing/selftests/bpf/prog_tests/bpf_insn_array.c). A concurrent writer
can clear bpf_jit_harden before irq_jit__open_and_load() runs, or cause
old_harden to sample and restore "2", leaving hardening permanently enabled
for subsequent tests like serial_test_btf_rust().

Other tests that set /proc/sys/net/core/bpf_jit_harden to "2" via
sysctl_set_or_fail(), such as serial_test_arena_scalar_blinded(),
serial_test_jit_insns_blinded(), and serial_test_kasan(), use the
serial_test_ prefix to avoid this race.

> +
> +	if (sysctl_set_or_fail(harden, old_harden, "2"))
> +		return;
> +	skel = irq_jit__open_and_load();
> +	if (!ASSERT_OK_PTR(skel, "irq_jit_open_and_load"))
> +		goto restore_harden;

[Severity: Medium]
What happens in test_irq_jit() when the kernel is built without
CONFIG_BPF_JIT or when the BPF JIT is disabled at runtime
(/proc/sys/net/core/bpf_jit_enable == 0)?

When CONFIG_BPF_JIT is not enabled, /proc/sys/net/core/bpf_jit_harden does
not exist and sysctl_set_or_fail() fails the test with ENOENT. When the JIT
is compiled in but disabled at runtime, add_kfunc_call() in
kernel/bpf/verifier.c rejects the kfunc calls with -ENOTSUPP:

    if (!env->prog->jit_requested) {
        verbose(env, "JIT is required for calling kernel function\n");
        return -ENOTSUPP;
    }

causing irq_jit__open_and_load() to fail the test instead of skipping.

Could test_irq_jit() check is_jit_enabled() and call test__skip() before
touching bpf_jit_harden, similar to serial_test_arena_scalar_blinded() in
tools/testing/selftests/bpf/prog_tests/arena_scalar_blinded.c:

    if (!is_jit_enabled()) {
        test__skip();
        return;
    }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261027162820.10887-1-emil@etsalapatis.com?part=5