Skip to contentSkip to conversation

tracing: More fixes for 7.2

PATCH New Steven Rostedt 1 participant 3 messages Jul 26

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

cover tracing: More fixes for 7.2

tracing fixes for 7.2:

- Add mutex to protect parser in ftrace filtering

The set_ftrace_filter file uses a parsing descriptor that is allocated at open and modified by writes. If multiple threads were to write to the descriptor at the same time, it can corrupt the parser.

Add a mutex around the modifications of the parser descriptor.

- Fix possible corruption in perf syscall tracing

The perf system call trace events can now read user space. To do so, the reads of user space enable preemption and disables it again. During this time that preemption is enabled, the task can migrate. The perf event list head is assigned via a per CPU pointer. It is done before the user space part is called. If the user space reading migrates the task to another CPU, then the head pointer is no longer valid.

Re-assign the head pointer after the reading of user space to keep it using the correct data.

git://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace.git
trace/fixes

Head SHA1: 2c2b322acdcc78575b8d6afa64a085cf92e03c12

Steven Rostedt (1):
tracing: perf: Fix stale head for perf syscall tracing

Tengda Wu (1):
ftrace: Add global mutex to serialize trace_parser access

----

 kernel/trace/ftrace.c         | 13 +++++++++++++
 kernel/trace/trace_syscalls.c |  5 +++++
 2 files changed, 18 insertions(+)
#1 lore raw

PATCH 1/2 ftrace: Add global mutex to serialize trace_parser access

From: Tengda Wu <[email protected]>

In ftrace, the trace_parser structure is allocated and initialized when a trace file is opened, and is subsequently used across write and release handlers to parse user input.

The affected handler paths and their specific functions are:
- Open paths: ftrace_regex_open(), ftrace_graph_open()
- Write paths: ftrace_regex_write(), ftrace_graph_write()
- Release paths: ftrace_regex_release(), ftrace_graph_release()

If userspace opens a trace file descriptor and shares it across multiple threads, concurrent write calls will race on the parser's internal state, specifically the 'idx', 'cont', and 'buffer' fields, leading to corrupted input or undefined behavior.

Fix this by adding a global mutex, parser_lock, to serialize all access to trace_parser across write and release paths, preventing concurrent corruption of parser state.

Fixes e704eff3ff51 ("ftrace: Have set_graph_function handle multiple functions in one write")
Fixes 689fd8b65d66 ("tracing: trace parser support for function and graph")
---
 kernel/trace/ftrace.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)
kernel/trace/ftrace.c +13 -0
index f93e34dd2328..6c47a94f5924 100644
@@ -1097,6 +1097,12 @@ struct ftrace_ops global_ops = {
 					  FTRACE_OPS_FL_PID,
 };
 
+/*
+ * parser_lock - Protects trace_parser state against concurrent operations.
+ * Held across trace_get_user() and subsequent buffer parsing to prevent races.
+ */
+static DEFINE_MUTEX(parser_lock);
+
 /*
  * Used by the stack unwinder to know about dynamic ftrace trampolines.
  */
@@ -5842,6 +5848,8 @@ ftrace_regex_write(struct file *file, const char __user *ubuf,
 	/* iter->hash is a local copy, so we don't need regex_lock */
 
 	parser = &iter->parser;
+
+	guard(mutex)(&parser_lock);
 	read = trace_get_user(parser, ubuf, cnt, ppos);
 
 	if (read >= 0 && trace_parser_loaded(parser) &&
@@ -6984,12 +6992,14 @@ int ftrace_regex_release(struct inode *inode, struct file *file)
 		iter = file->private_data;
 
 	parser = &iter->parser;
+	mutex_lock(&parser_lock);
 	if (trace_parser_loaded(parser)) {
 		int enable = !(iter->flags & FTRACE_ITER_NOTRACE);
 
 		ftrace_process_regex(iter, parser->buffer,
 				     parser->idx, enable);
 	}
+	mutex_unlock(&parser_lock);
 
 	trace_parser_put(parser);
 
@@ -7321,10 +7331,12 @@ ftrace_graph_release(struct inode *inode, struct file *file)
 
 		parser = &fgd->parser;
 
+		mutex_lock(&parser_lock);
 		if (trace_parser_loaded((parser))) {
 			ret = ftrace_graph_set_hash(fgd->new_hash,
 						    parser->buffer);
 		}
+		mutex_unlock(&parser_lock);
 
 		trace_parser_put(parser);
 
@@ -7437,6 +7449,7 @@ ftrace_graph_write(struct file *file, const char __user *ubuf,
 
 	parser = &fgd->parser;
 
+	guard(mutex)(&parser_lock);
 	read = trace_get_user(parser, ubuf, cnt, ppos);
 
 	if (read >= 0 && trace_parser_loaded(parser) &&
Signature · 1 line
2.53.0
#2 lore raw

PATCH 2/2 tracing: perf: Fix stale head for perf syscall tracing

In reply to Steven Rostedt

From: Steven Rostedt <[email protected]>

The code that can read the user space parameters of a system call may enable preemption and migrate. The head of the per CPU perf events list may be pointing to the wrong CPU event if the code migrates the task.

Reassign the head pointer if the system call event called the code that may have caused a migration.

Reported-by Sashiko <>
Fixes edca33a56297d ("tracing: Fix failure to read user space from system call trace events")
---
 kernel/trace/trace_syscalls.c | 5 +++++
 1 file changed, 5 insertions(+)
kernel/trace/trace_syscalls.c +5 -0
index e98ee7e1e66f..8a4f3c75e39f 100644
@@ -1451,6 +1451,11 @@ static void perf_syscall_enter(void *ignore, struct pt_regs *regs, long id)
 		if (syscall_get_data(sys_data, args, &user_ptr,
 				     &size, user_sizes, &uargs, buf_size) < 0)
 			return;
+
+		/* The above may have caused a migration */
+		head = this_cpu_ptr(sys_data->enter_event->perf_events);
+		if (hlist_empty(head))
+			return;
 	}
 
 	/* get the size after alignment with the u32 buffer size field */
Signature · 1 line
2.53.0