Re: [PATCH 1/2] ftrace: fix UAF when lookup kallsym after ftrace disabled
From: Steven Rostedt <rostedt@goodmis.org>
Date: 2025-05-28 14:12:19
Also in:
lkml
On Wed, 28 May 2025 21:22:37 +0800 yebin [off-list ref] wrote:
This problem can be solved by releasing the 'mod_map' resource when the module is unloaded. Freeing up these resources is just an address that cannot be translated into symbols, and there are no worse consequences.
OK, I'm fine with releasing the mod_map resource without doing the text modifications.
quoted
Again, the only solution to a ftrace_disable being set is a full reboot.We can't ask users to know such specialized details of the implementation, which are unclear even to developers unfamiliar with the ftrace module. Users can accept planned reboot system recovery, but should not accept casual operations and the system crashes.All we can do is do a good job of protection, give users more tolerance.Perhaps a system that is dead but won't lie down is also a very undesirable situation.However, ftrace is used to collect information and locate faults. Even if it does not work, it does not affect services.In the production environment, the most afraid of using ftrace suddenly crashes the system.Therefore, the robustness of the tool itself is very important.
Preventing ftrace from crashing the system is the reason ftrace_disabled is set and stops it from doing any more damage. If you are worried about users not knowing that a reboot is necessary, we could always add the "Dazed and confused, but trying to continue" message that could also recommend a reboot.
quoted hunk ↗ jump to hunk
quoted
-- SteveI reworked the two patches, and the changes to the existing process should be minimal. I don't know if I can get your approval. If you agree, I'll post another V3 version. PATCH[1/2]:diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c index 51580e54677f..b3436d86e470 100644 --- a/kernel/trace/ftrace.c +++ b/kernel/trace/ftrace.c@@ -7438,9 +7438,10 @@ void ftrace_release_mod(struct module *mod) mutex_lock(&ftrace_lock); - if (ftrace_disabled) - goto out_unlock; - + /* + * To avoid the UAF problem after the module is unloaded, the + * 'mod_map' resource needs to be released unconditionally. + */ list_for_each_entry_safe(mod_map, n, &ftrace_mod_maps, list) { if (mod_map->mod == mod) { list_del_rcu(&mod_map->list);@@ -7451,6 +7452,9 @@ void ftrace_release_mod(struct module *mod) } } + if (ftrace_disabled) + goto out_unlock; + /* * Each module has its own ftrace_pages, remove * them from the list.
Yes, this is more appropriate. Thanks, -- Steve
quoted hunk ↗ jump to hunk
PATCH[2/2]:diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c index a3d4dfad0cbc..51580e54677f 100644 --- a/kernel/trace/ftrace.c +++ b/kernel/trace/ftrace.c@@ -7629,6 +7629,9 @@ allocate_ftrace_mod_map(struct module *mod, { struct ftrace_mod_map *mod_map; + if (ftrace_disabled) + return NULL; + mod_map = kmalloc(sizeof(*mod_map), GFP_KERNEL); if (!mod_map) return NULL;