All of lore.kernel.org
 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,
	Donggeun Yoo <donggeunyoo.kernel@gmail.com>
Subject: [for-linus][PATCH 07/20] tracing: Free histogram the field rejected for a bad modifier
Date: Fri, 11 Sep 2026 14:16:43 -0400	[thread overview]
Message-ID: <20260911181729.623284631@kernel.org> (raw)
In-Reply-To: 20260911181636.485043797@kernel.org

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

Writing a hist trigger whose value or variable carries a modifier that is
not allowed there leaks the fields that were built for it.

__create_val_field() takes the field from parse_expr() and stores it in
hist_data->fields[] only after the modifier checks have run:

	hist_field = parse_expr(hist_data, file, field_str, flags, var_name,
				&n_subexprs);
	...
	if (hist_field->flags & HIST_FIELD_FL_VAR) {
		if (hist_field->flags & (...))
			goto err;
	} else {
		if (hist_field->flags & (...))
			goto err;
	}

	hist_data->fields[val_idx] = hist_field;

Both checks jump past that store, and the err label returns without
freeing anything. The error unwinds to create_hist_data(), which calls
destroy_hist_data() -> destroy_hist_fields(), and that reaches a field
only by walking fields[]. A field that never got there is unreachable.

commit e0213434fe3e ("tracing: Do not let histogram values have some
modifiers") set ret to -EINVAL and fell through to the store, which left
the field owned by fields[] and freed along with the rest of hist_data.
Splitting the check into a value case and a variable case replaced that
fall-through with a goto that skips it.

With CONFIG_DEBUG_KMEMLEAK, 200 writes of

  # echo 'hist:keys=prev_pid:vals=next_pid.log2' > \
	 events/sched/sched_switch/trigger

each correctly rejected with -EINVAL, leave 332 unreferenced objects
(63744 bytes) reported at create_hist_field(); 200 install and remove
cycles of a valid trigger leave none. A '.log2' field is two
allocations, since create_hist_field() puts the plain field in
operands[0] of the log2 field, and both are reported.

Use destroy_hist_field() rather than __destroy_hist_field() so that
operands[0] is freed as well. It returns early for HIST_FIELD_FL_VAR_REF,
which is what an operand owned by hist_data->var_refs[] needs; the
rejected field itself is never a var ref, because a var ref never carries
a modifier flag.

Cc: stable@vger.kernel.org
Fixes: e30fbc618e97 ("tracing/histograms: Allow variables to have some modifiers")
Link: https://patch.msgid.link/20260907034948.240387-1-donggeunyoo.kernel@gmail.com
Signed-off-by: Donggeun Yoo <donggeunyoo.kernel@gmail.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 kernel/trace/trace_events_hist.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index bbdd56208eff..8cad99a8d01e 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -4317,6 +4317,7 @@ static int __create_val_field(struct hist_trigger_data *hist_data,
 	return ret;
  err:
 	hist_err(file->tr, HIST_ERR_BAD_FIELD_MODIFIER, errpos(field_str));
+	destroy_hist_field(hist_field, 0);
 	return -EINVAL;
 }
 
-- 
2.53.0

  parent reply	other threads:[~2026-09-11 18:16 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-11 18:16 [for-linus][PATCH 00/20] tracing: Fixes for v7.3 Steven Rostedt
2026-09-11 18:16 ` [for-linus][PATCH 01/20] tracing/user_events: Dont destroy fields when event removal fails Steven Rostedt
2026-09-11 18:16 ` [for-linus][PATCH 02/20] ftrace: fork: Initialize function graph state before copy_exec_state() Steven Rostedt
2026-09-11 18:16 ` [for-linus][PATCH 03/20] fgraph: Remove unused FGRAPH_MAX_INDEX Steven Rostedt
2026-09-11 18:16 ` [for-linus][PATCH 04/20] function_graph: Use the saved entrys size when reprinting it Steven Rostedt
2026-09-11 18:16 ` [for-linus][PATCH 05/20] tracing: Free histogram var refs regardless of how often they are referenced Steven Rostedt
2026-09-11 18:16 ` [for-linus][PATCH 06/20] tracing: Free histogram the var ref when its initialization fails Steven Rostedt
2026-09-11 18:16 ` Steven Rostedt [this message]
2026-09-11 18:16 ` [for-linus][PATCH 08/20] tracing: Keep the entry count when the histogram stats allocation fails Steven Rostedt
2026-09-11 18:16 ` [for-linus][PATCH 09/20] tracing: Let histogram values keep the percent and graph modifiers Steven Rostedt
2026-09-11 18:16 ` [for-linus][PATCH 10/20] tracing: Fix typo "availabe" in comment Steven Rostedt
2026-09-11 18:16 ` [for-linus][PATCH 11/20] tracing: Fix typo "preceeded" " Steven Rostedt
2026-09-11 18:16 ` [for-linus][PATCH 12/20] tracing: Set the trace clock before registering the histogram trigger Steven Rostedt
2026-09-11 18:16 ` [for-linus][PATCH 13/20] tracing: Take the reference before publishing the named " Steven Rostedt
2026-09-11 18:16 ` [for-linus][PATCH 14/20] tracing: Undo the registration when enabling the histogram trigger fails Steven Rostedt
2026-09-11 18:16 ` [for-linus][PATCH 15/20] tracing: Fix memory corruption from the histogram stacktrace modifier Steven Rostedt
2026-09-11 18:16 ` [for-linus][PATCH 16/20] tracing: Fix memory corruption from a "STACKTRACE" histogram key Steven Rostedt
2026-09-11 18:16 ` [for-linus][PATCH 17/20] tracing: Restore :mod: trailer after parsing in ftrace_set_clr_event() Steven Rostedt
2026-09-11 18:16 ` [for-linus][PATCH 18/20] tracing: Fix ring_buffer_read_page_size() kernel-doc Steven Rostedt
2026-09-11 18:16 ` [for-linus][PATCH 19/20] tracing: Take trace_array reference when opening a tracer options file Steven Rostedt
2026-09-11 18:16 ` [for-linus][PATCH 20/20] ring-buffer: Acquire the lock with irqsave in rb_wake_up_waiters() 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=20260911181729.623284631@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=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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.