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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAADnVQJzCw=qcG+jHBYG0q0SxLPkwghni0wpgV4A4PkpgVbGPw@mail.gmail.com>
Date: Fri, 8 Dec 2023 12:41:03 -0800
From: Alexei Starovoitov <alexei.starovoitov@...il.com>
To: Peter Zijlstra <peterz@...radead.org>
Cc: Jiri Olsa <olsajiri@...il.com>, Song Liu <song@...nel.org>, 
	Song Liu <songliubraving@...a.com>, Paul Walmsley <paul.walmsley@...ive.com>, 
	Palmer Dabbelt <palmer@...belt.com>, Albert Ou <aou@...s.berkeley.edu>, 
	Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>, 
	Dave Hansen <dave.hansen@...ux.intel.com>, X86 ML <x86@...nel.org>, 
	"H. Peter Anvin" <hpa@...or.com>, "David S. Miller" <davem@...emloft.net>, David Ahern <dsahern@...nel.org>, 
	Alexei Starovoitov <ast@...nel.org>, Daniel Borkmann <daniel@...earbox.net>, 
	Andrii Nakryiko <andrii@...nel.org>, Martin KaFai Lau <martin.lau@...ux.dev>, 
	Yonghong Song <yonghong.song@...ux.dev>, John Fastabend <john.fastabend@...il.com>, 
	KP Singh <kpsingh@...nel.org>, Stanislav Fomichev <sdf@...gle.com>, Hao Luo <haoluo@...gle.com>, 
	Arnd Bergmann <arnd@...db.de>, Sami Tolvanen <samitolvanen@...gle.com>, 
	Kees Cook <keescook@...omium.org>, Nathan Chancellor <nathan@...nel.org>, 
	Nick Desaulniers <ndesaulniers@...gle.com>, linux-riscv <linux-riscv@...ts.infradead.org>, 
	LKML <linux-kernel@...r.kernel.org>, Network Development <netdev@...r.kernel.org>, 
	bpf <bpf@...r.kernel.org>, linux-arch <linux-arch@...r.kernel.org>, 
	clang-built-linux <llvm@...ts.linux.dev>, Josh Poimboeuf <jpoimboe@...nel.org>, 
	Joao Moreira <joao@...rdrivepizza.com>, Mark Rutland <mark.rutland@....com>
Subject: Re: [PATCH v2 2/2] x86/cfi,bpf: Fix BPF JIT call

On Fri, Dec 8, 2023 at 12:35 PM Peter Zijlstra <peterz@...radead.org> 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.