Re: [PATCH] tracing: Have format file honor EVENT_FILE_FL_FREED

[Ajay Kaher]

On Fri, Jul 26, 2024 at 9:33 PM Steven Rostedt <rostedt@xxxxxxxxxxx> wrote:
>
> On Fri, 26 Jul 2024 18:00:18 +0530
> Ajay Kaher <ajay.kaher@xxxxxxxxxxxx> wrote:
>
> > Some doubt:
> > Because of the same race condition, it may happen that kmem_cache_free(file)
> > was executed while f_start() is waiting to get event_mutex. Once
> > f_start() acquires
> > event_mutex, it will access the *file which points to the freed cache.
> > I am assuming in this case KASAN will not show anything as *file
> > belongs to cache.
>
> No, the file is freed by the callback from eventfs when the last reference
> to the file is released. That is, there's no more references to the files
> (nothing has it opened). As this code is only called when the file is
> opened, it will not race with the freeing of the descriptor.
>
> See event_create_dir(), it registers the dynamically created directory
> and files. It will also do call event_file_get() that adds a reference
> on this file/directory descriptor. It also registers the
> "event_release" function to be called when the last reference of all
> open files are closed in that directory.
>
> That event_release() will call event_file_put() that does the final
> release and frees the file. This prevents file from being freed while
> anything has it opened.
>
> While looking at this code I did realize that the "format" doesn't
> register an "event_release" and there's no bug with its data pointing
> to the call with respect to freeing something it shouldn't be. But it
> still needs the file pointer anyway so that it can have access to its
> flags.

Got it, VFS calls the callback tracefs_d_release() -> ... ->
kmem_cache_free(file). And this will happen on close of the last
reference.

Following is not related to this bug:
event_release callback executed once 'dir' closed (no more ref), any
specific reason to register with 'dir'/'enable' file. If not, could we
register with the 'dir' instead of 'enable'.

-Ajay