[PATCH] tracing: Fix uaf issue in tracing_open_file_tr

Tze-nan wu
2024-04-26T07:36:03Z

"tracing_event_file" is at the risk of use-after-free due to the race of
two functions "tracing_open_file_tr" and "synth_event_release".
Specifically, it could be freed by synth_event_release before
tracing_open_file_tr has the opportunity to access its members.

It's easy to reproduced by first executing script 'A' and then script 'B'
in my environment.

Script 'A':
echo "hello int aaa" > /sys/kernel/tracing/synthetic_events
while :
do
echo 0 > /sys/kernel/tracing/events/synthetic/hello/enable
done

Script 'B':
echo > /sys/kernel/tracing/synthetic/events

My environment:
arm64 + generic and swtag based KASAN both enabled + kernel-6.6.18

KASAN report
==================================================================
BUG: KASAN: slab-use-after-free in tracing_open_file_tr
Read of size 8 at addr 9*ffff********** by task sh/3485
Pointer tag: [9*], memory tag: [fe]

CPU: * PID: 3485 Comm: sh Tainted: ****************
Call trace:
__hwasan_tag_mismatch
tracing_open_file_tr
do_dentry_open
vfs_open
path_openat

Freed by task 3490:
slab_free_freelist_hook
kmem_cache_free
event_file_put
remove_event_file_dir
__trace_remove_event_call
trace_remove_event_call
synth_event_release
dyn_events_release_all
synth_events_open

page last allocated via order 0, migratetype Unmovable:
__slab_alloc
kmem_cache_alloc
trace_create_new_event
trace_add_event_call
register_synth_event
__create_synth_event
create_or_delete_synth_event
trace_parse_run_command
synth_events_write
vfs_write

Based on the assumption that eventfs_inode will persist throughout the
execution of the tracing_open_file_tr function,
we can fix this issue by incrementing the reference count of
trace_event_file once it is assigned to eventfs_inode->data.
The reference count will then be decremented during the release phase of
eventfs_inode.

This ensures that trace_event_file is not prematurely freed while the
tracing_open_file_tr function is being called.

Fixes: bb32500fb9b7 ("tracing: Have trace_event_file have ref counters")
Co-developed-by: Cheng-Jui Wang <cheng-jui.wang@mediatek.com>
Signed-off-by: Cheng-Jui Wang <cheng-jui.wang@mediatek.com>
Signed-off-by: Tze-nan wu <Tze-nan.Wu@mediatek.com>
---
BTW, I've also attempted to reproduce the same issue in another
environment (described below).
It's also reproducible but in a lower rate.

another environment:
x86 + ubuntu + generic kasan enabled + kernel-6.9-rc4

After applying the patch, KASAN no longer reports any issues when
following the same reproduction steps in my arm64 environment.
However, I am concerned about potential side effects that the patch might introduce.
Additionally, the newly added function "is_entry_event_callback" may not
be reliable in my opinion,
as the callback function used in the comparison could change in future.
Nonetheless, this is the best solution I can come up with now.

Looking for any suggestion or solution, appreciate.

fs/tracefs/event_inode.c | 3 +++
include/linux/trace_events.h | 4 ++++
kernel/trace/trace_events.c | 6 ++++++
3 files changed, 13 insertions(+)

diff --git a/fs/tracefs/event_inode.c b/fs/tracefs/event_inode.c
index 894c6ca1e500..fd49c0f88500 100644
--- a/fs/tracefs/event_inode.c
+++ b/fs/tracefs/event_inode.c
@@ -20,6 +20,7 @@
#include <linux/workqueue.h>
#include <linux/security.h>
#include <linux/tracefs.h>
+#include <linux/trace_events.h>
#include <linux/kref.h>
#include <linux/delay.h>
#include "internal.h"
@@ -90,6 +91,8 @@ static void release_ei(struct kref *ref)

kfree(ei->entry_attrs);
kfree_const(ei->name);
+ if (ei->nr_entries && is_entry_event_callback(ei->entries[0]))
+ event_file_put(ei->data);
if (ei->is_events) {
rei = get_root_inode(ei);
kfree_rcu(rei, ei.rcu);
diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h
index 6f9bdfb09d1d..602e87682ee2 100644
--- a/include/linux/trace_events.h
+++ b/include/linux/trace_events.h
@@ -9,6 +9,7 @@
#include <linux/hardirq.h>
#include <linux/perf_event.h>
#include <linux/tracepoint.h>
+#include <linux/tracefs.h>

struct trace_array;
struct array_buffer;
@@ -505,6 +506,7 @@ extern struct trace_event_file *trace_get_event_file(const char *instance,
const char *system,
const char *event);
extern void trace_put_event_file(struct trace_event_file *file);
+bool is_entry_event_callback(struct eventfs_entry entry);

#define MAX_DYNEVENT_CMD_LEN (2048)

@@ -731,6 +733,8 @@ extern void
event_triggers_post_call(struct trace_event_file *file,
enum event_trigger_type tt);

+extern void event_file_put(struct trace_event_file *file);
+
bool trace_event_ignore_this_pid(struct trace_event_file *trace_file);

bool __trace_trigger_soft_disabled(struct trace_event_file *file);
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index 52f75c36bbca..de01676b59ff 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -2626,6 +2626,7 @@ event_create_dir(struct eventfs_inode *parent, struct trace_event_file *file)
return -1;
}

+ event_file_get(file);
file->ei = ei;

ret = event_define_fields(call);
@@ -3420,6 +3421,11 @@ void trace_put_event_file(struct trace_event_file *file)
}
EXPORT_SYMBOL_GPL(trace_put_event_file);

+bool is_entry_event_callback(struct eventfs_entry entry)
+{
+ return entry.callback == event_callback;
+}
+
#ifdef CONFIG_DYNAMIC_FTRACE

/* Avoid typos */
--
2.18.0