On Sun, 29 Mar 2026 11:09:49 +0800
Pengpeng Hou <[email protected]> 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 <[email protected]>
> ---
> 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
