Re: [PATCH bpf-next v7 1/2] bpf: Introduce jit_required flag and refactor kfunc path

[KaFai Wan <kafai.wan@linux.dev>]

On Fri, 2026-07-03 at 10:57 +0800, Tiezhu Yang wrote:
> On 2026/7/2 下午10:58, sashiko-bot@kernel.org wrote:
> > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> > - [Medium] The `bpf_fixup_call_args()` function and `bpf_prog_has_kfunc_call()` macro were not
> > updated to use the new `jit_required` flag, leading to inconsistent interpreter fallback
> > behavior for pruned kfuncs.
> > --
> > 
> > commit 5a3f8861c2b6916fe194f97a407cb7265f545fd0
> > Author: Tiezhu Yang <yangtiezhu@loongson.cn>
> > 
> > bpf: Introduce jit_required flag and refactor kfunc path
> > 
> > Introduce a 'jit_required' bitfield flag at the end of the
> > flags group in struct bpf_prog. This bit tracks whether a
> > program strictly requires the JIT compiler.
> > 
> > > diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
> > > index 649cce41e13f3..5fcd19ccb41a9 100644
> > > --- a/kernel/bpf/core.c
> > > +++ b/kernel/bpf/core.c
> > > @@ -2619,8 +2619,7 @@ struct bpf_prog *__bpf_prog_select_runtime(struct bpf_verifier_env *env,
> > > struct
> > >   	if (fp->bpf_func)
> > >   		goto finalize;
> > >   
> > > -	if (IS_ENABLED(CONFIG_BPF_JIT_ALWAYS_ON) ||
> > > -	    bpf_prog_has_kfunc_call(fp))
> > > +	if (IS_ENABLED(CONFIG_BPF_JIT_ALWAYS_ON) || fp->jit_required)
> > >   		jit_needed = true;
> > >   
> > >   	if (!bpf_prog_select_interpreter(fp))
> > 
> > [Severity: Medium]
> > Since bpf_prog_has_kfunc_call() was replaced here with fp->jit_required,
> > should bpf_fixup_call_args() in kernel/bpf/fixups.c also be updated to use
> > the new flag?
> > 
> > Currently, bpf_fixup_call_args() still calls bpf_prog_has_kfunc_call(). If
> > a program has a pruned kfunc (allocating kfunc_tab but leaving
> > jit_required=0) and JIT falls back to the interpreter, wouldn't
> > bpf_fixup_call_args() incorrectly reject the program with -EINVAL?
> > 
> > kernel/bpf/fixups.c:bpf_fixup_call_args() {
> >      ...
> > 	bool has_kfunc_call = bpf_prog_has_kfunc_call(prog);
> >      ...
> > 	if (has_kfunc_call) {
> > 		verbose(env, "calling kernel functions are not allowed in non-JITed
> > programs\n");
> > 		return -EINVAL;
> > 	}
> >      ...
> > }
> > 
> > Should bpf_prog_has_kfunc_call() in kernel/bpf/verifier.c be updated to
> > check prog->jit_required, or removed entirely in favor of checking the flag
> > directly?
> 
> I think it is wrong to remove bpf_prog_has_kfunc_call() entirely,

I think it is ok to remove bpf_prog_has_kfunc_call() but with some changes. see below.

> because using prog->jit_required directly in bpf_fixup_call_args()
> would cause false positives.
> 
> I tested the following changes:
> 
> ```
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index 4e2b059d71f3..3a2fc194e2bb 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -3171,7 +3171,6 @@ const struct bpf_func_proto 
> *bpf_base_func_proto(enum bpf_func_id func_id,
>                                                   const struct bpf_prog 
> *prog);
>   void bpf_task_storage_free(struct task_struct *task);
>   void bpf_cgrp_storage_free(struct cgroup *cgroup);
> -bool bpf_prog_has_kfunc_call(const struct bpf_prog *prog);
>   const struct btf_func_model *
>   bpf_jit_find_kfunc_model(const struct bpf_prog *prog,
>                           const struct bpf_insn *insn);
> @@ -3510,11 +3509,6 @@ static inline void bpf_task_storage_free(struct 
> task_struct *task)
>   {
>   }
> 
> -static inline bool bpf_prog_has_kfunc_call(const struct bpf_prog *prog)
> -{
> -       return false;
> -}
> -
>   static inline const struct btf_func_model *
>   bpf_jit_find_kfunc_model(const struct bpf_prog *prog,
>                           const struct bpf_insn *insn)
> diff --git a/kernel/bpf/fixups.c b/kernel/bpf/fixups.c
> index 94e0457a0aa3..7fb92b5fa415 100644
> --- a/kernel/bpf/fixups.c
> +++ b/kernel/bpf/fixups.c
> @@ -1378,7 +1378,6 @@ int bpf_fixup_call_args(struct bpf_verifier_env *env)
>   #ifndef CONFIG_BPF_JIT_ALWAYS_ON
>          struct bpf_prog *prog = env->prog;
>          struct bpf_insn *insn = prog->insnsi;
> -       bool has_kfunc_call = bpf_prog_has_kfunc_call(prog);
>          int depth;
>   #endif
>          int i, err = 0;
> @@ -1404,7 +1403,7 @@ int bpf_fixup_call_args(struct bpf_verifier_env *env)
>                          return err;
>          }
>   #ifndef CONFIG_BPF_JIT_ALWAYS_ON
> -       if (has_kfunc_call) {
> +       if (prog->jit_required) {
>                  verbose(env, "calling kernel functions are not allowed 
> in non-JITed programs\n");

we can change the log for general purpose. patch #2 and future case will hit this too.

>                  return -EINVAL;
>          }
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index f496b45b9da4..3dbed3047254 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -2770,11 +2770,6 @@ int bpf_add_kfunc_call(struct bpf_verifier_env 
> *env, u32 func_id, u16 offset)
>          return 0;
>   }
> 
> -bool bpf_prog_has_kfunc_call(const struct bpf_prog *prog)
> -{
> -       return !!prog->aux->kfunc_tab;
> -}
> -
>   static int add_subprog_and_kfunc(struct bpf_verifier_env *env)
>   {
>          struct bpf_subprog_info *subprog = env->subprog_info;
> ```
> 
> Here is the local test log:
> 
> ```
> fedora@linux:~$ cat test_panic.c
>    #include <linux/bpf.h>
>    #include <bpf/bpf_helpers.h>
> 
>    SEC("kprobe/sys_getpid")
>    int test_panic(void *ctx)
>    {
>    	struct task_struct *task;
> 
>    	task = (struct task_struct *)bpf_get_current_task();
>    	if (task)
>    		bpf_printk("Task address: %p\n", task);
> 
>    	return 0;
>    }
> 
>    char LICENSE[] SEC("license") = "GPL";
> fedora@linux:~$ clang -target bpf -O2 -g -c test_panic.c -o test_panic.o
> fedora@linux:~$ sudo sysctl -w net.core.bpf_jit_enable=0
> [sudo] password for fedora:
> net.core.bpf_jit_enable = 0
> fedora@linux:~$ sudo bpftool prog load test_panic.o 
> /sys/fs/bpf/test_panic autoattach
> libbpf: prog 'test_panic': BPF program load failed: Invalid argument
> libbpf: prog 'test_panic': -- BEGIN PROG LOAD LOG --
> calling kernel functions are not allowed in non-JITed programs
> processed 10 insns (limit 1000000) max_states_per_insn 0 total_states 1 
> peak_states 1 mark_read 0
> -- END PROG LOAD LOG --
> libbpf: prog 'test_panic': failed to load: -22
> libbpf: failed to load object 'test_panic.o'
> Error: failed to load object file
> ```
> 
> This change incorrectly rejects programs that only contain normal
> inlined helper calls with -EINVAL.

Was it tested after compiling together with patch #2? seems patch #2 works.

> 
> Instead, I think bpf_prog_has_kfunc_call() should be updated to check
> prog->jit_required, like so:
> 
> ```
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index f496b45b9da4..1f5824c1c691 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -2772,7 +2772,7 @@ int bpf_add_kfunc_call(struct bpf_verifier_env 
> *env, u32 func_id, u16 offset)
> 
>   bool bpf_prog_has_kfunc_call(const struct bpf_prog *prog)
>   {
> -       return !!prog->aux->kfunc_tab;
> +       return prog->jit_required && !!prog->aux->kfunc_tab;

bpf_get_current_task() is a helper, not kfunc. test_panic has no kfunc, prog->aux->kfunc_tab
is null. so we get -ENOTSUPP(-524)

>   }
> 
>   static int add_subprog_and_kfunc(struct bpf_verifier_env *env)
> ```
> 
> Here is the local test log:
> 
> ```
> fedora@linux:~$ cat test_panic.c
>    #include <linux/bpf.h>
>    #include <bpf/bpf_helpers.h>
> 
>    SEC("kprobe/sys_getpid")
>    int test_panic(void *ctx)
>    {
>    	struct task_struct *task;
> 
>    	task = (struct task_struct *)bpf_get_current_task();
>    	if (task)
>    		bpf_printk("Task address: %p\n", task);
> 
>    	return 0;
>    }
> 
>    char LICENSE[] SEC("license") = "GPL";
> fedora@linux:~$ clang -target bpf -O2 -g -c test_panic.c -o test_panic.o
> fedora@linux:~$ sudo sysctl -w net.core.bpf_jit_enable=0
> [sudo] password for fedora:
> net.core.bpf_jit_enable = 0
> fedora@linux:~$ sudo bpftool prog load test_panic.o 
> /sys/fs/bpf/test_panic autoattach
> libbpf: prog 'test_panic': BPF program load failed: unknown error (-524)
> libbpf: prog 'test_panic': -- BEGIN PROG LOAD LOG --
> processed 10 insns (limit 1000000) max_states_per_insn 0 total_states 1 
> peak_states 1 mark_read 0
> -- END PROG LOAD LOG --
> libbpf: prog 'test_panic': failed to load: -524
> libbpf: failed to load object 'test_panic.o'
> Error: failed to load object file
> ```
> 
> This ensures that pruned kfuncs (allocating kfunc_tab but leaving
> jit_required=0) safely return false, at the same time, this check
> perfectly isolates normal inlined helper paths.

pruned kfuncs well handled in patch [1], see details in check_kfunc_call() and fixup_kfunc_call().

[1] https://lore.kernel.org/bpf/20211002011757.311265-3-memxor@gmail.com/
> 
> Let me wait for more review comments and will send v8 next week.
> 
> Thanks,
> Tiezhu
> 

-- 
Thanks,
KaFai