On Wed, Aug 28, 2024 at 10:59:40AM +0800, Zheng Yejian wrote: > From: Steven Rostedt <rostedt@xxxxxxxxxxx> > > commit b1560408692cd0ab0370cfbe9deb03ce97ab3f6d upstream. > > When eventfs was introduced, special care had to be done to coordinate the > freeing of the file meta data with the files that are exposed to user > space. The file meta data would have a ref count that is set when the file > is created and would be decremented and freed after the last user that > opened the file closed it. When the file meta data was to be freed, it > would set a flag (EVENT_FILE_FL_FREED) to denote that the file is freed, > and any new references made (like new opens or reads) would fail as it is > marked freed. This allowed other meta data to be freed after this flag was > set (under the event_mutex). > > All the files that were dynamically created in the events directory had a > pointer to the file meta data and would call event_release() when the last > reference to the user space file was closed. This would be the time that it > is safe to free the file meta data. > > A shortcut was made for the "format" file. It's i_private would point to > the "call" entry directly and not point to the file's meta data. This is > because all format files are the same for the same "call", so it was > thought there was no reason to differentiate them. The other files > maintain state (like the "enable", "trigger", etc). But this meant if the > file were to disappear, the "format" file would be unaware of it. > > This caused a race that could be trigger via the user_events test (that > would create dynamic events and free them), and running a loop that would > read the user_events format files: > > In one console run: > > # cd tools/testing/selftests/user_events > # while true; do ./ftrace_test; done > > And in another console run: > > # cd /sys/kernel/tracing/ > # while true; do cat events/user_events/__test_event/format; done 2>/dev/null > > With KASAN memory checking, it would trigger a use-after-free bug report > (which was a real bug). This was because the format file was not checking > the file's meta data flag "EVENT_FILE_FL_FREED", so it would access the > event that the file meta data pointed to after the event was freed. > > After inspection, there are other locations that were found to not check > the EVENT_FILE_FL_FREED flag when accessing the trace_event_file. Add a > new helper function: event_file_file() that will make sure that the > event_mutex is held, and will return NULL if the trace_event_file has the > EVENT_FILE_FL_FREED flag set. Have the first reference of the struct file > pointer use event_file_file() and check for NULL. Later uses can still use > the event_file_data() helper function if the event_mutex is still held and > was not released since the event_file_file() call. > > Link: https://lore.kernel.org/all/20240719204701.1605950-1-minipli@xxxxxxxxxxxxxx/ > > Cc: stable@xxxxxxxxxxxxxxx > Cc: Masami Hiramatsu <mhiramat@xxxxxxxxxx> > Cc: Mathieu Desnoyers <mathieu.desnoyers@xxxxxxxxxxxx> > Cc: Ajay Kaher <ajay.kaher@xxxxxxxxxxxx> > Cc: Ilkka Naulapää <digirigawa@xxxxxxxxx> > Cc: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> > Cc: Al Viro <viro@xxxxxxxxxxxxxxxxxx> > Cc: Dan Carpenter <dan.carpenter@xxxxxxxxxx> > Cc: Beau Belgrave <beaub@xxxxxxxxxxxxxxxxxxx> > Cc: Florian Fainelli <florian.fainelli@xxxxxxxxxxxx> > Cc: Alexey Makhalov <alexey.makhalov@xxxxxxxxxxxx> > Cc: Vasavi Sirnapalli <vasavi.sirnapalli@xxxxxxxxxxxx> > Link: https://lore.kernel.org/20240730110657.3b69d3c1@xxxxxxxxxxxxxxxxxx > Fixes: b63db58e2fa5d ("eventfs/tracing: Add callback for release of an eventfs_inode") > Reported-by: Mathias Krause <minipli@xxxxxxxxxxxxxx> > Tested-by: Mathias Krause <minipli@xxxxxxxxxxxxxx> > Signed-off-by: Steven Rostedt (Google) <rostedt@xxxxxxxxxxx> > [Resolve conflict due to lack of commit a1f157c7a3bb ("tracing: Expand all > ring buffers individually") which add tracing_update_buffers() in > event_enable_write(), that commit is more of a feature than a bugfix > and is not related to the problem fixed by this patch] > Signed-off-by: Zheng Yejian <zhengyejian@xxxxxxxxxxxxxxx> Now queued up, thanks. greg k-h
