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

[sashiko-bot@kernel.org]

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