Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [Medium] Memory leak of a newly created `VAR_REF` hist_field when `init_var_ref()` fails in `create_var_ref()`. -- commit 7e5deb418198a666953cde5c792cb003b22199c2 Author: Donggeun Yoo <[email protected]> tracing: hist: free var refs regardless of how often they are referenced This commit addresses a memory leak that occurs when the same variable is referenced three or more times in a single hist trigger. It fixes this by dropping the reference count and unconditionally freeing the object when the trigger is removed. > diff --git a/kernel/trace/trace_events_hist.c > b/kernel/trace/trace_events_hist.c > index 963e0d6b61fdf..f90680b33a373 100644 > --- a/kernel/trace/trace_events_hist.c > +++ b/kernel/trace/trace_events_hist.c [ ... ] > @@ -2223,10 +2212,8 @@ static struct hist_field *create_var_ref(struct > hist_trigger_data *hist_data, > 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); > + ref_field->var.hist_data == var_field->hist_data) > return ref_field; > - } > } > /* Sanity check to avoid out-of-bound write on 'hist_data->var_refs' */ > if (hist_data->n_var_refs >= TRACING_MAP_VARS_MAX) [Severity: Medium] This isn't a bug introduced by this patch, but while looking at create_var_ref() in kernel/trace/trace_events_hist.c, does this code leak a newly created hist_field when init_var_ref() fails? The function create_var_ref() allocates ref_field with HIST_FIELD_FL_VAR_REF set. If init_var_ref() fails, the error path calls destroy_hist_field(): if (init_var_ref(ref_field, var_field, system, event_name)) { destroy_hist_field(ref_field, 0); return NULL; } However, destroy_hist_field() has an early return for fields with HIST_FIELD_FL_VAR_REF set, expecting them to be cleaned up via hist_data->var_refs[]: if (hist_field->flags & HIST_FIELD_FL_VAR_REF) return; /* var refs will be destroyed separately */ Since ref_field has not yet been added to the hist_data->var_refs[] array at this point, it is bypassed by destroy_hist_field() and never freed, leading to a memory leak. -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
