Re: [PATCH 6.1.y] tracing/histograms: Reject non-stacktrace fields for synthetic stacktrace fields

[Lee Jones <lee@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