Re: ftrace: UAF in ftrace_regex_open() on a removed tracing instance

[Steven Rostedt <rostedt@goodmis.org>]

On Fri, 28 Aug 2026 08:59:39 -0700
Breno Leitao <leitao@debian.org> wrote:

> 
> My LLM came up with something like the following, and it doesn't
> reproduce the problem anymore. Very similar to your code above:

I know the solution but there's subtle things I want to make sure that
works.

> 
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index f9d80c7bd9f16..37845399fe5cf 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -4788,22 +4788,37 @@ 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;
> +	int ret;
>  
> -	/* Checks for tracefs lockdown */
> -	return ftrace_regex_open(ops,
> -			FTRACE_ITER_FILTER | FTRACE_ITER_DO_PROBES,
> -			inode, file);
> +	/* Checks for tracefs lockdown, and that the instance is still alive */
> +	ret = tracing_check_open_get_tr(tr);
> +	if (ret)
> +		return ret;
> +
> +	ret = ftrace_regex_open(tr->ops,
> +				FTRACE_ITER_FILTER | FTRACE_ITER_DO_PROBES,
> +				inode, file);
> +	/* ftrace_regex_open() holds its own reference on success */
> +	trace_array_put(tr);
> +	return ret;
>  }
>  
>  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;
> +	int ret;
> +
> +	/* Checks for tracefs lockdown, and that the instance is still alive */
> +	ret = tracing_check_open_get_tr(tr);
> +	if (ret)
> +		return ret;
>  
> -	/* Checks for tracefs lockdown */
> -	return ftrace_regex_open(ops, FTRACE_ITER_NOTRACE,
> -				 inode, file);
> +	ret = ftrace_regex_open(tr->ops, FTRACE_ITER_NOTRACE, inode, file);
> +	/* ftrace_regex_open() holds its own reference on success */
> +	trace_array_put(tr);
> +	return ret;
>  }
>  
>  /* Type for quick search ftrace basic regexes (globs) from filter_parse_regex */
> @@ -7496,11 +7511,15 @@ void ftrace_create_filter_files(struct ftrace_ops *ops,
>  				struct dentry *parent)
>  {
>  
> +	/*
> +	 * Pass the trace array and not @ops, as @ops is freed when the
> +	 * instance is removed, but the trace array can still be validated.
> +	 */

I hate these comments. It's really pointless to state this. It's a "no shit" comment ;-)


>  	trace_create_file("set_ftrace_filter", TRACE_MODE_WRITE, parent,
> -			  ops, &ftrace_filter_fops);
> +			  ops->private, &ftrace_filter_fops);

But this also assumes that ops->private is always a trace_array. Which may
someday change and we will get a hidden bug if it does.

-- Steve


>  
>  	trace_create_file("set_ftrace_notrace", TRACE_MODE_WRITE, parent,
> -			  ops, &ftrace_notrace_fops);
> +			  ops->private, &ftrace_notrace_fops);
>  }
>  
>  /*