Re: bpf: Question about odd BPF verifier behaviour

[Matt Bobrowski <mattbobrowski@google.com>]

On Mon, Feb 20, 2023 at 10:35:55PM +0000, Matt Bobrowski wrote:
> Hello!
> 
> Whilst in the midst of testing a v5.19 to v6.1 kernel upgrade, we
> happened to notice that one of our sleepable LSM based eBPF programs
> was failing to load on the newer v6.1 kernel. Using the below trivial
> eBPF program as our reproducer:
> 
> #include "vmlinux.h"
> #include <bpf/bpf_helpers.h>
> #include <bpf/bpf_tracing.h>
> 
> char LICENSE[] SEC("license") = "Dual BSD/GPL";
> 
> SEC("lsm.s/bprm_committed_creds")
> int BPF_PROG(dbg, struct linux_binprm *bprm)
> {
> 	char buf[64] = {0};
> 	bpf_ima_file_hash(bprm->file, buf, sizeof(buf));
> 	return 0;
> }
> 
> The verifier emits the following error message when attempting to load
> the above eBPF program:
> 
> -- BEGIN PROG LOAD LOG --
> reg type unsupported for arg#0 function dbg#5
> 0: R1=ctx(off=0,imm=0) R10=fp0
> ; int BPF_PROG(dbg, struct linux_binprm *bprm)
> 0: (79) r1 = *(u64 *)(r1 +0)
> func 'bpf_lsm_bprm_committed_creds' arg0 has btf_id 137293 type STRUCT 'linux_binprm'
> 1: R1_w=ptr_linux_binprm(off=0,imm=0)
> 1: (b7) r2 = 0                        ; R2_w=0
> ; char buf[64] = {0};
> [...]
> ; bpf_ima_file_hash(bprm->file, buf, 64);
> 10: (79) r1 = *(u64 *)(r1 +64)        ; R1_w=ptr_file(off=0,imm=0)
> 11: (bf) r2 = r10                     ; R2_w=fp0 R10=fp0
> ; 
> 12: (07) r2 += -64                    ; R2_w=fp-64
> ; bpf_ima_file_hash(bprm->file, buf, 64);
> 13: (b7) r3 = 64                      ; R3_w=64
> 14: (85) call bpf_ima_file_hash#193
> cannot access ptr member next with moff 0 in struct llist_node with off 0 size 1
> R1 is of type file but file is expected
> processed 15 insns (limit 1000000) max_states_per_insn 0 total_states 0 peak_states 0 mark_read 0
> -- END PROG LOAD LOG --
> 
> What particularly strikes out at me is the following 2 lines returned
> in the error message:
> 
> cannot access ptr member next with moff 0 in struct llist_node with off 0 size 1
> R1 is of type file but file is expected
> 
> In this particular case, the above message suggested to me that
> there's likely multiple struct file definitions that exist within the
> kernel's BTF and that the verifier is possibly getting confused about
> which one it should be using, or perhaps some of the struct file
> definitions included in the kernel's BTF actually differ and hence
> when performing the btf_struct_ids_match() check in check_reg_type()
> [0] the verifier fails with this error message? Could this potentially
> be a problem with the toolchain (Currently, using latest pahole/LLVM
> built from source)?
> 
> Additionally, I also noticed that when we walk the BTF struct
> defintions via btf_struct_walk() from btf_struct_ids_match(), the size
> passed to btf_struct_walk() is explicitly set to 1. Yet, msize used
> throughout btf_struct_walk() can certainly be > 1 when evaluating a
> struct defintions members and hence why we're also tripping over this
> condition [1] in btf_struct_walk(). Don't completely understaed this
> code yet, so I don't know whether this is actually a problem or not.
> 
> Keen to here what your thoughts are on this one.

Note that I'm using the latest pahole [0] and LLVM [1] when building
things here.

Andrii/Arnaldo, do you happen to have any pointers on debugging this
BTF ID redundancy, which I suspect is what's going on here?

[0] https://git.kernel.org/pub/scm/devel/pahole/pahole.git
[1] https://github.com/llvm/llvm-project.git

/M