On Thu, Mar 16, 2023 at 7:14 AM Eduard Zingerman <eddyz87@xxxxxxxxx> wrote:
>
> On Wed, 2023-03-15 at 15:36 -0700, Alexei Starovoitov wrote:
> > From: Alexei Starovoitov <ast@xxxxxxxxxx>
> >
> > Allow ld_imm64 insn with BPF_PSEUDO_BTF_ID to hold the address of kfunc.
> > PTR_MEM is already recognized for NULL-ness by is_branch_taken(),
> > so unresolved kfuncs will be seen as zero.
> > This allows BPF programs to detect at load time whether kfunc is present
> > in the kernel with bpf_kfunc_exists() macro.
> >
> > Signed-off-by: Alexei Starovoitov <ast@xxxxxxxxxx>
> > ---
> > kernel/bpf/verifier.c | 7 +++++--
> > tools/lib/bpf/bpf_helpers.h | 3 +++
> > 2 files changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> > index 60793f793ca6..4e49d34d8cd6 100644
> > --- a/kernel/bpf/verifier.c
> > +++ b/kernel/bpf/verifier.c
> > @@ -15955,8 +15955,8 @@ static int check_pseudo_btf_id(struct bpf_verifier_env *env,
> > goto err_put;
> > }
> >
> > - if (!btf_type_is_var(t)) {
> > - verbose(env, "pseudo btf_id %d in ldimm64 isn't KIND_VAR.\n", id);
> > + if (!btf_type_is_var(t) && !btf_type_is_func(t)) {
> > + verbose(env, "pseudo btf_id %d in ldimm64 isn't KIND_VAR or KIND_FUNC\n", id);
> > err = -EINVAL;
> > goto err_put;
> > }
> > @@ -15990,6 +15990,9 @@ static int check_pseudo_btf_id(struct bpf_verifier_env *env,
> > aux->btf_var.reg_type = PTR_TO_BTF_ID | MEM_PERCPU;
> > aux->btf_var.btf = btf;
> > aux->btf_var.btf_id = type;
> > + } else if (!btf_type_is_func(t)) {
> > + aux->btf_var.reg_type = PTR_TO_MEM | MEM_RDONLY;
> > + aux->btf_var.mem_size = 0;
>
> This if statement has the following conditions in master:
>
> if (percpu) {
> // ...
> } else if (!btf_type_is_struct(t)) {
> // ...
> } else {
> // ...
> }
>
> Conditions `!btf_type_is_func()` and `!btf_type_is_struct()` are
> not mutually exclusive, thus adding `if (!btf_type_is_func())`
> would match certain conditions that were previously matched by struct
> case, wouldn't it? E.g. if type is `BTF_KIND_INT`?
ohh. good catch!
> Although, I was not able to trigger it, as it seems that pahole only
> encodes per-cpu vars in BTF.
right. that's why we don't have selftests for this code
that could have caught my braino.
There were patches to add vars to vmlinux btf, but it didn't materialize yet.
