linux-kernel - Re: [PATCH] tracing: Have format file honor EVENT_FILE_FL

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAD2QZ9aFHOTLSryw88PKdvXBQaug9di=CDQ9KjPBhuOTNhjxTQ@mail.gmail.com>
Date: Mon, 29 Jul 2024 18:29:49 +0530
From: Ajay Kaher <ajay.kaher@...adcom.com>
To: Steven Rostedt <rostedt@...dmis.org>
Cc: LKML <linux-kernel@...r.kernel.org>, 
	Linux Trace Kernel <linux-trace-kernel@...r.kernel.org>, Masami Hiramatsu <mhiramat@...nel.org>, 
	Mathieu Desnoyers <mathieu.desnoyers@...icios.com>, Mathias Krause <minipli@...ecurity.net>, 
	Ilkka Naulapää <digirigawa@...il.com>, 
	Linus Torvalds <torvalds@...ux-foundation.org>, Al Viro <viro@...iv.linux.org.uk>, 
	regressions@...mhuis.info, Dan Carpenter <dan.carpenter@...aro.org>, 
	Beau Belgrave <beaub@...ux.microsoft.com>, 
	Florian Fainelli <florian.fainelli@...adcom.com>, 
	Alexey Makhalov <alexey.makhalov@...adcom.com>, 
	Vasavi Sirnapalli <vasavi.sirnapalli@...adcom.com>
Subject: Re: [PATCH] tracing: Have format file honor EVENT_FILE_FL_FREED

On Fri, Jul 26, 2024 at 9:33 PM Steven Rostedt <rostedt@...dmis.org> wrote:
>
> On Fri, 26 Jul 2024 18:00:18 +0530
> Ajay Kaher <ajay.kaher@...adcom.com> 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