[PATCH] ftrace: Take trace_array reference before accessing its ftrace_ops

[PATCH] ftrace: Take trace_array reference before accessing its ftrace_ops

[Steven Rostedt]

From: Steven Rostedt <rostedt@goodmis.org>

The trace instance files set_ftrace_filter and set_ftrace_notrace was
updated to work with specific trace instances (trace_arrays). The issue is
that when these files are opened, there is a small race window where it
will use the ftrace_ops from the inode->private pointer to get a reference
to the trace_array and then take its reference. The problem is that the
ftrace_ops itself could be freed. If the rmdir on the instance happens at
the same time the set_ftrace_filter file is opened, the rmdir could have
also freed the ftrace_ops and referencing it will cause a use-after-free
bug and crash the kernel.

Instead, pass in the trace_array as the file private data (NULL for the
top level instance), and then pass both the trace_array and the ftrace_ops
to the ftrace_regex_open() function. If the trace_array is NULL, then it
just uses the ftrace_ops without the need to take its reference (like
normal). If the ftrace_ops is NULL, that is only the case for the top
level instance and the global_ops can be used.

This allows the trace_array to have its reference incremented before
touching the ftrace_ops that could also be freed when the instance is.

Cc: stable@vger.kernel.org
Fixes: 591dffdade9f0 ("ftrace: Allow for function tracing instance to filter functions")
Reported-by: Breno Leitao <leitao@debian.org>
Closes: https://lore.kernel.org/all/apGORjltZgAiAYHT@gmail.com/
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 include/linux/ftrace.h         |  5 ++--
 kernel/trace/ftrace.c          | 52 ++++++++++++++++++++--------------
 kernel/trace/trace.h           |  5 ++--
 kernel/trace/trace_functions.c |  2 +-
 kernel/trace/trace_stack.c     |  2 +-
 5 files changed, 39 insertions(+), 27 deletions(-)

diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index 02bc5027523a..bd76a16a63af 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -866,8 +866,9 @@ unsigned long ftrace_get_addr_new(struct dyn_ftrace *rec);
 unsigned long ftrace_get_addr_curr(struct dyn_ftrace *rec);
 
 extern ftrace_func_t ftrace_trace_function;
+struct trace_array;
 
