On Thu, Nov 23, 2023 at 2:48 PM Daniel Borkmann <daniel@xxxxxxxxxxxxx> wrote:
>
> On 11/22/23 10:31 PM, Andrii Nakryiko wrote:
> > See patch #2 for justification. In few words, current eager verification of
> > global func prevents BPF CO-RE approaches to be applied to global functions.
> >
> > Patch #1 is just a nicety to emit global subprog names in verifier logs.
> >
> > Patch #3 adds selftests validating new lazy semantics.
> >
> > Andrii Nakryiko (3):
> > bpf: emit global subprog name in verifier logs
> > bpf: validate global subprogs lazily
> > selftests/bpf: add lazy global subprog validation tests
> >
> > include/linux/bpf.h | 2 +
> > kernel/bpf/verifier.c | 88 ++++++++++++++----
> > .../selftests/bpf/prog_tests/verifier.c | 2 +
> > .../selftests/bpf/progs/test_global_func12.c | 4 +-
> > .../bpf/progs/verifier_global_subprogs.c | 92 +++++++++++++++++++
> > .../bpf/progs/verifier_subprog_precision.c | 4 +-
> > 6 files changed, 168 insertions(+), 24 deletions(-)
> > create mode 100644 tools/testing/selftests/bpf/progs/verifier_global_subprogs.c
>
> Series looks good to me, ACK. It needs a rebase however after the fast
> forward of the bpf-next tree from today.
Sure, no problem.
>
> > With BPF CO-RE approach, the natural way would be to still compile BPF
> > object file once and guard calls to this global subprog with some CO-RE
> > check or using .rodata variables. That's what people do to guard usage
> > of new helpers or kfuncs, and any other new BPF-side feature that might
> > be missing on old kernels.
>
> I was wondering for selftests, could we also add something similar to the
> above to guard calls via co-re? Just to have this use-case covered in CI.
> Also perhaps one global_bad function which is dead-code would be nice to
> have.
>
We already have that in patch #3. See `const volatile bool
skip_unsupp_global = true;` and then
if (!skip_unsupp_global)
return global_unsupp(NULL);
Because skip_unsupp_global is true, we won't call global_unsupp() from
that main program and it will be dead-code eliminated.
This `const volatile` value can actually be set to false before
loading BPF program, though, and in that case global function will be
actually callable.
Keep in mind that each main (entry) BPF program gets its own copies of
each subprogram (global or static, doesn't matter). So global_bad() is
dead-code for guarded_unsupp_global_called main program, but is not a
dead-code for unguarded_unsupp_global_called prog, in which we
unconditionally call global_bad().
So this should cover all the use cases, I think.
> Thanks,
> Daniel
>
