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 <[email protected]> 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/[email protected]/ > 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/[email protected]?part=2
