Re: [PATCH v3] tracing: Take trace_array reference when opening options file
From: sashiko-bot@kernel.org
Date: 2026-09-02 16:39:44
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: New issues: - [High] Cross-instance privilege escalation via SLAB reuse in `trace_array_options_get`. - [Low] Missing definition of `__trace_array_get()`, causing a compilation failure. Pre-existing issues: - [Critical] The patch only fixes the use-after-free for core options files (`trace_options_core_fops`), leaving other instance files vulnerable to the exact same UAF. -- commit 692e0bd47e6544a583d3d3e1c5faabd7a045c813 Author: Steven Rostedt [off-list ref] tracing: Take trace_array reference when opening options file The options files do not take the trace_array reference for the options they represent. This could cause a use-after-free kernel crash if one of these files is opened by one task and another task removes the instance that the option is for. Because it doesn't take a reference upon opening, it will not stop the removal which will free the options descriptor that is being used. Closes: https://lore.kernel.org/linux-trace-kernel/20260828135858.2AC501F000E9@smtp.kernel.org/ (local)
quoted hunk ↗ jump to hunk
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c index d3005279165d9..f172946edcb23 100644 --- a/kernel/trace/trace.c +++ b/kernel/trace/trace.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -8809,11 +8809,70 @@ trace_options_core_write(struct file *filp, const char __user *ubuf, size_t cnt, return cnt; } +/* + * The tr_index is the address of a trace_array->trace_flags_index[] + * element that holds the index of the trace flag. But since the + * trace_array reference has not been taken yet, it cannot be referenced + * 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 tr_index to the + * address of the entire trace_array trace_flags_index 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_options_get(void *tr_index) +{ + 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_index >= (void *)&tr->trace_flags_index[0] && + tr_index < (void *)&tr->trace_flags_index[TRACE_FLAGS_MAX_SIZE])
[Severity: High] Can this raw memory address validation in trace_array_options_get() be spoofed if a trace instance is removed and its memory is reused by the SLAB allocator for a new, potentially more privileged trace instance? If an unprivileged user starts opening an option file for instance A, but instance A is removed and its memory is reallocated to instance B before this check runs, could the stale tr_index match the new instance B's memory bounds and grant unauthorized access to its options?
+ return __trace_array_get(tr);
[Severity: Low] Is __trace_array_get() defined anywhere? Will this cause a compilation failure because of an implicit function declaration or undefined reference when compiling the kernel with tracing enabled?
+ } + return -ENODEV; +} +
[ ... ]
+static int trace_options_release(struct inode *inode, struct file *filp)
+{
+ void *tr_index = filp->private_data;
+ struct trace_array *tr;
+ unsigned int index;
+
+ get_tr_index(tr_index, &tr, &index);
+
+ trace_array_put(tr);
+
+ return 0;
+}
+
static const struct file_operations trace_options_core_fops = {
- .open = tracing_open_generic,
- .read = trace_options_core_read,
- .write = trace_options_core_write,
- .llseek = generic_file_llseek,
+ .open = trace_options_open,
+ .read = trace_options_core_read,
+ .write = trace_options_core_write,
+ .llseek = generic_file_llseek,
+ .release = trace_options_release,
};[Severity: Critical] This is a pre-existing issue, but since we are fixing the use-after-free for core option files by changing .open to trace_options_open here, do we also need to fix other instance files like trace_options_fops, set_tracer_fops, and tracing_max_lat_fops? These structures still use tracing_open_generic, which does not take a reference on the trace array. Could they be vulnerable to the same use-after-free if another task removes the trace instance while their files are held open? -- Sashiko AI review · https://sashiko.dev/#/patchset/20260902121918.5a9e9d1b@gandalf.local.home?part=1