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

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 [off-list ref]

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/ (local)

[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