Thread (3 messages) flat view 3 messages, 2 authors, 10d ago

Re: [PATCH] tracing: hist: free var refs regardless of how often they are referenced

From: sashiko-bot@kernel.org
Date: 2026-09-06 12:50:28

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 [off-list ref]

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.
quoted hunk ↗ jump to hunk
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
[ ... ]
quoted hunk ↗ jump to hunk
@@ -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/20260906124025.3550596-1-donggeunyoo.kernel@gmail.com?part=1
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help