Re: [PATCH 1/2] tracing: hist: take the reference before publishing the named trigger

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The patch leaves an identical Use-After-Free and memory leak if trigger registration fails after `.init()` succeeds.
- [Medium] Ignored return value from `save_named_trigger()` causes silent failure and breaks histogram sharing.
--

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

tracing: hist: take the reference before publishing the named trigger

This commit delays the publication of a named trigger to the global list
until after its initialization succeeds. This prevents a failed
initialization from leaving a dangling pointer in the named_triggers list
and fixes a use-after-free bug.

Link: https://lore.kernel.org/linux-trace-kernel/20260907092944.3950E1F00A3D@smtp.kernel.org/

> diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
> index 963e0d6b61fdf..c6c04926bdf0c 100644
> --- a/kernel/trace/trace_events_hist.c
> +++ b/kernel/trace/trace_events_hist.c
> @@ -6383,17 +6383,18 @@ static int event_hist_trigger_named_init(struct event_trigger_data *data)
>  {
>  	int ret;
>  
> -	data->ref++;
> -
> -	save_named_trigger(data->named_data->name, data);
> -
>  	ret = event_hist_trigger_init(data->named_data);
>  	if (ret < 0) {
>  		kfree(data->cmd_ops);
>  		data->cmd_ops = &trigger_hist_cmd;
> +		return ret;
>  	}
>  
> -	return ret;
> +	data->ref++;
> +
> +	save_named_trigger(data->named_data->name, data);

[Severity: Medium]
This is a pre-existing issue, but does ignoring the return value of
save_named_trigger() break histogram sharing?

save_named_trigger() uses kstrdup() to allocate the name string, which can
fail and return -ENOMEM. If this happens, the trigger is not added to the
named_triggers list, but the function continues as if it succeeded.

Could this lead to a state where the trigger exists but cannot be found by
functions like pause_named_trigger()? Does this silently break histogram
sharing semantics (causing subsequent triggers with the same name to create
independent maps instead of sharing them) and group pause/unpause logic?

This same return value is also ignored in event_hist_trigger_init():

kernel/trace/trace_events_hist.c:event_hist_trigger_init() {
    ...
    if (!data->ref && hist_data->attrs->name)
        save_named_trigger(hist_data->attrs->name, data);
    ...
}

> +
> +	return 0;

[Severity: High]
This isn't a bug introduced by this patch, but does this fix leave another
identical use-after-free and memory leak reachable from tracefs?

If trigger registration fails in hist_register_trigger() after this .init()
function succeeds, the error propagates up without undoing it:

kernel/trace/trace_events_hist.c:hist_register_trigger() {
    ...
    ret = tracing_set_clock(file->tr, hist_data->attrs->clock);
    if (ret) {
        hist_err(tr, HIST_ERR_SET_CLOCK_FAIL, errpos(clock));
        goto out;
    }
    ...
}

The caller event_hist_trigger_parse() then catches this error and blindly
frees the trigger data:

kernel/trace/trace_events_hist.c:event_hist_trigger_parse() {
    ...
 out_free:
    remove_hist_vars(hist_data);
    trigger_data_free(trigger_data);
    ...
}

Because trigger_data_free() does not invoke cmd_ops->free(), does this leave
the newly freed trigger linked in the global named_triggers list and leak
data->cmd_ops, leading to the exact same KASAN slab-use-after-free this patch
was intended to fix?

>  }

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