Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues: - [Critical] Missing rollback of data->ops->init() when tracing_set_clock() fails, leading to a use-after-free in the global named_triggers list. - [Critical] Double-free and use-after-free of `hist_data` and global list corruption of `trigger_data` when a reused named trigger successfully registers but fails to enable. -- --- Patch [1]: [PATCH] tracing/histograms: Fix use-after-free with same-name named triggers --- commit 5aedb1f49e82add364d36e7e38374f9abcb9b2b9 Author: Hui Su <[email protected]> tracing/histograms: Fix use-after-free with same-name named triggers When two hist triggers on different events are registered with the same name=, the second one reuses the first as named_data. Both are added to tr->hist_vars by save_hist_vars() during event_hist_trigger_parse(), but the named reuse is only detected later. This commit fixes a dangling pointer issue by properly removing the hist_data from tr->hist_vars before freeing it. > diff --git a/kernel/trace/trace_events_hist.c > b/kernel/trace/trace_events_hist.c > index fdf784620c283..50e86f81ea32a 100644 > --- a/kernel/trace/trace_events_hist.c > +++ b/kernel/trace/trace_events_hist.c > @@ -6044,8 +6044,10 @@ static int hist_register_trigger(char *glob, > tracing_set_filter_buffering(file->tr, true); > } > [Severity: Critical] This is a pre-existing issue, but I noticed a potential problem in the error handling right above this block when tracing_set_clock() fails. If tracing_set_clock() fails, the code executes goto out, returning without rolling back the data->ops->init() call that added the trigger to the global list. Would this result in event_hist_trigger_parse() executing kfree(trigger_data) via its out_free path while trigger_data->named_list is still linked in the global named_triggers list? Could this lead to a slab-use-after-free during global list traversal? > - if (named_data) > + if (named_data) { > + remove_hist_vars(hist_data); > destroy_hist_data(hist_data); [Severity: Critical] This is another pre-existing issue, but might freeing hist_data here cause a double-free if registration ultimately fails? When reusing a named trigger, hist_data is freed here. However, it appears event_hist_trigger_parse() does not update its local hist_data pointer. If a subsequent operation like hist_trigger_enable() fails, will the out_free path in event_hist_trigger_parse() execute remove_hist_vars() and destroy_hist_data() on the already-freed hist_data pointer? Additionally, it seems kfree(trigger_data) would be called in that same error path without unlinking it from the global named_triggers list. Can this corrupt the global list? > + } > out: > return ret; > } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
