[PATCH] tracing: hist: free the field rejected for a bad modifier

Writing a hist trigger whose value or variable carries a modifier that is
not allowed there leaks the fields that were built for it.

__create_val_field() takes the field from parse_expr() and stores it in
hist_data->fields[] only after the modifier checks have run:

	hist_field = parse_expr(hist_data, file, field_str, flags, var_name,
				&n_subexprs);
	...
	if (hist_field->flags & HIST_FIELD_FL_VAR) {
		if (hist_field->flags & (...))
			goto err;
	} else {
		if (hist_field->flags & (...))
			goto err;
	}

	hist_data->fields[val_idx] = hist_field;

Both checks jump past that store, and the err label returns without
freeing anything. The error unwinds to create_hist_data(), which calls
destroy_hist_data() -> destroy_hist_fields(), and that reaches a field
only by walking fields[]. A field that never got there is unreachable.

commit e0213434fe3e ("tracing: Do not let histogram values have some
modifiers") set ret to -EINVAL and fell through to the store, which left
the field owned by fields[] and freed along with the rest of hist_data.
Splitting the check into a value case and a variable case replaced that
fall-through with a goto that skips it.

With CONFIG_DEBUG_KMEMLEAK, 200 writes of

  # echo 'hist:keys=prev_pid:vals=next_pid.log2' > \
	 events/sched/sched_switch/trigger

each correctly rejected with -EINVAL, leave 332 unreferenced objects
(63744 bytes) reported at create_hist_field(); 200 install and remove
cycles of a valid trigger leave none. A '.log2' field is two
allocations, since create_hist_field() puts the plain field in
operands[0] of the log2 field, and both are reported.

Use destroy_hist_field() rather than __destroy_hist_field() so that
operands[0] is freed as well. It returns early for HIST_FIELD_FL_VAR_REF,
which is what an operand owned by hist_data->var_refs[] needs; the
rejected field itself is never a var ref, because a var ref never carries
a modifier flag.

Fixes: e30fbc618e97 ("tracing/histograms: Allow variables to have some modifiers")
Cc: stable@vger.kernel.org
Signed-off-by: Donggeun Yoo <redacted>
---
Tested under QEMU x86_64 on 1fc5a74b108f, both kernels built from the
same config:

  vals=next_pid.log2, 200 writes   332 objects / 63744 bytes -> 0
  x=next_pid.log2,    200 writes   471 objects / 61544 bytes -> 0
  valid trigger, 200 install/remove  0 -> 0

All 400 writes are still rejected with -EINVAL after the change. A
CONFIG_KASAN build reports nothing on the same runs. tools/testing/
selftests/ftrace trigger tests are unchanged: 32 pass, 3 fail, 2
unresolved before and after, with the failures also present on an
unpatched kernel.

 kernel/trace/trace_events_hist.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index 963e0d6b61fd..b65d79d6e132 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c

@@ -4331,6 +4331,7 @@ static int __create_val_field(struct hist_trigger_data *hist_data,
 	return ret;
  err:
 	hist_err(file->tr, HIST_ERR_BAD_FIELD_MODIFIER, errpos(field_str));
+	destroy_hist_field(hist_field, 0);
 	return -EINVAL;
 }
 

-- 
2.53.0

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Preexisting memory leak of VAR_REF hist_field objects when a variable is referenced 3 or more times.
- [High] Preexisting memory leak of VAR_REF hist_field in create_var_ref error path.
--

commit 899dd39af79bb308de7335dd48b6656e713c7a62
Author: Donggeun Yoo [off-list ref]

tracing: hist: free the field rejected for a bad modifier

Writing a hist trigger whose value or variable carries a modifier that is
not allowed there leaks the fields that were built for it. This patch fixes
the leak by using destroy_hist_field() to free the rejected fields when
a bad modifier is encountered.

[Severity: High]
This is a pre-existing issue, and was not introduced by the patch under review,
but since the commit message mentions that destroy_hist_field() returns
early for HIST_FIELD_FL_VAR_REF, does this behavior cause a memory leak in
the error path of create_var_ref()?

Looking at create_var_ref() in kernel/trace/trace_events_hist.c, if
init_var_ref() fails:

		if (init_var_ref(ref_field, var_field, system, event_name)) {
			destroy_hist_field(ref_field, 0);
			return NULL;
		}

destroy_hist_field() explicitly skips freeing fields with the
HIST_FIELD_FL_VAR_REF flag:

	if (hist_field->flags & HIST_FIELD_FL_VAR_REF)
		return; /* var refs will be destroyed separately */

Since the ref_field failed initialization and hasn't been added to the
global hist_data->var_refs array yet, will this object be permanently
leaked rather than freed during global trigger teardown?

[Severity: High]
This is also a pre-existing issue, and was not introduced by the patch under
review, but is there a permanent memory leak of VAR_REF hist_field objects
when a variable is referenced 3 or more times?

For example, when a trigger is created referencing the same variable
multiple times, or fails during parsing after reaching 3 references:
hist:keys=pid:val1=$x:val2=$x:val3=$x

destroy_hist_fields() calls __destroy_hist_field() exactly once per unique
variable in hist_data->var_refs:

		__destroy_hist_field(hist_data->var_refs[i]);
		hist_data->var_refs[i] = NULL;

However, in __destroy_hist_field(), there is a refcount check:

	if (--hist_field->ref > 1)
		return;

If a variable is used 3 or more times, its initial refcount would be at
least 3. When __destroy_hist_field() decrements it by one, the refcount
remains 2 or higher, causing an early return without freeing the memory.

Since destroy_hist_fields() sets the pointer to NULL immediately after,
could this permanently leak the VAR_REF object because no other code drops
references for these objects?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260907034948.240387-1-donggeunyoo.kernel@gmail.com?part=1

On Mon, Sep 07, 2026 at 04:04:25AM +0000, sashiko-bot@kernel.org wrote:

- [High] Preexisting memory leak of VAR_REF hist_field objects when a
variable is referenced 3 or more times.
- [High] Preexisting memory leak of VAR_REF hist_field in create_var_ref
error path.

Both are real, and both already have fixes on the list:

  tracing: hist: free var refs regardless of how often they are referenced
  https://lore.kernel.org/linux-trace-kernel/20260906124025.3550596-1-donggeunyoo.kernel@gmail.com/

  tracing: hist: free the var ref when its initialization fails
  https://lore.kernel.org/linux-trace-kernel/20260906133352.3815019-1-donggeunyoo.kernel@gmail.com/

The first removes the counter rather than changing the comparison. Its only
decrement site has been a no-op since 656fe2ba85e8 ("tracing: Use hist
trigger's var_ref array to destroy var_refs"), and var_refs[] is the sole
owner of a var ref, so two references leak as well as three.

The second switches that error path to __destroy_hist_field(), which is the
one that does not skip HIST_FIELD_FL_VAR_REF.

Neither overlaps with the patch under review. The field freed here is never
a var ref: reaching the err label requires one of the modifier flags, and
parse_atom() returns var refs before parse_field() can set any.