[PATCH] tracing: hist: free the var ref when its initialization fails

[PATCH] tracing: hist: free the var ref when its initialization fails

[Donggeun Yoo]

create_var_ref() allocates a VAR_REF hist_field and then calls
init_var_ref() to fill it in. When that fails the field is leaked.

commit 656fe2ba85e8 ("tracing: Use hist trigger's var_ref array to destroy
var_refs") made destroy_hist_field() return early for
HIST_FIELD_FL_VAR_REF, since var refs are freed by walking the trigger's
var_refs[] array instead. create_var_ref() adds the field to that array
only after init_var_ref() has succeeded, so on this path the field is in
neither place and nothing frees it. The call was correct when it was
written, before var refs were taken out of destroy_hist_field().

init_var_ref() cannot free it either. The caller owns the field, so
init_var_ref() undoes only its own string allocations and leaves the
field alone. Freeing it there would leave create_var_ref() passing freed
memory to destroy_hist_field(), which reads its flags.

Call __destroy_hist_field(), which frees the field without consulting
the flag.

Fixes: 656fe2ba85e8 ("tracing: Use hist trigger's var_ref array to destroy var_refs")
Signed-off-by: Donggeun Yoo <donggeunyoo.kernel@gmail.com>
---
Found by the Sashiko bot while reviewing an unrelated hist trigger patch:
https://lore.kernel.org/linux-trace-kernel/20260906124025.3550596-1-donggeunyoo.kernel@gmail.com/

That patch and this one are independent; this applies with or without it.

init_var_ref() only fails when kstrdup() returns NULL, so to reproduce it I
built a kernel with its last allocation forced to fail, leaving everything
else stock, and installed

  hist:keys=next_pid:delta=common_timestamp-$start

200 times against a sched_waking trigger defining $start. All 200 installs
fail, as intended; the question is what each failure leaves behind. With
CONFIG_DEBUG_KMEMLEAK:

  before  200 unreferenced objects, 38400 bytes
  after     0 unreferenced objects,     0 bytes

38400 is 200 * 192, one struct hist_field per failed call, which also
confirms the strings are not leaked: init_var_ref() frees those itself.
kmemleak points at the allocation in create_hist_field() reached from
create_var_ref().

checkpatch --strict is clean and an x86_64 W=1 build of the file adds no
warnings.

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

diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index 963e0d6b61fd..34831a01bb9b 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -2234,7 +2234,7 @@ static struct hist_field *create_var_ref(struct hist_trigger_data *hist_data,
 	ref_field = create_hist_field(var_field->hist_data, NULL, flags, NULL);
 	if (ref_field) {
 		if (init_var_ref(ref_field, var_field, system, event_name)) {
-			destroy_hist_field(ref_field, 0);
+			__destroy_hist_field(ref_field);
 			return NULL;
 		}
 

base-commit: 1fc5a74b108fc90951890ec513ac81869f5eaff1
-- 
2.53.0

Re: [PATCH] tracing: hist: free the var ref when its initialization fails

[sashiko-bot]

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

Pre-existing issues:
- [High] Pre-existing memory leak in `__destroy_hist_field()` for variable references (`VAR_REF` fields) reused three or more times within a histogram trigger.
--

commit e42abc814ef3aa510a981868dbef3bcabc30250f
Author: Donggeun Yoo <donggeunyoo.kernel@gmail.com>

tracing: hist: free the var ref when its initialization fails

This commit fixes a memory leak in create_var_ref() when init_var_ref()
fails. It modifies the error path to call __destroy_hist_field() directly,
ensuring the VAR_REF hist_field is freed without checking flags.

Link: https://lore.kernel.org/linux-trace-kernel/20260906124025.3550596-1-donggeunyoo.kernel@gmail.com/

> diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
> index 963e0d6b61fdf..34831a01bb9b5 100644
> --- a/kernel/trace/trace_events_hist.c
> +++ b/kernel/trace/trace_events_hist.c
> @@ -2234,7 +2234,7 @@ static struct hist_field *create_var_ref(struct hist_trigger_data *hist_data,
>  	ref_field = create_hist_field(var_field->hist_data, NULL, flags, NULL);
>  	if (ref_field) {
>  		if (init_var_ref(ref_field, var_field, system, event_name)) {
> -			destroy_hist_field(ref_field, 0);
> +			__destroy_hist_field(ref_field);

[Severity: High]
This isn't a bug introduced by this patch, but does the reference counting
logic for VAR_REF fields lead to a permanent memory leak when a variable
is referenced three or more times?

When a tracefs user creates a histogram trigger where a variable is
referenced multiple times (for example:
delta=common_timestamp-$start,delta2=common_timestamp-$start,
delta3=common_timestamp-$start), create_var_ref() increments the reference
count for each reuse:

kernel/trace/trace_events_hist.c:create_var_ref() {
    ...
    for (i = 0; i < hist_data->n_var_refs; i++) {
        ref_field = hist_data->var_refs[i];
        if (ref_field->var.idx == var_field->var.idx &&
            ref_field->var.hist_data == var_field->hist_data) {
            get_hist_field(ref_field);
            return ref_field;
        }
    }
    ...
}

During normal trigger destruction, destroy_hist_fields() visits each unique
variable reference in the var_refs[] array exactly once:

kernel/trace/trace_events_hist.c:destroy_hist_fields() {
    ...
    for (i = 0; i < hist_data->n_var_refs; i++) {
        ...
        __destroy_hist_field(hist_data->var_refs[i]);
    }
    ...
}

Since destroy_hist_fields() only calls __destroy_hist_field() once per
unique var_ref regardless of how many times it was referenced, the
reference count is only decremented by one. If the initial count was 3
or more, the decrement check in __destroy_hist_field() evaluates to
true and returns early without freeing the struct hist_field memory:

kernel/trace/trace_events_hist.c:__destroy_hist_field() {
    if (--hist_field->ref > 1)
        return;
    ...
    kfree(hist_field);
}

Could this allow a user with tracefs access to repeatedly create and destroy
such triggers, leading to memory exhaustion?

>  			return NULL;
>  		}

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

Re: [PATCH] tracing: hist: free the var ref when its initialization fails

[Donggeun Yoo]

Sashiko wrote:
> - [High] Pre-existing memory leak in `__destroy_hist_field()` for variable
> references (`VAR_REF` fields) reused three or more times within a histogram
> trigger.

That is the bug fixed by the patch linked in this one's notes:

  https://lore.kernel.org/linux-trace-kernel/20260906124025.3550596-1-donggeunyoo.kernel@gmail.com/

It removes the reference count so __destroy_hist_field() frees
unconditionally, and the three-reference case no longer leaks. The two
patches are independent and apply in either order.

Re: [PATCH] tracing: hist: free the var ref when its initialization fails

[Donggeun Yoo]

The Sashiko AI reviewer reported this bug before I sent the fix, in its
review of "tracing: hist: free var refs regardless of how often they are
referenced". The patch should carry:

Reported-by: sashiko-bot@kernel.org
Closes: https://lore.kernel.org/linux-trace-kernel/20260906125025.711111F00A3A@smtp.kernel.org/

No change to the patch itself.

Thanks,
Donggeun