Re: [PATCH 1/2] tracing/hist: bound full field-name construction

Steven Rostedt
2026-03-29T18:39:04Z

On Sun, 29 Mar 2026 11:09:49 +0800
Pengpeng Hou <pengpeng@iscas.ac.cn> wrote:

> hist_field_name() builds a fully qualified synthetic field name in a
> fixed MAX_FILTER_STR_VAL buffer using repeated strcat() calls. Long
> system, event, and field names can therefore overflow the static staging
> buffer.
>
> Build the qualified name with snprintf() and fall back to the plain
> field name if it does not fit.

The fallback breaks
>
> Fixes: 067fe038e70f ("tracing: Add variable reference handling to hist triggers")

Do you have any examples where it actually does overflow or is this just theoretical?

If it's just theoretical, it does not get a "Fixes" tag.

Hmm, but actually I don't see it resetting full_name to a '\0' so I can see
this concatenating on top of a previous value. THAT would need fixing and
require a fixes tag. An example of triggering the overflow would also be
required.

> Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
> ---
> kernel/trace/trace_events_hist.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
> index 73ea180cad55..4a27da628a71 100644
> --- a/kernel/trace/trace_events_hist.c
> +++ b/kernel/trace/trace_events_hist.c
> @@ -1362,12 +1362,12 @@ static const char *hist_field_name(struct hist_field *field,
> if (field->system) {
> static char full_name[MAX_FILTER_STR_VAL];
>
> - 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;
> + if (snprintf(full_name, sizeof(full_name), "%s.%s.%s",
> + field->system, field->event_name,
> + field->name) < sizeof(full_name))

Ug, please do not use a horribly looking if conditional. And it should most
definitely error on overflow: Break it up:

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 >= size(full_name))
return NULL;

field_name = full_name;

> + field_name = full_name;
> + else
> + field_name = field->name;
> } else
> field_name = field->name;
> } else if (field->flags & HIST_FIELD_FL_TIMESTAMP)

-- Steve