-int ftrace_regex_open(struct ftrace_ops *ops, int flag,
+int ftrace_regex_open(struct trace_array *tr, struct ftrace_ops *ops, int flag,
 		  struct inode *inode, struct file *file);
 ssize_t ftrace_filter_write(struct file *file, const char __user *ubuf,
 			    size_t cnt, loff_t *ppos);
@@ -1077,7 +1078,7 @@ static inline unsigned long ftrace_location(unsigned long ip)
  * have them defined when ftrace is not enabled, but these
  * functions may still be called. Use a macro instead of inline.
  */
-#define ftrace_regex_open(ops, flag, inod, file) ({ -ENODEV; })
+#define ftrace_regex_open(tr, ops, flag, inode, file) ({ -ENODEV; })
 #define ftrace_set_early_filter(ops, buf, enable) do { } while (0)
 #define ftrace_set_filter_ip(ops, ip, remove, reset) ({ -ENODEV; })
 #define ftrace_set_filter_ips(ops, ips, cnt, remove, reset) ({ -ENODEV; })
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index f9d80c7bd9f1..4babd86c7be0 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -4677,7 +4677,8 @@ ftrace_avail_addrs_open(struct inode *inode, struct file *file)
 
 /**
  * ftrace_regex_open - initialize function tracer filter files
- * @ops: The ftrace_ops that hold the hash filters
+ * @tr: The trace_array that holds the ftrace_ops [optional]
+ * @ops: The ftrace_ops that hold the hash filters [optional]
  * @flag: The type of filter to process
  * @inode: The inode, usually passed in to your open routine
  * @file: The file, usually passed in to your open routine
@@ -4691,26 +4692,38 @@ ftrace_avail_addrs_open(struct inode *inode, struct file *file)
  * tracing_lseek() should be used as the lseek routine, and
  * release must call ftrace_regex_release().
  *
+ * Note, If @tr is not NULL, its reference has to be taken before
+ *       @ops may be referenced.
+ *       If @ops is NULL and @tr is not, then @tr->ops is used.
+ *       If both @tr and @ops are NULL, then the &global_ops is
+ *       to be used.
+ *
  * Returns: 0 on success or a negative errno value on failure
  */
 int
-ftrace_regex_open(struct ftrace_ops *ops, int flag,
+ftrace_regex_open(struct trace_array *tr, struct ftrace_ops *ops, int flag,
 		  struct inode *inode, struct file *file)
 {
-	struct ftrace_iterator *iter;
+	struct ftrace_iterator *iter = NULL;
 	struct ftrace_hash *hash;
 	struct list_head *mod_head;
-	struct trace_array *tr = ops->private;
-	int ret = -ENOMEM;
-
-	ftrace_ops_init(ops);
+	int ret = -ENODEV;
 
 	if (unlikely(ftrace_disabled))
 		return -ENODEV;
 
-	if (tracing_check_open_get_tr(tr))
+	if (tr && tracing_check_open_get_tr(tr))
 		return -ENODEV;
 
+	if (!ops)
+		ops = tr ? tr->ops : &global_ops;
+
+	if (WARN_ON_ONCE(!ops))
+		goto out;
+
+	ftrace_ops_init(ops);
+
+	ret = -ENOMEM;
 	iter = kzalloc_obj(*iter);
 	if (!iter)
 		goto out;
@@ -4788,21 +4801,19 @@ ftrace_regex_open(struct ftrace_ops *ops, int flag,
 static int
 ftrace_filter_open(struct inode *inode, struct file *file)
 {
-	struct ftrace_ops *ops = inode->i_private;
+	struct trace_array *tr = inode->i_private;
 
-	/* Checks for tracefs lockdown */
-	return ftrace_regex_open(ops,
-			FTRACE_ITER_FILTER | FTRACE_ITER_DO_PROBES,
-			inode, file);
+	return ftrace_regex_open(tr, NULL,
+				 FTRACE_ITER_FILTER | FTRACE_ITER_DO_PROBES,
+				 inode, file);
 }
 
 static int
 ftrace_notrace_open(struct inode *inode, struct file *file)
 {
-	struct ftrace_ops *ops = inode->i_private;
+	struct trace_array *tr = inode->i_private;
 
-	/* Checks for tracefs lockdown */
-	return ftrace_regex_open(ops, FTRACE_ITER_NOTRACE,
+	return ftrace_regex_open(tr, NULL, FTRACE_ITER_NOTRACE,
 				 inode, file);
 }
 
@@ -7492,15 +7503,15 @@ static const struct file_operations ftrace_graph_notrace_fops = {
 };
 #endif /* CONFIG_FUNCTION_GRAPH_TRACER */
 
-void ftrace_create_filter_files(struct ftrace_ops *ops,
+void ftrace_create_filter_files(struct trace_array *tr,
 				struct dentry *parent)
 {
 
 	trace_create_file("set_ftrace_filter", TRACE_MODE_WRITE, parent,
-			  ops, &ftrace_filter_fops);
+			  tr, &ftrace_filter_fops);
 
 	trace_create_file("set_ftrace_notrace", TRACE_MODE_WRITE, parent,
-			  ops, &ftrace_notrace_fops);
+			  tr, &ftrace_notrace_fops);
 }
 
 /*
@@ -7525,7 +7536,6 @@ void ftrace_destroy_filter_files(struct ftrace_ops *ops)
 
 static __init int ftrace_init_dyn_tracefs(struct dentry *d_tracer)
 {
-
 	trace_create_file("available_filter_functions", TRACE_MODE_READ,
 			d_tracer, NULL, &ftrace_avail_fops);
 
@@ -7538,7 +7548,7 @@ static __init int ftrace_init_dyn_tracefs(struct dentry *d_tracer)
 	trace_create_file("touched_functions", TRACE_MODE_READ,
 			d_tracer, NULL, &ftrace_touched_fops);
 
-	ftrace_create_filter_files(&global_ops, d_tracer);
+	ftrace_create_filter_files(NULL, d_tracer);
 
 #ifdef CONFIG_FUNCTION_GRAPH_TRACER
 	trace_create_file("set_graph_function", TRACE_MODE_WRITE, d_tracer,
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 74a7a50d1e78..3c111ca88e32 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -1340,7 +1340,7 @@ extern void clear_ftrace_function_probes(struct trace_array *tr);
 int register_ftrace_command(struct ftrace_func_command *cmd);
 int unregister_ftrace_command(struct ftrace_func_command *cmd);
 
-void ftrace_create_filter_files(struct ftrace_ops *ops,
+void ftrace_create_filter_files(struct trace_array *tr,
 				struct dentry *parent);
 void ftrace_destroy_filter_files(struct ftrace_ops *ops);
 
@@ -1363,11 +1363,12 @@ static inline void clear_ftrace_function_probes(struct trace_array *tr)
 {
 }
 
+static inline void ftrace_create_filter_files(struct trace_array *tr,
+					      struct dentry *parent) { }
 /*
  * The ops parameter passed in is usually undefined.
  * This must be a macro.
  */
-#define ftrace_create_filter_files(ops, parent) do { } while (0)
 #define ftrace_destroy_filter_files(ops) do { } while (0)
 #endif /* CONFIG_FUNCTION_TRACER && CONFIG_DYNAMIC_FTRACE */
 
diff --git a/kernel/trace/trace_functions.c b/kernel/trace/trace_functions.c
index cd37f2013758..c879d43a5fbb 100644
--- a/kernel/trace/trace_functions.c
+++ b/kernel/trace/trace_functions.c
@@ -101,7 +101,7 @@ int ftrace_create_function_files(struct trace_array *tr,
 		return ret;
 	}
 
-	ftrace_create_filter_files(tr->ops, parent);
+	ftrace_create_filter_files(tr, parent);
 
 	return 0;
 }
diff --git a/kernel/trace/trace_stack.c b/kernel/trace/trace_stack.c
index 0aa2514a6593..e7f4e523587d 100644
--- a/kernel/trace/trace_stack.c
+++ b/kernel/trace/trace_stack.c
@@ -499,7 +499,7 @@ stack_trace_filter_open(struct inode *inode, struct file *file)
 	struct ftrace_ops *ops = inode->i_private;
 
 	/* Checks for tracefs lockdown */
-	return ftrace_regex_open(ops, FTRACE_ITER_FILTER,
+	return ftrace_regex_open(NULL, ops, FTRACE_ITER_FILTER,
 				 inode, file);
 }
 
-- 
2.53.0

Re: [PATCH] ftrace: Take trace_array reference before accessing its ftrace_ops

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] Tracefs lockdown and tracing_disabled checks are bypassed for the global ftrace instance and stack tracer.
- [High] Module and probe filters for the global ftrace instance cannot be cleared or read.
--

commit 42c008f904dcd516dc1681a548fae6a4bba9a162
Author: Steven Rostedt <rostedt@goodmis.org>

ftrace: Take trace_array reference before accessing its ftrace_ops

The trace instance files set_ftrace_filter and set_ftrace_notrace are
updated to work with specific trace instances (trace_arrays). This
patch passes the trace_array as the file private data to allow incrementing
its reference before touching the ftrace_ops, preventing a use-after-free.

Closes: https://lore.kernel.org/all/apGORjltZgAiAYHT@gmail.com/

> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index f9d80c7bd9f16..4babd86c7be00 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c

[ ... ]

>  int
> -ftrace_regex_open(struct ftrace_ops *ops, int flag,
> +ftrace_regex_open(struct trace_array *tr, struct ftrace_ops *ops, int flag,
>  		  struct inode *inode, struct file *file)
>  {
> -	struct ftrace_iterator *iter;
> +	struct ftrace_iterator *iter = NULL;
>  	struct ftrace_hash *hash;
>  	struct list_head *mod_head;
> -	struct trace_array *tr = ops->private;
> -	int ret = -ENOMEM;
> -
> -	ftrace_ops_init(ops);
> +	int ret = -ENODEV;
>  
>  	if (unlikely(ftrace_disabled))
>  		return -ENODEV;
>  
> -	if (tracing_check_open_get_tr(tr))
> +	if (tr && tracing_check_open_get_tr(tr))
>  		return -ENODEV;

[Severity: Critical]
By adding the `tr &&` short-circuit here, does this cause a regression
where tracefs lockdown and tracing_disabled checks are bypassed for the
global ftrace instance and the stack tracer?

Since `tr` is explicitly passed as `NULL` for these instances,
`tracing_check_open_get_tr(NULL)` is never called.

[ ... ]

> @@ -4788,21 +4801,19 @@ ftrace_regex_open(struct ftrace_ops *ops, int flag,
>  static int
>  ftrace_filter_open(struct inode *inode, struct file *file)
>  {
> -	struct ftrace_ops *ops = inode->i_private;
> +	struct trace_array *tr = inode->i_private;
>  
> -	/* Checks for tracefs lockdown */
> -	return ftrace_regex_open(ops,
> -			FTRACE_ITER_FILTER | FTRACE_ITER_DO_PROBES,
> -			inode, file);
> +	return ftrace_regex_open(tr, NULL,
> +				 FTRACE_ITER_FILTER | FTRACE_ITER_DO_PROBES,
> +				 inode, file);
>  }

[Severity: High]
For the global ftrace instance, `tr` is `NULL`. Does passing `NULL` for `tr`
break the module and probe filters?

In `ftrace_regex_open()`, the `mod_head` pointer is initialized like this:

    mod_head = tr ? &tr->mod_trace : NULL;

Because `tr` is `NULL`, `mod_head` will also be `NULL`. When opening the
file with `O_TRUNC`, `clear_ftrace_mod_list(mod_head)` will do nothing,
failing to clear the module filters.

Additionally, `iter->tr` is set to `NULL`. When reading the file,
`t_mod_start()` relies on `iter->tr` and will return early:

    /* probes are only available if tr is set */
    if (!iter->tr)
        return NULL;

Could this skip the enumeration of active module and probe filters for the
global instance entirely?

[ ... ]

> diff --git a/kernel/trace/trace_stack.c b/kernel/trace/trace_stack.c
> index 0aa2514a6593b..e7f4e523587d0 100644
> --- a/kernel/trace/trace_stack.c
> +++ b/kernel/trace/trace_stack.c
> @@ -499,7 +499,7 @@ stack_trace_filter_open(struct inode *inode, struct file *file)
>  	struct ftrace_ops *ops = inode->i_private;
>  
>  	/* Checks for tracefs lockdown */
> -	return ftrace_regex_open(ops, FTRACE_ITER_FILTER,
> +	return ftrace_regex_open(NULL, ops, FTRACE_ITER_FILTER,
>  				 inode, file);
>  }

[Severity: Critical]
The stack tracer explicitly passes `NULL` for the trace array. Due to the
short-circuit mentioned earlier, doesn't this also bypass the lockdown
check documented in the comment right above this call?

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

Re: [PATCH] ftrace: Take trace_array reference before accessing its ftrace_ops

[Steven Rostedt]

On Fri, 28 Aug 2026 20:13:09 +0000
sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [Critical] Tracefs lockdown and tracing_disabled checks are bypassed for the global ftrace instance and stack tracer.
> - [High] Module and probe filters for the global ftrace instance cannot be cleared or read.
> --
> 
> commit 42c008f904dcd516dc1681a548fae6a4bba9a162
> Author: Steven Rostedt <rostedt@goodmis.org>
> 
> ftrace: Take trace_array reference before accessing its ftrace_ops
> 
> The trace instance files set_ftrace_filter and set_ftrace_notrace are
> updated to work with specific trace instances (trace_arrays). This
> patch passes the trace_array as the file private data to allow incrementing
> its reference before touching the ftrace_ops, preventing a use-after-free.
> 
> Closes: https://lore.kernel.org/all/apGORjltZgAiAYHT@gmail.com/
> 
> > diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> > index f9d80c7bd9f16..4babd86c7be00 100644
> > --- a/kernel/trace/ftrace.c
> > +++ b/kernel/trace/ftrace.c  
> 
> [ ... ]
> 
> >  int
> > -ftrace_regex_open(struct ftrace_ops *ops, int flag,
> > +ftrace_regex_open(struct trace_array *tr, struct ftrace_ops *ops, int flag,
> >  		  struct inode *inode, struct file *file)
> >  {
> > -	struct ftrace_iterator *iter;
> > +	struct ftrace_iterator *iter = NULL;
> >  	struct ftrace_hash *hash;
> >  	struct list_head *mod_head;
> > -	struct trace_array *tr = ops->private;
> > -	int ret = -ENOMEM;
> > -
> > -	ftrace_ops_init(ops);
> > +	int ret = -ENODEV;
> >  
> >  	if (unlikely(ftrace_disabled))
> >  		return -ENODEV;
> >  
> > -	if (tracing_check_open_get_tr(tr))
> > +	if (tr && tracing_check_open_get_tr(tr))
> >  		return -ENODEV;  
> 
> [Severity: Critical]
> By adding the `tr &&` short-circuit here, does this cause a regression
> where tracefs lockdown and tracing_disabled checks are bypassed for the
> global ftrace instance and the stack tracer?
> 
> Since `tr` is explicitly passed as `NULL` for these instances,
> `tracing_check_open_get_tr(NULL)` is never called.

Yeah, right after sending this I noticed that the if statement isn't needed.

> 
> [ ... ]
> 
> > @@ -4788,21 +4801,19 @@ ftrace_regex_open(struct ftrace_ops *ops, int flag,
> >  static int
> >  ftrace_filter_open(struct inode *inode, struct file *file)
> >  {
> > -	struct ftrace_ops *ops = inode->i_private;
> > +	struct trace_array *tr = inode->i_private;
> >  
> > -	/* Checks for tracefs lockdown */
> > -	return ftrace_regex_open(ops,
> > -			FTRACE_ITER_FILTER | FTRACE_ITER_DO_PROBES,
> > -			inode, file);
> > +	return ftrace_regex_open(tr, NULL,
> > +				 FTRACE_ITER_FILTER | FTRACE_ITER_DO_PROBES,
> > +				 inode, file);
> >  }  
> 
> [Severity: High]
> For the global ftrace instance, `tr` is `NULL`. Does passing `NULL` for `tr`
> break the module and probe filters?
> 
> In `ftrace_regex_open()`, the `mod_head` pointer is initialized like this:
> 
>     mod_head = tr ? &tr->mod_trace : NULL;
> 
> Because `tr` is `NULL`, `mod_head` will also be `NULL`. When opening the
> file with `O_TRUNC`, `clear_ftrace_mod_list(mod_head)` will do nothing,
> failing to clear the module filters.
> 
> Additionally, `iter->tr` is set to `NULL`. When reading the file,
> `t_mod_start()` relies on `iter->tr` and will return early:
> 
>     /* probes are only available if tr is set */
>     if (!iter->tr)
>         return NULL;
> 
> Could this skip the enumeration of active module and probe filters for the
> global instance entirely?

Hmm, yeah.

I guess I need to add a:

perhaps I need to add at the start:

	if (!tr && ops)
		tr = ops->private;

> 
> [ ... ]
> 
> > diff --git a/kernel/trace/trace_stack.c b/kernel/trace/trace_stack.c
> > index 0aa2514a6593b..e7f4e523587d0 100644
> > --- a/kernel/trace/trace_stack.c
> > +++ b/kernel/trace/trace_stack.c
> > @@ -499,7 +499,7 @@ stack_trace_filter_open(struct inode *inode, struct file *file)
> >  	struct ftrace_ops *ops = inode->i_private;
> >  
> >  	/* Checks for tracefs lockdown */
> > -	return ftrace_regex_open(ops, FTRACE_ITER_FILTER,
> > +	return ftrace_regex_open(NULL, ops, FTRACE_ITER_FILTER,
> >  				 inode, file);
> >  }  
> 
> [Severity: Critical]
> The stack tracer explicitly passes `NULL` for the trace array. Due to the
> short-circuit mentioned earlier, doesn't this also bypass the lockdown
> check documented in the comment right above this call?
> 

Yes, and I mentioned at the start I need to remove the if statement.

-- Steve