Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] Pre-existing memory leak in `__destroy_hist_field()` for variable references (`VAR_REF` fields) reused three or more times within a histogram trigger. -- commit e42abc814ef3aa510a981868dbef3bcabc30250f Author: Donggeun Yoo <[email protected]> tracing: hist: free the var ref when its initialization fails This commit fixes a memory leak in create_var_ref() when init_var_ref() fails. It modifies the error path to call __destroy_hist_field() directly, ensuring the VAR_REF hist_field is freed without checking flags. Link: 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 963e0d6b61fdf..34831a01bb9b5 100644 > --- a/kernel/trace/trace_events_hist.c > +++ b/kernel/trace/trace_events_hist.c > @@ -2234,7 +2234,7 @@ static struct hist_field *create_var_ref(struct > hist_trigger_data *hist_data, > ref_field = create_hist_field(var_field->hist_data, NULL, flags, NULL); > if (ref_field) { > if (init_var_ref(ref_field, var_field, system, event_name)) { > - destroy_hist_field(ref_field, 0); > + __destroy_hist_field(ref_field); [Severity: High] This isn't a bug introduced by this patch, but does the reference counting logic for VAR_REF fields lead to a permanent memory leak when a variable is referenced three or more times? When a tracefs user creates a histogram trigger where a variable is referenced multiple times (for example: delta=common_timestamp-$start,delta2=common_timestamp-$start, delta3=common_timestamp-$start), create_var_ref() increments the reference count for each reuse: kernel/trace/trace_events_hist.c:create_var_ref() { ... for (i = 0; i < hist_data->n_var_refs; i++) { ref_field = hist_data->var_refs[i]; if (ref_field->var.idx == var_field->var.idx && ref_field->var.hist_data == var_field->hist_data) { get_hist_field(ref_field); return ref_field; } } ... } During normal trigger destruction, destroy_hist_fields() visits each unique variable reference in the var_refs[] array exactly once: kernel/trace/trace_events_hist.c:destroy_hist_fields() { ... for (i = 0; i < hist_data->n_var_refs; i++) { ... __destroy_hist_field(hist_data->var_refs[i]); } ... } Since destroy_hist_fields() only calls __destroy_hist_field() once per unique var_ref regardless of how many times it was referenced, the reference count is only decremented by one. If the initial count was 3 or more, the decrement check in __destroy_hist_field() evaluates to true and returns early without freeing the struct hist_field memory: kernel/trace/trace_events_hist.c:__destroy_hist_field() { if (--hist_field->ref > 1) return; ... kfree(hist_field); } Could this allow a user with tracefs access to repeatedly create and destroy such triggers, leading to memory exhaustion? > return NULL; > } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
