From: Tze-nan wu <Tze-nan.Wu@mediatek.com>
To: <rostedt@goodmis.org>, <mhiramat@kernel.org>
Cc: <bobule.chang@mediatek.com>, <wsd_upstream@mediatek.com>,
Tze-nan wu <Tze-nan.Wu@mediatek.com>,
Cheng-Jui Wang <cheng-jui.wang@mediatek.com>,
Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
<linux-kernel@vger.kernel.org>,
<linux-trace-kernel@vger.kernel.org>,
<linux-arm-kernel@lists.infradead.org>,
<linux-mediatek@lists.infradead.org>
Subject: [PATCH] tracing: Fix uaf issue in tracing_open_file_tr
Date: Fri, 26 Apr 2024 15:34:08 +0800 [thread overview]
Message-ID: <20240426073410.17154-1-Tze-nan.Wu@mediatek.com> (raw)
"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
next reply other threads:[~2024-04-26 7:35 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-26 7:34 Tze-nan wu [this message]
2024-04-29 0:28 ` [PATCH] tracing: Fix uaf issue in tracing_open_file_tr Steven Rostedt
2024-04-29 18:46 ` Steven Rostedt
2024-05-02 3:10 ` Tze-nan Wu (吳澤南)
2024-05-02 3:50 ` Steven Rostedt
2024-05-02 4:23 ` Tze-nan Wu (吳澤南)
2024-05-02 6:49 ` Tze-nan Wu (吳澤南)
2024-05-02 14:09 ` Steven Rostedt
2024-05-03 1:20 ` Tze-nan Wu (吳澤南)
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240426073410.17154-1-Tze-nan.Wu@mediatek.com \
--to=tze-nan.wu@mediatek.com \
--cc=bobule.chang@mediatek.com \
--cc=cheng-jui.wang@mediatek.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mediatek@lists.infradead.org \
--cc=linux-trace-kernel@vger.kernel.org \
--cc=mathieu.desnoyers@efficios.com \
--cc=mhiramat@kernel.org \
--cc=rostedt@goodmis.org \
--cc=wsd_upstream@mediatek.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).