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? If there's no way to hit this path, I much rather remove it than "fix" it. -- Steve > } else > field_name = field->name;
