Re: [PATCH bpf] selftests/bpf: Do not attach kprobe_multi bench to bpf_dispatcher_xdp_func

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Jul 14, 2022 at 09:59:38AM -0700, Stanislav Fomichev wrote:
> On Thu, Jul 14, 2022 at 1:23 AM Jiri Olsa <jolsa@xxxxxxxxxx> wrote:
> >
> > Alexei reported crash by running test_progs -j on system
> > with 32 cpus.
> >
> > It turned out the kprobe_multi bench test that attaches all
> > ftrace-able functions will race with bpf_dispatcher_update,
> > that calls bpf_arch_text_poke on bpf_dispatcher_xdp_func,
> > which is ftrace-able function.
> >
> > Ftrace is not aware of this update so this will cause
> > ftrace_bug with:
> >
> >   WARNING: CPU: 6 PID: 1985 at
> >   arch/x86/kernel/ftrace.c:94 ftrace_verify_code+0x27/0x50
> >   ...
> >   ftrace_replace_code+0xa3/0x170
> >   ftrace_modify_all_code+0xbd/0x150
> >   ftrace_startup_enable+0x3f/0x50
> >   ftrace_startup+0x98/0xf0
> >   register_ftrace_function+0x20/0x60
> >   register_fprobe_ips+0xbb/0xd0
> >   bpf_kprobe_multi_link_attach+0x179/0x430
> >   __sys_bpf+0x18a1/0x2440
> >   ...
> >   ------------[ ftrace bug ]------------
> >   ftrace failed to modify
> >   [<ffffffff818d9380>] bpf_dispatcher_xdp_func+0x0/0x10
> >    actual:   ffffffe9:7b:ffffff9c:77:1e
> >   Setting ftrace call site to call ftrace function
> >
> > It looks like we need some way to way to hide some functions
> > from ftrace, but meanwhile we workaround this by skipping
> > bpf_dispatcher_xdp_func from kprobe_multi bench test.
> 
> Tangential: I've seen the same happen on our internal kernel, I
> thought it's just due to our older ftrace subtree, but now looking at
> the bpf-next tree I'm not sure. Maybe you can clarify for me?
> 
> I think what happens is: we attach a bpf program that uses text_poke
> and enable ftrace graph and ftrace fails with the same
> ftrace_verify_code.
> I see that on the bpf side, we try to play nicely and use text_poke or
> modify_ftrace_direct if the location is ftrace-managed, but I don't
> see something similar on the ftrace side?

so ftrace keeps track of all the ftrace-able locations and assumes
it's the only one that changes them, so when you change some of them
with text_poke ftrace won't see expected value there and will think
it's a bug

> How is it supposed to work? Do we have some way to signal to ftrace
> that we've text_poke'd the location and ftrace shouldn't try to touch
> it?

we discussed to have a way to exclude some functions from ftrace,
starting with bpf_dispatcher_xdp_func, I'll send some rfc to start
discussion

jirka

> 
> 
> 
> > Reported-by: Alexei Starovoitov <ast@xxxxxxxxxx>
> > Signed-off-by: Jiri Olsa <jolsa@xxxxxxxxxx>
> > ---
> >  tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c b/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
> > index 5b93d5d0bd93..8c442051f312 100644
> > --- a/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
> > +++ b/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
> > @@ -364,6 +364,8 @@ static int get_syms(char ***symsp, size_t *cntp)
> >                         continue;
> >                 if (!strncmp(name, "rcu_", 4))
> >                         continue;
> > +               if (!strncmp(name, "bpf_dispatcher_xdp_func", 23))
> > +                       continue;
> >                 if (!strncmp(name, "__ftrace_invalid_address__",
> >                              sizeof("__ftrace_invalid_address__") - 1))
> >                         continue;
> > --
> > 2.35.3
> >



[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux