[PATCH v2] tracing: Take trace_array reference when opening options file

[PATCH v2] tracing: Take trace_array reference when opening options file

[Steven Rostedt]

From: Steven Rostedt <rostedt@goodmis.org>

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.

As the options are somewhat dynamic in their creation at boot up, each
file represents a flag in the trace_array. The trace_array has an array of
indexes to represent each of these flags that is stored in the
trace_flags_index array. The address of the index array element is used to
pass to the inode->i_private pointer. Then that element is read which
holds the index (which represents the flag) and then the index is used to
calculate the trace_array descriptor from its trace_flags_index array.

One issue is that the index element can not be referenced until the
trace_array's reference is taken. To handle this, create a new helper
function called: trace_array_options_get() that will iterate all the
existing trace_arrays in the ftrace_trace_arrays list (under the
trace_types_lock), and compare the passed in address of the index element
with the entire array of the trace_array's trace_flags_index array.
If it matches, then up the corresponding trace_array's reference and
return.

Cc: stable@vger.kernel.org
Fixes: 577b785f55168 ("tracing: add tracer dependent options to options directory")
Reported-by: sashiko-bot@kernel.org
Closes: https://lore.kernel.org/linux-trace-kernel/20260828135858.2AC501F000E9@smtp.kernel.org/
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
Changes since v1: https://patch.msgid.link/20260901163620.6cbe0ada@gandalf.local.home

- Added (void *) typecast to comparison. (kernel test robot)

 kernel/trace/trace.c | 67 +++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 63 insertions(+), 4 deletions(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index a946e0183fd1..61ca05ee991d 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -7842,11 +7842,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 >= &tr->trace_flags_index[0] &&
+		    tr_index < &tr->trace_flags_index[TRACE_FLAGS_MAX_SIZE])
+			return __trace_array_get(tr);
+	}
+	return -ENODEV;
+}
+
+static int trace_options_open(struct inode *inode, struct file *filp)
+{
+	void *tr_index = inode->i_private;
+
+	if (trace_array_options_get(tr_index) < 0)
+		return -ENODEV;
+
+	filp->private_data = tr_index;
+
+	return 0;
+}
+
+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,
 };
 
 struct dentry *trace_create_file(const char *name,
-- 
2.53.0

Re: [PATCH v2] tracing: Take trace_array reference when opening options file

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:

New issues:
- [Low] The patch introduces a call to `__trace_array_get()`, which is neither declared nor defined anywhere in the kernel, causing a build failure.
- [Low] The commit message claims to have added a `(void *)` cast to fix a kernel test robot warning, but the cast is missing from the submitted patch.

Pre-existing issues:
- [Critical] The patch fixes the use-after-free for core options, but leaves tracer-specific options and other per-instance files vulnerable to the exact same use-after-free.
- [Critical] The validation logic for the dangling trace_array pointer is vulnerable to an ABA (Slab Reuse) problem, allowing cross-instance privilege escalation.
--

commit 16f2151bdbc0994e69e88bb0c5631ea2027da7fa
Author: Steven Rostedt <rostedt@goodmis.org>

tracing: Take trace_array reference when opening options file

This patch addresses a use-after-free when an options file is opened by
one task and the underlying trace instance is removed by another. It adds
a new helper to validate the index element address and safely acquire a
reference to the trace_array.

Closes: https://lore.kernel.org/linux-trace-kernel/20260828135858.2AC501F000E9@smtp.kernel.org/

> - Added (void *) typecast to comparison. (kernel test robot)

[Severity: Low]
Does the pointer comparison in the code still trigger a compiler warning?
The patch notes mention adding a (void *) typecast to fix a kernel test
robot warning, but the cast appears to be missing from the submitted code.

> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index d3005279165d9..3e001ed8cf376 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c

[ ... ]

> +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 >= &tr->trace_flags_index[0] &&
> +		    tr_index < &tr->trace_flags_index[TRACE_FLAGS_MAX_SIZE])

[Severity: Critical]
This is a pre-existing issue, but does this validation logic remain
vulnerable to an ABA (slab reuse) problem? If an unprivileged user holds
an O_PATH file descriptor to a freed instance's options file, the inode
is kept alive with a dangling i_private pointer.

