Hi Steve, On Tue, 2026-04-07 at 21:05 -0400, Steven Rostedt wrote: > > Tom, > > On Wed, 1 Apr 2026 19:22:23 +0800 > Pengpeng Hou <[email protected]> wrote: > > > hist_field_name() uses a static MAX_FILTER_STR_VAL buffer for fully > > qualified variable-reference names, but it currently appends into that > > buffer with strcat() without rebuilding it first. As a result, repeated > > calls append a new "system.event.field" name onto the previous one, > > which can eventually run past the end of full_name. > > > > Build the name with snprintf() on each call and return NULL if the fully > > qualified name does not fit in MAX_FILTER_STR_VAL. > > > > Fixes: 067fe038e70f ("tracing: Add variable reference handling to hist > > triggers") > > Signed-off-by: Pengpeng Hou <[email protected]> > > --- > > Changes since v1: > > https://lore.kernel.org/all/[email protected]/ > > > > - rebuild full_name on each call instead of falling back to field->name > > - return NULL on overflow as suggested > > - split out the snprintf() length check instead of using an inline if > > > > kernel/trace/trace_events_hist.c | 12 +++++++----- > > 1 file changed, 7 insertions(+), 5 deletions(-) > > > > diff --git a/kernel/trace/trace_events_hist.c > > b/kernel/trace/trace_events_hist.c > > index 73ea180cad55..f9c8a4f078ea 100644 > > --- a/kernel/trace/trace_events_hist.c > > +++ b/kernel/trace/trace_events_hist.c > > @@ -1361,12 +1361,14 @@ static const char *hist_field_name(struct > > hist_field *field, > > field->flags & HIST_FIELD_FL_VAR_REF) { > > if (field->system) { > > static char full_name[MAX_FILTER_STR_VAL]; > > + int len; > > + > > + len = snprintf(full_name, sizeof(full_name), "%s.%s.%s", > > + field->system, field->event_name, > > + field->name); > > + if (len >= sizeof(full_name)) > > + return NULL; > > > > - strcat(full_name, field->system); > > - strcat(full_name, "."); > > - strcat(full_name, field->event_name); > > - strcat(full_name, "."); > > - strcat(full_name, field->name); > > field_name = full_name; > > I wanted to test this but I can't find anything that triggers this path. > How does a field here get its ->system set? >
->system is set when using fully-qualified variable names. For instance: echo 'hist:keys=pid:ts0=common_timestamp.usecs' >> sys/kernel/debug/tracing/events/sched/sched_waking/trigger echo 'hist:keys=pid:ts0=common_timestamp.usecs' >> /sys/kernel/debug/tracing/events/sched/sched_wakeup/trigger echo 'hist:keys=next_pid:lat0=common_timestamp.usecs-sched.sched_waking.$ts0:lat1=common_timestamp.usecs-sched.sched_wakeup.$ts0' >> /sys/kernel/debug/tracing/events/sched/sched_switch/trigger echo 'hist:keys=next_pid:vals=$lat0,$lat1' >> /sys/kernel/debug/tracing/events/sched/sched_switch/trigger Here, the sched_switch trigger would error out if the unqualified $ts0 variables were used instead of the fully-qualified ones because there's no way to distinguish which $ts0 was meant. Tom > If there's no way to hit this path, I much rather remove it than "fix" it. > > -- Steve > > > > } else > > field_name = field->name; >
