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

[Jiri Olsa]

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@xxxxxxxxx> 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@xxxxxxxxx> 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