Re: [PATCH bpf-next 0/5] bpf: Fixes for CONFIG_X86_KERNEL_IBT

[Jiri Olsa <olsajiri@gmail.com>]

On Fri, Jul 29, 2022 at 03:18:54PM -0700, Andrii Nakryiko wrote:
> On Sun, Jul 24, 2022 at 2:21 PM Jiri Olsa <jolsa@kernel.org> wrote:
> >
> > hi,
> > Martynas reported bpf_get_func_ip returning +4 address when
> > CONFIG_X86_KERNEL_IBT option is enabled and I found there are
> > some failing bpf tests when this option is enabled.
> >
> > The CONFIG_X86_KERNEL_IBT option adds endbr instruction at the
> > function entry, so the idea is to 'fix' entry ip for kprobe_multi
> > and trampoline probes, because they are placed on the function
> > entry.
> >
> > For kprobes I only fixed the bpf test program to adjust ip based
> > on CONFIG_X86_KERNEL_IBT option. I'm not sure what the right fix
> > should be in here, because I think user should be aware where the
> 
> user can't be aware of this when using multi-kprobe attach by symbolic
> name of the function. So I think bpf_get_func_ip() at least in that
> case should be compensating for KERNEL_IBT.

sorry I said kprobes, but that does not include kprobe multi link,
I meant what you call general kprobe below

I do the adjustment for kprobe multi version of bpf_get_func_ip,
so that should be fine

> 
> BTW, given in general kprobe can be placed in them middle of the
> function, should bpf_get_func_ip() return zero or something for such
> cases instead of wrong value somewhere in the middle of kprobe? If
> user cares about current IP, they can get it with PT_REGS_IP(ctx),
> right?

true.. we could add flag to 'struct kprobe' to indicate it's placed
on function's entry and check on endbr instruction for IBT config,
and return 0 for anything else

jirka

> > kprobe is placed, on the other hand we move the kprobe address if
> > its placed on top of endbr instruction.
> >
> > v1 changes:
> >   - read previous instruction in kprobe_multi link handler
> >     and adjust entry_ip for CONFIG_X86_KERNEL_IBT option
> >   - split first patch into 2 separate changes
> >   - update changelogs
> >
> > thanks,
> > jirka
> >
> >
> > ---
> > Jiri Olsa (5):
> >       ftrace: Keep the resolved addr in kallsyms_callback
> >       bpf: Adjust kprobe_multi entry_ip for CONFIG_X86_KERNEL_IBT
> >       bpf: Use given function address for trampoline ip arg
> >       selftests/bpf: Disable kprobe attach test with offset for CONFIG_X86_KERNEL_IBT
> >       selftests/bpf: Fix kprobe get_func_ip tests for CONFIG_X86_KERNEL_IBT
> >
> >  arch/x86/net/bpf_jit_comp.c                               |  9 ++++-----
> >  kernel/trace/bpf_trace.c                                  |  4 ++++
> >  kernel/trace/ftrace.c                                     |  3 +--
> >  tools/testing/selftests/bpf/prog_tests/get_func_ip_test.c | 25 ++++++++++++++++++++-----
> >  tools/testing/selftests/bpf/progs/get_func_ip_test.c      |  7 +++++--
> >  tools/testing/selftests/bpf/progs/kprobe_multi.c          |  2 +-
> >  6 files changed, 35 insertions(+), 15 deletions(-)