If a new instance is created and allocated at the exact same memory
address, wouldn't this bounds check falsely succeed on the dangling
pointer, granting read and write access to the new (potentially root-created)
trace_array options? This same ABA flaw seems to exist in the general
trace_array_get() function as well.

> +			return __trace_array_get(tr);

[Severity: Low]
Is __trace_array_get() defined anywhere in the kernel? It looks like
this might cause a build failure because the function is neither declared
nor defined.

> +	}
> +	return -ENODEV;
> +}

[ ... ]

>  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 by only updating trace_options_core_fops,
do tracer-specific options and other per-instance files remain vulnerable
to the exact same use-after-free? 

Files like trace_options_fops, set_tracer_fops, and tracing_max_lat_fops
still use tracing_open_generic for their open callbacks, which fails to
take a reference to the instance's trace_array.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260902114502.723d8243@gandalf.local.home?part=1

Re: [PATCH v2] tracing: Take trace_array reference when opening options file

[Steven Rostedt]

On Wed, 02 Sep 2026 16:02:07 +0000
sashiko-bot@kernel.org wrote:

> 
> [Severity: Low]
> Does the pointer comparison in the code still trigger a compiler warning?
> The patch notes mention adding a (void *) typecast to fix a kernel test
> robot warning, but the cast appears to be missing from the submitted code.
> 

Bah! I forgot to hit git commit -a --amend before sending the patch!


> > diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> > index d3005279165d9..3e001ed8cf376 100644
> > --- a/kernel/trace/trace.c
> > +++ b/kernel/trace/trace.c  
> 
> [ ... ]
> 
> > +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 >= &tr->trace_flags_index[0] &&
> > +		    tr_index < &tr->trace_flags_index[TRACE_FLAGS_MAX_SIZE])  
> 
> [Severity: Critical]
> This is a pre-existing issue, but does this validation logic remain
> vulnerable to an ABA (slab reuse) problem? If an unprivileged user holds
> an O_PATH file descriptor to a freed instance's options file, the inode
> is kept alive with a dangling i_private pointer.
> 
> If a new instance is created and allocated at the exact same memory
> address, wouldn't this bounds check falsely succeed on the dangling
> pointer, granting read and write access to the new (potentially root-created)
> trace_array options? This same ABA flaw seems to exist in the general
> trace_array_get() function as well.

As said before, we don't care.

> 
> > +			return __trace_array_get(tr);  
> 
> [Severity: Low]
> Is __trace_array_get() defined anywhere in the kernel? It looks like
> this might cause a build failure because the function is neither declared
> nor defined.
> 
> > +	}
> > +	return -ENODEV;
> > +}  
> 
> [ ... ]
> 
> >  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 by only updating trace_options_core_fops,
> do tracer-specific options and other per-instance files remain vulnerable
> to the exact same use-after-free? 
> 
> Files like trace_options_fops, set_tracer_fops, and tracing_max_lat_fops
> still use tracing_open_generic for their open callbacks, which fails to
> take a reference to the instance's trace_array.
> 

The trace_options_fops does indeed have the issue as it uses topts->tr
where it can not trust the topts. But set_tracer_fops and
tracing_max_lat_fops use tracing_open_generic_tr().

Are you using the master branch of the repo? That's from 2022 and very old.

I'll set the default branch to be the for-next branch so hopefully you
don't report old bugs that have been fixed a long time ago anymore.

-- Steve

Re: [PATCH v2] tracing: Take trace_array reference when opening options file

[Steven Rostedt]

On Wed, 2 Sep 2026 12:15:27 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Wed, 02 Sep 2026 16:02:07 +0000
> sashiko-bot@kernel.org wrote:
> 
> > 
> > [Severity: Low]
> > Does the pointer comparison in the code still trigger a compiler warning?
> > The patch notes mention adding a (void *) typecast to fix a kernel test
> > robot warning, but the cast appears to be missing from the submitted code.
> >   
> 
> Bah! I forgot to hit git commit -a --amend before sending the patch!

Looking at my bash history, I did a "git commit --amend" without the "-a".

Oh well. Next time should include the update.

-- Steve