linux-kernel - Re: [PATCH 7/7] tracing: add arguments to function 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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20241004184317.39199750@gandalf.local.home>
Date: Fri, 4 Oct 2024 18:43:17 -0400
From: Steven Rostedt <rostedt@...dmis.org>
To: Zheng Yejian <zhengyejian@...weicloud.com>
Cc: Sven Schnelle <svens@...ux.ibm.com>, Masami Hiramatsu
 <mhiramat@...nel.org>, Mark Rutland <mark.rutland@....com>, Mathieu
 Desnoyers <mathieu.desnoyers@...icios.com>, linux-kernel@...r.kernel.org,
 linux-trace-kernel@...r.kernel.org
Subject: Re: [PATCH 7/7] tracing: add arguments to function tracer

On Fri, 6 Sep 2024 11:49:10 +0800
Zheng Yejian <zhengyejian@...weicloud.com> wrote:


> > diff --git a/kernel/trace/trace_entries.h b/kernel/trace/trace_entries.h
> > index c47422b20908..f2021ab52da2 100644
> > --- a/kernel/trace/trace_entries.h
> > +++ b/kernel/trace/trace_entries.h
> > @@ -61,8 +61,11 @@ FTRACE_ENTRY_REG(function, ftrace_entry,
> >   	TRACE_FN,
> >   
> >   	F_STRUCT(
> > -		__field_fn(	unsigned long,	ip		)
> > -		__field_fn(	unsigned long,	parent_ip	)
> > +		__field_fn(	unsigned long,		ip		)
> > +		__field_fn(	unsigned long,		parent_ip	)
> > +#ifdef CONFIG_FUNCTION_TRACE_ARGS
> > +		__field_struct( struct ftrace_regs,	regs		)  
> 
> Only function arguments are printed, they are several registers in ftrace_regs,
> would it be better to store what are needed?
> Although different archs save function arguments in different registers, store
> the entire ftrace_regs are much more simple..

Agreed, and I stated as much in my reply to patch 6.

This too will need two versions of the event. One will be function the
other will be function_args. And it will only record the necessary
arguments not the full ftrace_regs structure, as I plan on making that
structure have "zero size".

> 
> > +#endif
> >   	),
> >   
> >   	F_printk(" %ps <-- %ps",  
> 
> F_printk should also match F_STRUCT, otherwise 'format' info may be incorrect,
> it may confuse data parsing in user tools.

Well, it will just ignore the struct part, as its not listed.

-- Steve

> 
> > diff --git a/kernel/trace/trace_functions.c b/kernel/trace/trace_functions.c
> > index 3b0cea37e029..7ff651a0b45a 100644
> > --- a/kernel/trace/trace_functions.c
> > +++ b/kernel/trace/trace_functions.c
> > @@ -25,6 +25,9 @@ static void
> >   function_trace_call(unsigned long ip, unsigned long parent_ip,
> >   		    struct ftrace_ops *op, struct ftrace_regs *fregs);
> >   static void
> > +function_args_trace_call(unsigned long ip, unsigned long parent_ip,
> > +			 struct ftrace_ops *op, struct ftrace_regs *fregs);