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
Subject: [for-linus][PATCH 19/20] tracing: Take trace_array reference when opening a tracer options file
Date: Fri, 11 Sep 2026 14:16:55 -0400	[thread overview]
Message-ID: <20260911181732.007790558@kernel.org> (raw)
In-Reply-To: <20260911181636.485043797@kernel.org>

From: Steven Rostedt <rostedt@goodmis.org>

When a tracer option file is opened, it is passed a descriptor that points
to an element on the trace_array's topts array. This element has
information to find the trace array and other information. It uses this
element to take a reference of the trace_array so that the trace_array
does not get removed while this file is opened.

Unfortunately, there's a race condition where the element itself could be
freed by the removal of the instance the trace_array represents causing a
use-after-free as this element that is used to find the trace_array to
increment its reference counter is also freed when the instance is
removed.

To solve this, add a trace_array_tracer_options_get() helper function that
will take the address of the element that is passed to the open function
by the inode->i_private pointer and search all the trace_arrays under a
lock to find the one that the element's address is in the range of the
trace_arrays topts array elements. When a match happens, that trace_array's
reference would be increased.

Note, there's a race where if an admin was deleting and creating trace
instances at the same time and the memory of the old trace_array's array
matched the memory of the new trace_array that it could in theory open the
option from the wrong trace array. But we do not care because it would be
stupid to perform that kind of action. As long as the only thing that can
happen is that the option from the wrong trace array is used and doesn't
crash the kernel it will only make the user confused. But if they are
doing something stupid like this, they are already confused, so no harm
done.

Cc: stable@vger.kernel.org
Link: https://patch.msgid.link/20260910221209.62dad8d3@robin
Fixes: 7e2cfbd2d3c86 ("tracing: Have option files inc the trace array ref count")
Reported-by: sashiko-bot@kernel.org
Closes: https://lore.kernel.org/linux-trace-kernel/20260902121918.5a9e9d1b@gandalf.local.home/
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 kernel/trace/trace.c | 46 +++++++++++++++++++++++++++++++++++++++++++-
 kernel/trace/trace.h |  1 +
 2 files changed, 46 insertions(+), 1 deletion(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 8658cad53cb5..e4a490d3d08c 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -7717,12 +7717,55 @@ trace_options_write(struct file *filp, const char __user *ubuf, size_t cnt,
 	return cnt;
 }
 
+static bool tr_option_match(struct trace_array *tr, void *topt)
+{
+	for (int i = 0; i < tr->nr_topts; i++) {
+		struct trace_options *tr_topts = &tr->topts[i];
+
+		if (topt >= (void *)&tr_topts->topts[0] &&
+		    topt < (void *)&tr_topts->topts[tr_topts->nr_topts])
+			return true;
+	}
+	return false;
+}
+
+/*
+ * The topt is the address of a trace_array->topts[] element that holds the
+ * the tracer options descriptor. But since the trace_array reference has not
+ * been taken yet, it cannot be dereferenced as it could have been freed by
+ * a rmdir of the instance the trace_array represents.
+ *
+ * Search the list of trace_arrays and compare the topt to the address of
+ * the entire trace_array topts array for each trace_array in the list.
+ * If one is matched, then take the reference and return it. If not, the
+ * trace_array no longer exits.
+ */
+static int trace_array_tracer_options_get(void *topt)
+{
+	struct trace_array *tr;
+	int ret;
+
+	ret = security_locked_down(LOCKDOWN_TRACEFS);
+	if (ret)
+		return ret;
+
+	if (tracing_disabled)
+		return -ENODEV;
+
+	guard(mutex)(&trace_types_lock);
+	list_for_each_entry(tr, &ftrace_trace_arrays, list) {
+		if (tr_option_match(tr, topt))
+			return __trace_array_get(tr);
+	}
+	return -ENODEV;
+}
+
 static int tracing_open_options(struct inode *inode, struct file *filp)
 {
 	struct trace_option_dentry *topt = inode->i_private;
 	int ret;
 
-	ret = tracing_check_open_get_tr(topt->tr);
+	ret = trace_array_tracer_options_get(topt);
 	if (ret)
 		return ret;
 
@@ -7984,6 +8027,7 @@ create_trace_option_files(struct trace_array *tr, struct tracer *tracer,
 	tr->topts = tr_topts;
 	tr->topts[tr->nr_topts].tracer = tracer;
 	tr->topts[tr->nr_topts].topts = topts;
+	tr->topts[tr->nr_topts].nr_topts = cnt;
 	tr->nr_topts++;
 
 	for (cnt = 0; opts[cnt].name; cnt++) {
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 5e76f94e7a80..bd3c8f80300f 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -227,6 +227,7 @@ struct array_buffer {
 struct trace_options {
 	struct tracer			*tracer;
 	struct trace_option_dentry	*topts;
+	int				nr_topts;
 };
 
 struct trace_pid_list *trace_pid_list_alloc(void);
-- 
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 ` [for-linus][PATCH 07/20] tracing: Free histogram the field rejected for a bad modifier Steven Rostedt
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 ` Steven Rostedt [this message]
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=20260911181732.007790558@kernel.org \
    --to=rostedt@kernel.org \
    --cc=akpm@linux-foundation.org \
    --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®