From: Lee Jones <lee@kernel.org>
To: Sasha Levin <sashal@kernel.org>
Cc: stable@vger.kernel.org,
"Masami Hiramatsu (Google)" <mhiramat@kernel.org>,
Steven Rostedt <rostedt@goodmis.org>
Subject: Re: [PATCH 6.1.y] tracing/histograms: Reject non-stacktrace fields for synthetic stacktrace fields
Date: Thu, 22 Oct 2026 08:50:18 +0100 [thread overview]
Message-ID: <20261022075018.GJ1795367@google.com> (raw)
In-Reply-To: <20261021153403.303474-1-sashal@kernel.org>
On Wed, 21 Oct 2026, Sasha Levin wrote:
> From: Lee Jones <lee@kernel.org>
>
> [ Upstream commit a244c241593b5b38a08f0df2cb43c26574efbdcb ]
>
> In check_synth_field(), when a synthetic event field is a stacktrace
> (field->is_stack) and the histogram field is not a "long[" stacktrace
> array, validation falls through to the generic size and signedness
> comparison.
>
> Because parse_synth_field() sets a stacktrace field's type to
> "__data_loc unsigned long[]", its size is sizeof(u64) and
> synth_field_signed() marks it as signed. Consequently, any 8-byte
> signed scalar histogram field passes the fallback check, and its scalar
> value is subsequently dereferenced as a kernel stacktrace pointer in
> get_field_size() and trace_stack().
>
> Furthermore, checking strstr(hist_field->type, "long[") is insufficient
> because tracepoint fields defined with __bitmask() or __rel_bitmask()
> also have type "__data_loc unsigned long[]" or "__rel_loc unsigned
> long[]", whilst create_hist_field() treats them as 4-byte scalar values
> without HIST_FIELD_FL_STACKTRACE.
>
> Propagate HIST_FIELD_FL_STACKTRACE to variable references and field
> variables (whilst excluding variable aliases from per-element
> stacktrace capture), and reject any mismatch between
> HIST_FIELD_FL_STACKTRACE and field->is_stack in check_synth_field().
>
> Cc: stable@vger.kernel.org
> Fixes: 00cf3d672a9d ("tracing: Allow synthetic events to pass around stacktraces")
> Link: https://patch.msgid.link/20261013112449.1006088-1-lee@kernel.org
> Signed-off-by: Lee Jones <lee@kernel.org>
> Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
> [ sashal: Retained string-only buffer conditions because histogram stacktrace-variable support is absent. ]
Nice. Thanks Robo Sasha!
> Signed-off-by: Sasha Levin <sashal@kernel.org>
> ---
> kernel/trace/trace_events_hist.c | 15 +++++++++++----
> 1 file changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
> index 40d9de933fc85..f04037d01bdd7 100644
> --- a/kernel/trace/trace_events_hist.c
> +++ b/kernel/trace/trace_events_hist.c
> @@ -2067,7 +2067,8 @@ static int init_var_ref(struct hist_field *ref_field,
> ref_field->size = var_field->size;
> ref_field->is_signed = var_field->is_signed;
> ref_field->flags |= var_field->flags &
> - (HIST_FIELD_FL_TIMESTAMP | HIST_FIELD_FL_TIMESTAMP_USECS);
> + (HIST_FIELD_FL_TIMESTAMP | HIST_FIELD_FL_TIMESTAMP_USECS |
> + HIST_FIELD_FL_STACKTRACE);
>
> if (system) {
> ref_field->system = kstrdup(system, GFP_KERNEL);
> @@ -3210,6 +3211,7 @@ static struct field_var *create_field_var(struct hist_trigger_data *hist_data,
> ret = PTR_ERR(var);
> goto err;
> }
> + var->flags |= val->flags & HIST_FIELD_FL_STACKTRACE;
>
> field_var = kzalloc(sizeof(struct field_var), GFP_KERNEL);
> if (!field_var) {
> @@ -3837,7 +3839,10 @@ static int check_synth_field(struct synth_event *event,
> && field->is_dynamic)
> return 0;
>
> - if (strstr(hist_field->type, "long[") && field->is_stack)
> + if (!!(hist_field->flags & HIST_FIELD_FL_STACKTRACE) != field->is_stack)
> + return -EINVAL;
> +
> + if (field->is_stack)
> return 0;
>
> if (strcmp(field->type, hist_field->type) != 0) {
> @@ -4360,7 +4365,8 @@ static int create_var_field(struct hist_trigger_data *hist_data,
> if (!ret && hist_data->fields[val_idx]->flags & HIST_FIELD_FL_EXECNAME)
> update_var_execname(hist_data->fields[val_idx]);
>
> - if (!ret && hist_data->fields[val_idx]->flags & HIST_FIELD_FL_STRING)
> + if (!ret && !(hist_data->fields[val_idx]->flags & HIST_FIELD_FL_ALIAS) &&
> + hist_data->fields[val_idx]->flags & HIST_FIELD_FL_STRING)
> hist_data->fields[val_idx]->var_str_idx = hist_data->n_var_str++;
>
> return ret;
> @@ -5093,7 +5099,8 @@ static void hist_trigger_elt_update(struct hist_trigger_data *hist_data,
> if (hist_field->flags & HIST_FIELD_FL_VAR) {
> var_idx = hist_field->var.idx;
>
> - if (hist_field->flags & HIST_FIELD_FL_STRING) {
> + if (!(hist_field->flags & HIST_FIELD_FL_ALIAS) &&
> + hist_field->flags & HIST_FIELD_FL_STRING) {
> unsigned int str_start, var_str_idx, idx;
> char *str, *val_str;
> unsigned int size;
> --
> 2.53.0
>
--
Lee Jones
prev parent reply other threads:[~2026-10-22 7:50 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-10-21 14:42 FAILED: patch "[PATCH] tracing/histograms: Reject non-stacktrace fields for" failed to apply to 6.1-stable tree gregkh
2026-10-21 15:37 ` [PATCH 6.1.y] tracing/histograms: Reject non-stacktrace fields for synthetic stacktrace fields Sasha Levin
2026-10-22 7:50 ` Lee Jones [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20261022075018.GJ1795367@google.com \
--to=lee@kernel.org \
--cc=mhiramat@kernel.org \
--cc=rostedt@goodmis.org \
--cc=sashal@kernel.org \
--cc=stable@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox