On Wed, 1 Apr 2026 10:40:01 -0400
Steven Rostedt <[email protected]> wrote:

> On Wed, 1 Apr 2026 12:19:57 +0900
> Masami Hiramatsu (Google) <[email protected]> wrote:
> 
> > > 
> > >   CPU 0                                                   CPU 1
> > >   -----                                                   -----
> > >   open(trace_pipe);
> > >   read(..);
> > >   close(trace_pipe);
> > >      kick the work queue to delete it....
> > >                                           rmdir();
> > >                                                   [instance deleted]  
> > 
> > I thought this requires trace_types_lock, and after kicked the queue,
> > can rmdir() gets the tr? (__trace_array_get() return error if
> > tr->free_on_close is set)
> 
> rmdir() doesn't use __trace_array_get(), it uses trace_array_find() which
> we shouldn't need to modify.
> 
> static int instance_rmdir(const char *name)
> {
>       struct trace_array *tr;
> 
>       guard(mutex)(&event_mutex);
>       guard(mutex)(&trace_types_lock);
> 
>       tr = trace_array_find(name);
>       if (!tr)
>               return -ENODEV;
> 
>       return __remove_instance(tr);
> }

Oops, OK it must be updated too.

Thanks,

> 
> > 
> > > 
> > >   __remove_instance();
> > > 
> > >    [ now the tr is freed, and the remove will crash!]
> > > 
> > > 
> > > What would prevent this is this is to use trace_array_destroy() that 
> > > checks
> > > this and also adds the proper locking:
> > > 
> > > static void trace_array_autoremove(struct work_struct *work)
> > > {
> > >   struct trace_array *tr = container_of(work, struct trace_array, 
> > > autoremove_work);
> > > 
> > >   trace_array_destroy(tr);
> > > }  
> > 
> > OK, let's use it.
> 
> Yes, by using trace_array_destroy(), it will fix this.
> 
> Thanks,
> 
> -- Steve


-- 
Masami Hiramatsu (Google) <[email protected]>

Reply via email to