Re: [PATCH] tracing: Do not add length to print format in synthetic events

[Masami Hiramatsu (Google) <mhiramat@kernel.org>]

On Mon, 7 Apr 2025 15:41:39 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> From: Steven Rostedt <rostedt@goodmis.org>
> 
> The following causes a vsnprintf fault:
> 
>   # echo 's:wake_lat char[] wakee; u64 delta;' >> /sys/kernel/tracing/dynamic_events
>   # echo 'hist:keys=pid:ts=common_timestamp.usecs if !(common_flags & 0x18)' > /sys/kernel/tracing/events/sched/sched_waking/trigger
>   # echo 'hist:keys=next_pid:delta=common_timestamp.usecs-$ts:onmatch(sched.sched_waking).trace(wake_lat,next_comm,$delta)' > /sys/kernel/tracing/events/sched/sched_switch/trigger
> 
> Because the synthetic event's "wakee" field is created as a dynamic string
> (even though the string copied is not). The print format to print the
> dynamic string changed from "%*s" to "%s" because another location
> (__set_synth_event_print_fmt()) exported this to user space, and user
> space did not need that. But it is still used in print_synth_event(), and
> the output looks like:
> 
>           <idle>-0       [001] d..5.   193.428167: wake_lat: wakee=(efault)sshd-sessiondelta=155
>     sshd-session-879     [001] d..5.   193.811080: wake_lat: wakee=(efault)kworker/u34:5delta=58
>           <idle>-0       [002] d..5.   193.811198: wake_lat: wakee=(efault)bashdelta=91
>             bash-880     [002] d..5.   193.811371: wake_lat: wakee=(efault)kworker/u35:2delta=21
>           <idle>-0       [001] d..5.   193.811516: wake_lat: wakee=(efault)sshd-sessiondelta=129
>     sshd-session-879     [001] d..5.   193.967576: wake_lat: wakee=(efault)kworker/u34:5delta=50
> 
> The length isn't needed as the string is always nul terminated. Just print
> the string and not add the length (which was hard coded to the max string
> length anyway).
> 

Good catch!

Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>

Thanks!

> Cc: stable@vger.kernel.org
> Fixes: 4d38328eb442d ("tracing: Fix synth event printk format for str fields");
> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
> ---
>  kernel/trace/trace_events_synth.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/kernel/trace/trace_events_synth.c b/kernel/trace/trace_events_synth.c
> index 969f48742d72..33cfbd4ed76d 100644
> --- a/kernel/trace/trace_events_synth.c
> +++ b/kernel/trace/trace_events_synth.c
> @@ -370,7 +370,6 @@ static enum print_line_t print_synth_event(struct trace_iterator *iter,
>  				union trace_synth_field *data = &entry->fields[n_u64];
>  
>  				trace_seq_printf(s, print_fmt, se->fields[i]->name,
> -						 STR_VAR_LEN_MAX,
>  						 (char *)entry + data->as_dynamic.offset,
>  						 i == se->n_fields - 1 ? "" : " ");
>  				n_u64++;
> -- 
> 2.47.2
> 


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>