mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Steven Rostedt <rostedt@kernel.org>
To: linux-kernel@vger.kernel.org
Cc: Masami Hiramatsu <mhiramat@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	stable@vger.kernel.org, sashiko-bot@kernel.org,
	Donggeun Yoo <donggeunyoo.kernel@gmail.com>
Subject: [for-next][PATCH 15/18] tracing: Unregister field variable histograms when the command fails
Date: Fri, 16 Oct 2026 10:31:30 -0400	[thread overview]
Message-ID: <20261016143137.182901087@kernel.org> (raw)
In-Reply-To: <20261016143115.392980228@kernel.org>

From: Donggeun Yoo <donggeunyoo.kernel@gmail.com>

An action that names a field on another event needs that field as a
variable, so create_field_var_hist() registers a second hist trigger on
that event to provide it, and records it in
target_hist_data->field_var_hists[].

If a later part of the same command fails, create_actions() returns an
error and event_hist_trigger_parse() jumps to out_free. The write fails
with -EINVAL and sched_switch gets no trigger, but:

  # echo 'hist:keys=pid:ts0=common_timestamp.usecs' >> \
      events/sched/sched_waking/trigger
  # echo 'my_synth u64 lat; int a; int b' >> synthetic_events
  # echo 'hist:keys=next_pid:wakeup_lat=common_timestamp.usecs-$ts0:\
      onmatch(sched.sched_waking).my_synth($wakeup_lat,prio,nosuchfld)'\
      >> events/sched/sched_switch/trigger
  # cat events/sched/sched_waking/trigger
  hist:keys=pid:vals=hitcount:ts0=common_timestamp.usecs:sort=hitcount:size=2048:clock=global [active]
  hist:keys=pid:vals=hitcount:synthetic_prio=prio:sort=hitcount:size=2048 [active]

The second histogram is the one created for 'prio'. It is still active and
still counting.

out_free reaches destroy_field_var_hists(), which frees the
field_var_hists[] entries, but never unregister_field_var_hists(), which is
what removes the triggers and runs only from event_hist_trigger_free(). The
entry is freed and the trigger is left with nothing tracking it.

Unregister the field variable histograms in out_free as well, and record
each one as soon as its trigger is registered rather than after the lookup
that follows it, so no exit can leave a registered trigger untracked. The
entry then belongs to field_var_hists[] and is released by
destroy_field_var_hists(), so freeing it on that exit would be a double
free.

Cc: stable@vger.kernel.org
Link: https://patch.msgid.link/20260914104036.1768529-1-donggeunyoo.kernel@gmail.com
Fixes: 02205a6752f2 ("tracing: Add support for 'field variables'")
Reported-by: sashiko-bot@kernel.org
Closes: https://lore.kernel.org/all/20260913204302.DB2551F000FF@smtp.kernel.org/
Signed-off-by: Donggeun Yoo <donggeunyoo.kernel@gmail.com>
Assisted-by: Claude:claude-fable-5
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 kernel/trace/trace_events_hist.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index fdd097abb0d6..972804709b14 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -3137,20 +3137,18 @@ create_field_var_hist(struct hist_trigger_data *target_hist_data,
 
 	kfree(cmd);
 
+	n = target_hist_data->n_field_var_hists;
+	target_hist_data->field_var_hists[n] = var_hist;
+	target_hist_data->n_field_var_hists++;
+
 	/* If we can't find the variable, something went wrong */
 	event_var = find_synthetic_field_var(target_hist_data, subsys_name,
 					     event_name, field_name);
 	if (IS_ERR_OR_NULL(event_var)) {
-		kfree(var_hist->cmd);
-		kfree(var_hist);
 		hist_err(tr, HIST_ERR_SYNTH_VAR_NOT_FOUND, errpos(field_name));
 		return ERR_PTR(-EINVAL);
 	}
 
-	n = target_hist_data->n_field_var_hists;
-	target_hist_data->field_var_hists[n] = var_hist;
-	target_hist_data->n_field_var_hists++;
-
 	return event_var;
 }
 
@@ -6983,6 +6981,8 @@ static int event_hist_trigger_parse(struct event_command *cmd_ops,
 
 	trigger_data_free(trigger_data);
 
+	unregister_field_var_hists(hist_data);
+
 	destroy_hist_data(hist_data);
 	goto out;
 }
-- 
2.53.0

  parent reply	other threads:[~2026-10-16 14:31 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-16 14:31 [for-next][PATCH 00/18] tracing: Updates for 7.4 Steven Rostedt
2026-10-16 14:31 ` [for-next][PATCH 01/18] tracing: Add "within" filter for call-stack-based event filtering Steven Rostedt
2026-10-16 14:31 ` [for-next][PATCH 02/18] tracnig: Add proper error message for glob use with "within" filter Steven Rostedt
2026-10-16 14:31 ` [for-next][PATCH 03/18] tracing: Remove duplicate declaration of ftrace_set_clr_event() Steven Rostedt
2026-10-16 14:31 ` [for-next][PATCH 04/18] tracing: Stop modifying the input buffer in ftrace_set_clr_event() Steven Rostedt
2026-10-16 14:31 ` [for-next][PATCH 05/18] tracing: Split the event string parsing logic into a dedicated function Steven Rostedt
2026-10-16 14:31 ` [for-next][PATCH 06/18] tracing: Add a test for ftrace_parse_event_string() Steven Rostedt
2026-10-16 14:31 ` [for-next][PATCH 07/18] tracing: Add a filter argument to __ftrace_set_clr_event() Steven Rostedt
2026-10-16 14:31 ` [for-next][PATCH 08/18] tracing: Parse filter from boot event string Steven Rostedt
2026-10-16 14:31 ` [for-next][PATCH 09/18] ring-buffer: Rename ring_buffer_size() to ring_buffer_capacity() Steven Rostedt
2026-10-16 14:31 ` [for-next][PATCH 10/18] tracing: Rename trace_array::entries to capacity Steven Rostedt
2026-10-16 14:31 ` [for-next][PATCH 11/18] tracing: Rename tracing_entries_fops to tracing_capacity_fops Steven Rostedt
2026-10-16 14:31 ` [for-next][PATCH 12/18] tracing: Print the bucket size as unsigned Steven Rostedt
2026-10-16 14:31 ` [for-next][PATCH 13/18] tracing: Add the bucket size to expr_field_str() Steven Rostedt
2026-10-16 14:31 ` [for-next][PATCH 14/18] tracing: Only report the stacktrace modifier on a real field Steven Rostedt
2026-10-16 14:31 ` Steven Rostedt [this message]
2026-10-16 14:31 ` [for-next][PATCH 16/18] tracing: Clamp the printed end of a histogram bucket at U64_MAX Steven Rostedt
2026-10-16 14:31 ` [for-next][PATCH 17/18] tracing: Fix 64-bit and signed histogram fields on 32-bit kernels Steven Rostedt
2026-10-16 14:31 ` [for-next][PATCH 18/18] tracing: Print a signed histogram key as signed Steven Rostedt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20261016143137.182901087@kernel.org \
    --to=rostedt@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=donggeunyoo.kernel@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=mhiramat@kernel.org \
    --cc=sashiko-bot@kernel.org \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®