Re: [PATCH v2 2/2] x86/cfi,bpf: Fix BPF JIT call

[Alexei Starovoitov]

On Fri, Dec 8, 2023 at 12:35 PM Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote:
>
> On Fri, Dec 08, 2023 at 11:40:27AM -0800, Alexei Starovoitov wrote:
>
> > typedef void (*btf_dtor_kfunc_t)(void *);
> >         btf_dtor_kfunc_t dtor;
> > but the bpf_cgroup_release takes 'struct cgroup*'.
> > From kcfi pov void * == struct cgroup * ?
> > Do we need to change it to 'void *cgrp' ?
>
> Yes, doing that naively like the below, gets me lovely things like:
>
> validate_case:FAIL:expect_msg unexpected error: -22
> VERIFIER LOG:
> =============
> =============
> EXPECTED MSG: 'Possibly NULL pointer passed to trusted arg0'
> #48/7    cgrp_kfunc/cgrp_kfunc_acquire_untrusted:FAIL
> run_subtest:PASS:obj_open_mem 0 nsec
> libbpf: extern (func ksym) 'bpf_cgroup_release': func_proto [148] incompatible with vmlinux [125610]
> libbpf: failed to load object 'cgrp_kfunc_failure'
>
>
> But let me try rebuilding everything..
>
>
> ---
> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> index b3be5742d6f1..078b207af7f0 100644
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c
> @@ -2145,10 +2145,11 @@ __bpf_kfunc struct task_struct *bpf_task_acquire(struct task_struct *p)
>   * bpf_task_release - Release the reference acquired on a task.
>   * @p: The task on which a reference is being released.
>   */
> -__bpf_kfunc void bpf_task_release(struct task_struct *p)
> +__bpf_kfunc void bpf_task_release(void *p)

Yeah. That won't work. We need a wrapper.
Since bpf prog is also calling it directly.
In progs/task_kfunc_common.h
void bpf_task_release(struct task_struct *p) __ksym;

than later both libbpf and the verifier check that
what bpf prog is calling actually matches the proto
of what is in the kernel.
Effectively we're doing strong prototype check at load time.

btw instead of EXPORT_SYMBOL_GPL(bpf_task_release)
can __ADDRESSABLE be used ?
Since it's not an export symbol.