I should have also mentioned. Please, new versions of a patch should always start a new thread. Don't reply to the previous version. That makes it very difficult to find which is the latest version.
On Thu, 23 Apr 2026 23:33:00 +0800 Pengpeng Hou <[email protected]> wrote: > The synthetic field helpers build a prefixed synthetic variable name and > a generated hist command in fixed MAX_FILTER_STR_VAL buffers. The > current code appends those strings with raw strcat(), so long key lists, > field names, or saved filters can run past the end of the staging > buffers. > > Build both strings with seq_buf and propagate -E2BIG if either the > synthetic variable name or the generated command exceeds > MAX_FILTER_STR_VAL. This keeps the existing tracing-side limit while > using the helper intended for bounded command construction. > > Fixes: 02205a6752f2 ("tracing: Add support for 'field variables'") > Signed-off-by: Pengpeng Hou <[email protected]> > --- > Changes since v4: To keep a link to the previous version, use lore links to access it (the message ID of the previous version attached to "https://lore.kernel.org/all/$message-id";) Changes since v4: https://lore.kernel.org/all/[email protected]/ > - add the requested blank lines around seq_buf_str() comments > - add the seq_buf_str() comment for the generated command buffer too > - keep saved_filter scoped next to its point of use > > diff --git a/kernel/trace/trace_events_hist.c > b/kernel/trace/trace_events_hist.c > index 0dbbf6cca9bc..87429567417f 100644 > --- a/kernel/trace/trace_events_hist.c > +++ b/kernel/trace/trace_events_hist.c > @@ -8,6 +8,7 @@ > #include <linux/module.h> > #include <linux/kallsyms.h> > #include <linux/security.h> > +#include <linux/seq_buf.h> > #include <linux/mutex.h> > #include <linux/slab.h> > #include <linux/stacktrace.h> > @@ -2968,14 +2969,24 @@ find_synthetic_field_var(struct hist_trigger_data > *target_hist_data, > char *system, char *event_name, char *field_name) > { > struct hist_field *event_var; > + struct seq_buf s; > char *synthetic_name; > > synthetic_name = kzalloc(MAX_FILTER_STR_VAL, GFP_KERNEL); > if (!synthetic_name) > return ERR_PTR(-ENOMEM); > > - strcpy(synthetic_name, "synthetic_"); > - strcat(synthetic_name, field_name); > + seq_buf_init(&s, synthetic_name, MAX_FILTER_STR_VAL); > + seq_buf_puts(&s, "synthetic_"); > + seq_buf_puts(&s, field_name); Hmm, the above is probably simpler with: seq_buf_printf(&s, "synthetic_%s", field_name); > + > + /* Terminate synthetic_name with a NUL. */ > + seq_buf_str(&s); > + > + if (seq_buf_has_overflowed(&s)) { > + kfree(synthetic_name); > + return ERR_PTR(-E2BIG); > + } > > event_var = find_event_var(target_hist_data, system, event_name, > synthetic_name); > > @@ -3020,7 +3031,7 @@ create_field_var_hist(struct hist_trigger_data > *target_hist_data, > struct trace_event_file *file; > struct hist_field *key_field; > struct hist_field *event_var; > - char *saved_filter; Don't remove this for an anonymous block. > + struct seq_buf s; > char *cmd; > int ret; > > @@ -3065,28 +3076,37 @@ create_field_var_hist(struct hist_trigger_data > *target_hist_data, > return ERR_PTR(-ENOMEM); > } > > + seq_buf_init(&s, cmd, MAX_FILTER_STR_VAL); > + > /* Use the same keys as the compatible histogram */ > - strcat(cmd, "keys="); > + seq_buf_puts(&s, "keys="); > > for_each_hist_key_field(i, hist_data) { > key_field = hist_data->fields[i]; > if (!first) > - strcat(cmd, ","); > - strcat(cmd, key_field->field->name); > + seq_buf_putc(&s, ','); > + seq_buf_puts(&s, key_field->field->name); > first = false; > } > > /* Create the synthetic field variable specification */ > - strcat(cmd, ":synthetic_"); > - strcat(cmd, field_name); > - strcat(cmd, "="); > - strcat(cmd, field_name); > + seq_buf_printf(&s, ":synthetic_%s=%s", field_name, field_name); > > /* Use the same filter as the compatible histogram */ > - saved_filter = find_trigger_filter(hist_data, file); > - if (saved_filter) { > - strcat(cmd, " if "); > - strcat(cmd, saved_filter); > + { We don't add anonymous blocks in the kernel (with the exception of needing to be around #ifdefs). -- Steve > + char *saved_filter = find_trigger_filter(hist_data, file); > + > + if (saved_filter) > + seq_buf_printf(&s, " if %s", saved_filter); > + } > + > + /* Terminate cmd with a NUL. */ > + seq_buf_str(&s); > + > + if (seq_buf_has_overflowed(&s)) { > + kfree(cmd); > + kfree(var_hist); > + return ERR_PTR(-E2BIG); > } > > var_hist->cmd = kstrdup(cmd, GFP_KERNEL);
