Re: [PATCH bpf-next v4 09/10] selftests/bpf: move bpf_jit_harden helper into testing_helpers

["Alexis Lothoré" <alexis.lothore@bootlin.com>]

On Wed Jul 8, 2026 at 10:16 AM CEST, sashiko-bot wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] The refactored `set_bpf_jit_harden` function has been moved to a common helper file and stripped of its internal `ASSERT_FAIL` calls. However, its usage in the cleanup path of `check_blindness()` was not updated to check the return value. Consequently, if restoring the sysctl fails during cleanup, the test will silently leave the system-wide `bpf_jit_harden` configuration in an altered state.
> --
>
> --- Patch [9]: [PATCH bpf-next v4 09/10] selftests/bpf: move bpf_jit_harden helper into testing_helpers ---
> commit 806a4cb4e9c458805fd71ac7ae0f57e00e8af6bc
> Author: Alexis Lothoré (eBPF Foundation) <alexis.lothore@bootlin.com>
>
> selftests/bpf: move bpf_jit_harden helper into testing_helpers
>
> Move set_bpf_jit_harden to testing helpers so that other selftests can
> change the hardening configuration without re-implementing a helper.
>
>> diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_insn_array.c b/tools/testing/selftests/bpf/prog_tests/bpf_insn_array.c
>> index 0222a9a5d0761..37c405fc2b0d2 100644
>> --- a/tools/testing/selftests/bpf/prog_tests/bpf_insn_array.c
>> +++ b/tools/testing/selftests/bpf/prog_tests/bpf_insn_array.c
>> @@ -227,42 +227,6 @@ static void check_incorrect_index(void)
>>  	check_mid_insn_index();
>>  }
>>  
>> -static int set_bpf_jit_harden(char *level)
>> -{
>> -	char old_level;
>> -	int err = -1;
>> -	int fd = -1;
>> -
>> -	fd = open("/proc/sys/net/core/bpf_jit_harden", O_RDWR | O_NONBLOCK);
>> -	if (fd < 0) {
>> -		ASSERT_FAIL("open .../bpf_jit_harden returned %d (errno=%d)", fd, errno);
>> -		return -1;
>> -	}
>
> [Severity: Medium]
> Since this function's internal ASSERT_FAIL() calls were removed when moving it
> to testing_helpers.c, are we missing an error check in the cleanup path of
> check_blindness() where it restores the sysctl?
>
> tools/testing/selftests/bpf/prog_tests/bpf_insn_array.c:check_blindness() {
>     ...
> cleanup:
>     /* restore the old one */
>     if (bpf_jit_harden != '@')
>         set_bpf_jit_harden(&bpf_jit_harden);
>     ...
> }
>
> If restoring the sysctl fails during cleanup, this will silently leave the
> system-wide bpf_jit_harden configuration in an altered state. Should the return
> value of set_bpf_jit_harden() in the cleanup path be checked and asserted?

I guess I can add an extra ASSERT_OK in the cleanup path to at least get
a log if the setting restore fails.

Alexis

-- 
Alexis Lothoré, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com