If CONFIG_BPF_JIT_ALWAYS_ON is not set and bpf_jit_enable is 0, there
exist 6 failed tests.
[root@linux bpf]# echo 0 > /proc/sys/net/core/bpf_jit_enable
[root@linux bpf]# echo 0 > /proc/sys/kernel/unprivileged_bpf_disabled
[root@linux bpf]# ./test_verifier | grep FAIL
#106/p inline simple bpf_loop call FAIL
#107/p don't inline bpf_loop call, flags non-zero FAIL
#108/p don't inline bpf_loop call, callback non-constant FAIL
#109/p bpf_loop_inline and a dead func FAIL
#110/p bpf_loop_inline stack locations for loop vars FAIL
#111/p inline bpf_loop call in a big program FAIL
Summary: 768 PASSED, 15 SKIPPED, 6 FAILED
The test log shows that callbacks are not allowed in non-JITed programs,
interpreter doesn't support them yet, thus these tests should be skipped
if jit is disabled, just return -ENOTSUPP instead of -EINVAL for pseudo
calls in fixup_call_args().
With this patch:
[root@linux bpf]# echo 0 > /proc/sys/net/core/bpf_jit_enable
[root@linux bpf]# echo 0 > /proc/sys/kernel/unprivileged_bpf_disabled
[root@linux bpf]# ./test_verifier | grep FAIL
Summary: 768 PASSED, 21 SKIPPED, 0 FAILED
Additionally, as Eduard suggested, return -ENOTSUPP instead of -EINVAL
for the other three places where "non-JITed" is used in error messages
to keep consistent.
Signed-off-by: Tiezhu Yang <[email protected]>
---
v2:
-- rebase on the latest bpf-next tree.
-- return -ENOTSUPP instead of -EINVAL for the other three places
where "non-JITed" is used in error messages to keep consistent.
-- update the patch subject and commit message.
kernel/bpf/verifier.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index d5f4ff1eb235..99558a5186b2 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -8908,7 +8908,7 @@ static int check_map_func_compatibility(struct bpf_verifier_env *env,
goto error;
if (env->subprog_cnt > 1 && !allow_tail_call_in_subprogs(env)) {
verbose(env, "tail_calls are not allowed in non-JITed programs with bpf-to-bpf calls\n");
- return -EINVAL;
+ return -ENOTSUPP;
}
break;
case BPF_FUNC_perf_event_read:
@@ -19069,14 +19069,14 @@ static int fixup_call_args(struct bpf_verifier_env *env)
#ifndef CONFIG_BPF_JIT_ALWAYS_ON
if (has_kfunc_call) {
verbose(env, "calling kernel functions are not allowed in non-JITed programs\n");
- return -EINVAL;
+ return -ENOTSUPP;
}
if (env->subprog_cnt > 1 && env->prog->aux->tail_call_reachable) {
/* When JIT fails the progs with bpf2bpf calls and tail_calls
* have to be rejected, since interpreter doesn't support them yet.
*/
verbose(env, "tail_calls are not allowed in non-JITed programs with bpf-to-bpf calls\n");
- return -EINVAL;
+ return -ENOTSUPP;
}
for (i = 0; i < prog->len; i++, insn++) {
if (bpf_pseudo_func(insn)) {
@@ -19084,7 +19084,7 @@ static int fixup_call_args(struct bpf_verifier_env *env)
* have to be rejected, since interpreter doesn't support them yet.
*/
verbose(env, "callbacks are not allowed in non-JITed programs\n");
- return -EINVAL;
+ return -ENOTSUPP;
}
if (!bpf_pseudo_call(insn))
--
2.42.0
On Thu, Jan 04, 2024 at 09:08:17PM +0800, Tiezhu Yang wrote:
> If CONFIG_BPF_JIT_ALWAYS_ON is not set and bpf_jit_enable is 0, there
> exist 6 failed tests.
>
> [root@linux bpf]# echo 0 > /proc/sys/net/core/bpf_jit_enable
> [root@linux bpf]# echo 0 > /proc/sys/kernel/unprivileged_bpf_disabled
> [root@linux bpf]# ./test_verifier | grep FAIL
> #106/p inline simple bpf_loop call FAIL
> #107/p don't inline bpf_loop call, flags non-zero FAIL
> #108/p don't inline bpf_loop call, callback non-constant FAIL
> #109/p bpf_loop_inline and a dead func FAIL
> #110/p bpf_loop_inline stack locations for loop vars FAIL
> #111/p inline bpf_loop call in a big program FAIL
> Summary: 768 PASSED, 15 SKIPPED, 6 FAILED
>
> The test log shows that callbacks are not allowed in non-JITed programs,
> interpreter doesn't support them yet, thus these tests should be skipped
> if jit is disabled, just return -ENOTSUPP instead of -EINVAL for pseudo
> calls in fixup_call_args().
>
> With this patch:
>
> [root@linux bpf]# echo 0 > /proc/sys/net/core/bpf_jit_enable
> [root@linux bpf]# echo 0 > /proc/sys/kernel/unprivileged_bpf_disabled
> [root@linux bpf]# ./test_verifier | grep FAIL
> Summary: 768 PASSED, 21 SKIPPED, 0 FAILED
>
> Additionally, as Eduard suggested, return -ENOTSUPP instead of -EINVAL
> for the other three places where "non-JITed" is used in error messages
> to keep consistent.
>
> Signed-off-by: Tiezhu Yang <[email protected]>
> ---
>
> v2:
> -- rebase on the latest bpf-next tree.
> -- return -ENOTSUPP instead of -EINVAL for the other three places
> where "non-JITed" is used in error messages to keep consistent.
> -- update the patch subject and commit message.
>
> kernel/bpf/verifier.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index d5f4ff1eb235..99558a5186b2 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -8908,7 +8908,7 @@ static int check_map_func_compatibility(struct bpf_verifier_env *env,
> goto error;
> if (env->subprog_cnt > 1 && !allow_tail_call_in_subprogs(env)) {
> verbose(env, "tail_calls are not allowed in non-JITed programs with bpf-to-bpf calls\n");
> - return -EINVAL;
> + return -ENOTSUPP;
FWIW I agree with John review earlier [1], also there's chance (however small)
we could mess up with some app already checking on that
jirka
[1] https://lore.kernel.org/bpf/[email protected]/
> }
> break;
> case BPF_FUNC_perf_event_read:
> @@ -19069,14 +19069,14 @@ static int fixup_call_args(struct bpf_verifier_env *env)
> #ifndef CONFIG_BPF_JIT_ALWAYS_ON
> if (has_kfunc_call) {
> verbose(env, "calling kernel functions are not allowed in non-JITed programs\n");
> - return -EINVAL;
> + return -ENOTSUPP;
> }
> if (env->subprog_cnt > 1 && env->prog->aux->tail_call_reachable) {
> /* When JIT fails the progs with bpf2bpf calls and tail_calls
> * have to be rejected, since interpreter doesn't support them yet.
> */
> verbose(env, "tail_calls are not allowed in non-JITed programs with bpf-to-bpf calls\n");
> - return -EINVAL;
> + return -ENOTSUPP;
> }
> for (i = 0; i < prog->len; i++, insn++) {
> if (bpf_pseudo_func(insn)) {
> @@ -19084,7 +19084,7 @@ static int fixup_call_args(struct bpf_verifier_env *env)
> * have to be rejected, since interpreter doesn't support them yet.
> */
> verbose(env, "callbacks are not allowed in non-JITed programs\n");
> - return -EINVAL;
> + return -ENOTSUPP;
> }
>
> if (!bpf_pseudo_call(insn))
> --
> 2.42.0
>
>
On 1/8/24 11:05 AM, Jiri Olsa wrote:
> On Thu, Jan 04, 2024 at 09:08:17PM +0800, Tiezhu Yang wrote:
>> If CONFIG_BPF_JIT_ALWAYS_ON is not set and bpf_jit_enable is 0, there
>> exist 6 failed tests.
>>
>> [root@linux bpf]# echo 0 > /proc/sys/net/core/bpf_jit_enable
>> [root@linux bpf]# echo 0 > /proc/sys/kernel/unprivileged_bpf_disabled
>> [root@linux bpf]# ./test_verifier | grep FAIL
>> #106/p inline simple bpf_loop call FAIL
>> #107/p don't inline bpf_loop call, flags non-zero FAIL
>> #108/p don't inline bpf_loop call, callback non-constant FAIL
>> #109/p bpf_loop_inline and a dead func FAIL
>> #110/p bpf_loop_inline stack locations for loop vars FAIL
>> #111/p inline bpf_loop call in a big program FAIL
>> Summary: 768 PASSED, 15 SKIPPED, 6 FAILED
>>
>> The test log shows that callbacks are not allowed in non-JITed programs,
>> interpreter doesn't support them yet, thus these tests should be skipped
>> if jit is disabled, just return -ENOTSUPP instead of -EINVAL for pseudo
>> calls in fixup_call_args().
>>
>> With this patch:
>>
>> [root@linux bpf]# echo 0 > /proc/sys/net/core/bpf_jit_enable
>> [root@linux bpf]# echo 0 > /proc/sys/kernel/unprivileged_bpf_disabled
>> [root@linux bpf]# ./test_verifier | grep FAIL
>> Summary: 768 PASSED, 21 SKIPPED, 0 FAILED
>>
>> Additionally, as Eduard suggested, return -ENOTSUPP instead of -EINVAL
>> for the other three places where "non-JITed" is used in error messages
>> to keep consistent.
>>
>> Signed-off-by: Tiezhu Yang <[email protected]>
>> ---
>>
>> v2:
>> -- rebase on the latest bpf-next tree.
>> -- return -ENOTSUPP instead of -EINVAL for the other three places
>> where "non-JITed" is used in error messages to keep consistent.
>> -- update the patch subject and commit message.
>>
>> kernel/bpf/verifier.c | 8 ++++----
>> 1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> index d5f4ff1eb235..99558a5186b2 100644
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
>> @@ -8908,7 +8908,7 @@ static int check_map_func_compatibility(struct bpf_verifier_env *env,
>> goto error;
>> if (env->subprog_cnt > 1 && !allow_tail_call_in_subprogs(env)) {
>> verbose(env, "tail_calls are not allowed in non-JITed programs with bpf-to-bpf calls\n");
>> - return -EINVAL;
>> + return -ENOTSUPP;
>
> FWIW I agree with John review earlier [1], also there's chance (however small)
> we could mess up with some app already checking on that
+1, the ship on this has sailed unfortunately. Tiezhu, it would be good if you could
update the selftest handling instead.
> jirka
>
> [1] https://lore.kernel.org/bpf/[email protected]/