Re: [PATCH 2/2] tracing: hist: undo the registration when enabling the trigger fails

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Missing cleanup in hist_register_trigger() on error path leads to UAF on named_triggers and memory leaks.
--

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

tracing: hist: undo the registration when enabling the trigger fails

This commit fixes a use-after-free issue when a trigger fails to enable in
hist_trigger_enable(). It ensures that if
trace_event_trigger_enable_disable() fails, hist_unregister_trigger() will
properly undo the registration, preventing the trigger from being freed
while still on the named_triggers list.

Closes: 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 c6c04926bdf0c..1de224a5a2bba 100644
> --- a/kernel/trace/trace_events_hist.c
> +++ b/kernel/trace/trace_events_hist.c
[ ... ]
> @@ -6972,6 +6973,8 @@ static int event_hist_trigger_parse(struct event_command *cmd_ops,
>  	return ret;
>   out_unreg:
>  	event_trigger_unregister(cmd_ops, file, glob+1, trigger_data);
> +	/* The unregister frees trigger_data, skip out_free */
> +	goto out;
>   out_free:

[Severity: High]
This is a pre-existing issue, but missing cleanup in the error path before
this label can lead to a use-after-free on the global named_triggers list.

When hist_register_trigger() fails after successfully executing
data->cmd_ops->init(data), it returns a negative error code without undoing
the initialization. For instance, if tracing_set_clock() fails due to an
invalid clock name:

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() handles this error from
event_trigger_register() by jumping to out_free:

kernel/trace/trace_events_hist.c:event_hist_trigger_parse() {
    ...
	ret = event_trigger_register(cmd_ops, file, glob, trigger_data);
	if (ret < 0)
		goto out_free;
    ...
}

Since trigger_data_free(trigger_data) is called unconditionally under
out_free, it frees the memory while it remains linked in named_triggers.
Could this missing cleanup cause memory corruption or a kernel panic on
subsequent list traversals?

>  	remove_hist_vars(hist_data);
>

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