Re: [PATCH v6 00/36] tracing: fprobe: function_graph: Multi-function graph and fprobe on fgraph

Jiri Olsa
2024-01-29T09:54:23Z

On Sun, Jan 28, 2024 at 04:51:53PM +0900, Masami Hiramatsu wrote:
> On Sat, 27 Jan 2024 19:44:06 +0100
> Jiri Olsa <olsajiri@gmail.com> wrote:
>
> > On Sat, Jan 27, 2024 at 12:14:05AM +0900, Masami Hiramatsu wrote:
> > > On Thu, 25 Jan 2024 15:54:53 +0100
> > > Jiri Olsa <olsajiri@gmail.com> wrote:
> > >
> > > > On Fri, Jan 12, 2024 at 07:10:50PM +0900, Masami Hiramatsu (Google) wrote:
> > > > > Hi,
> > > > >
> > > > > Here is the 6th version of the series to re-implement the fprobe on
> > > > > function-graph tracer. The previous version is;
> > > > >
> > > > > https://lore.kernel.org/all/170290509018.220107.1347127510564358608.stgit@devnote2/
> > > > >
> > > > > This version fixes use-after-unregister bug and arm64 stack unwinding
> > > > > bug [13/36], add an improvement for multiple interrupts during push
> > > > > operation[20/36], keep SAVE_REGS until BPF and fprobe_event using
> > > > > ftrace_regs[26/36], also reorder the patches[30/36][31/36] so that new
> > > > > fprobe can switch to SAVE_ARGS[32/36] safely.
> > > > > This series also temporarily adds a DIRECT_CALLS bugfix[1/36], which
> > > > > should be pushed separatedly as a stable bugfix.
> > > > >
> > > > > There are some TODOs:
> > > > > - Add s390x and loongarch support to fprobe (multiple fgraph).
> > > > > - Fix to get the symbol address from ftrace entry address on arm64.
> > > > > (This should be done in BPF trace event)
> > > > > - Cleanup code, rename some terms(offset/index) and FGRAPH_TYPE_BITMAP
> > > > > part should be merged to FGRAPH_TYPE_ARRAY patch.
> > > >
> > > > hi,
> > > > I'm getting kasan bugs below when running bpf selftests on top of this
> > > > patchset.. I think it's probably the reason I see failures in some bpf
> > > > kprobe_multi/fprobe tests
> > > >
> > > > so far I couldn't find the reason.. still checking ;-)
> > >
> > > Thanks for reporting! Have you built the kernel with debuginfo? In that
> > > case, can you decode the line from the address?
> > >
> > > $ eu-addr2line -fi -e vmlinux ftrace_push_return_trace.isra.0+0x346
> > >
> > > This helps me a lot.
> >
> > I had to recompile/regenerate the fault, it points in here:
> >
> > ffffffff8149b390 <ftrace_push_return_trace.isra.0>:
> > ...
> >
> > current->ret_stack[rindex - 1] = val;
> > ffffffff8149b6b1: 48 8d bd 78 28 00 00 lea 0x2878(%rbp),%rdi
> > ffffffff8149b6b8: e8 63 e4 28 00 call ffffffff81729b20 <__asan_load8>
> > ffffffff8149b6bd: 48 8b 95 78 28 00 00 mov 0x2878(%rbp),%rdx
> > ffffffff8149b6c4: 41 8d 47 ff lea -0x1(%r15),%eax
> > ffffffff8149b6c8: 48 98 cltq
> > ffffffff8149b6ca: 4c 8d 24 c2 lea (%rdx,%rax,8),%r12
> > ffffffff8149b6ce: 4c 89 e7 mov %r12,%rdi
> > ffffffff8149b6d1: e8 ea e4 28 00 call ffffffff81729bc0 <__asan_store8>
> > ---> ffffffff8149b6d6: 49 89 1c 24 mov %rbx,(%r12)
> > current->curr_ret_stack = index = rindex;
> > ffffffff8149b6da: 48 8d bd 6c 28 00 00 lea 0x286c(%rbp),%rdi
> > ffffffff8149b6e1: e8 9a e3 28 00 call ffffffff81729a80 <__asan_store4>
> > ffffffff8149b6e6: 44 89 bd 6c 28 00 00 mov %r15d,0x286c(%rbp)
> > ffffffff8149b6ed: e9 8d fd ff ff jmp ffffffff8149b47f <ftrace_push_return_trace.isra.0+0xef>
> > if (WARN_ON_ONCE(idx <= 0))
> >
>
> Thanks! So this shows that this bug is failed to check the boundary of
> shadow stack while pushing the return trace.
>
> diff --git a/kernel/trace/fgraph.c b/kernel/trace/fgraph.c
> index 0f11f80bdd6c..8e1fcc3f4bda 100644
> --- a/kernel/trace/fgraph.c
> +++ b/kernel/trace/fgraph.c
> @@ -550,7 +550,7 @@ ftrace_push_return_trace(unsigned long ret, unsigned long func,
> smp_rmb();
>
> /* The return trace stack is full */
> - if (current->curr_ret_stack + FGRAPH_RET_INDEX >= SHADOW_STACK_MAX_INDEX) {
> + if (current->curr_ret_stack + FGRAPH_RET_INDEX + 1 >= SHADOW_STACK_MAX_INDEX) {
> atomic_inc(&current->trace_overrun);
> return -EBUSY;
> }
>
> Sorry, I forgot to increment the space for reserved entry...

hum, I'm getting same error even with the change above, same backtrace/line

jirka