linux-kernel - Re: [PATCH v2 2/4] ftrace: Add support for function argument to graph tracer

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20241225170225.0e1ac99e@batman.local.home>
Date: Wed, 25 Dec 2024 17:02:25 -0500
From: Steven Rostedt <rostedt@...dmis.org>
To: "Masami Hiramatsu (Google)" <mhiramat@...nel.org>
Cc: linux-kernel@...r.kernel.org, linux-trace-kernel@...r.kernel.org, Mark
 Rutland <mark.rutland@....com>, Mathieu Desnoyers
 <mathieu.desnoyers@...icios.com>, Andrew Morton
 <akpm@...ux-foundation.org>, Sven Schnelle <svens@...ux.ibm.com>, Paul
 Walmsley <paul.walmsley@...ive.com>, Palmer Dabbelt <palmer@...belt.com>,
 Albert Ou <aou@...s.berkeley.edu>, Guo Ren <guoren@...nel.org>, Donglin
 Peng <dolinux.peng@...il.com>, Zheng Yejian <zhengyejian@...weicloud.com>,
 bpf@...r.kernel.org
Subject: Re: [PATCH v2 2/4] ftrace: Add support for function argument to
 graph tracer

On Thu, 26 Dec 2024 00:41:52 +0900
Masami Hiramatsu (Google) <mhiramat@...nel.org> wrote:
> > diff --git a/kernel/trace/trace_functions_graph.c b/kernel/trace/trace_functions_graph.c
> > index d0e4f412c298..c8eda9bebdf4 100644
> > --- a/kernel/trace/trace_functions_graph.c
> > +++ b/kernel/trace/trace_functions_graph.c
> > @@ -12,6 +12,8 @@
> >  #include <linux/interrupt.h>
> >  #include <linux/slab.h>
> >  #include <linux/fs.h>
> > +#include <linux/btf.h>
> > +#include <linux/bpf.h>  
> 
> Do we need these headers? I think it is wrapped by print_function_args().

Oh, probably not. This is just leftovers from the original patch.

> > @@ -814,7 +853,14 @@ static void print_graph_retval(struct trace_seq *s, struct ftrace_graph_ent_entr
> >  		if (entry->ent.type != TRACE_GRAPH_RETADDR_ENT)
> >  			print_retaddr = false;
> >  
> > -		trace_seq_printf(s, "%ps();", func);
> > +		trace_seq_printf(s, "%ps", func);
> > +
> > +		if (args_size >= FTRACE_REGS_MAX_ARGS * sizeof(long)) {
> > +			print_function_args(s, entry->args, (unsigned long)func);
> > +			trace_seq_putc(s, ';');
> > +		} else
> > +			trace_seq_puts(s, "();");
> > +
> >  		if (print_retval || print_retaddr)
> >  			trace_seq_puts(s, " /*");
> >  		else
> > @@ -836,12 +882,13 @@ static void print_graph_retval(struct trace_seq *s, struct ftrace_graph_ent_entr
> >  	}
> >  
> >  	if (!entry || print_retval || print_retaddr)
> > -		trace_seq_puts(s, " */\n");
> > +		trace_seq_puts(s, " */");  
> 
> Do we need this change?

Hmm, maybe not. It may have been caused by added (and removed) debugging.

Thanks for the review.

-- Steve