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

[Steven Rostedt <rostedt@goodmis.org>]

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