On Wed, Aug 28, 2024 at 10:59:40AM +0800, Zheng Yejian wrote:
From: Steven Rostedt rostedt@goodmis.org
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@grsecurity.net/
Cc: stable@vger.kernel.org Cc: Masami Hiramatsu mhiramat@kernel.org Cc: Mathieu Desnoyers mathieu.desnoyers@efficios.com Cc: Ajay Kaher ajay.kaher@broadcom.com Cc: Ilkka Naulapää digirigawa@gmail.com Cc: Linus Torvalds torvalds@linux-foundation.org Cc: Al Viro viro@zeniv.linux.org.uk Cc: Dan Carpenter dan.carpenter@linaro.org Cc: Beau Belgrave beaub@linux.microsoft.com Cc: Florian Fainelli florian.fainelli@broadcom.com Cc: Alexey Makhalov alexey.makhalov@broadcom.com Cc: Vasavi Sirnapalli vasavi.sirnapalli@broadcom.com Link: https://lore.kernel.org/20240730110657.3b69d3c1@gandalf.local.home Fixes: b63db58e2fa5d ("eventfs/tracing: Add callback for release of an eventfs_inode") Reported-by: Mathias Krause minipli@grsecurity.net Tested-by: Mathias Krause minipli@grsecurity.net Signed-off-by: Steven Rostedt (Google) rostedt@goodmis.org [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@huaweicloud.com
Now queued up, thanks.
greg k-h