On Mon, 7 Apr 2025 15:41:39 -0400
Steven Rostedt <[email protected]> wrote:

> From: Steven Rostedt <[email protected]>
> 
> 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) <[email protected]>

Thanks!

> Cc: [email protected]
> Fixes: 4d38328eb442d ("tracing: Fix synth event printk format for str 
> fields");
> Signed-off-by: Steven Rostedt (Google) <[email protected]>
> ---
>  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) <[email protected]>

Reply via email to