Re: [PATCH bpf-next v2] bpf: Return -ENOTSUPP if calls are not allowed in non-JITed programs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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 <yangtiezhu@xxxxxxxxxxx>
---

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/6594a4c15a677_11e86208cd@john.notmuch/




[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux