| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20240613133711.2867745-7-zhengyejian1@huawei.com>
Date: Thu, 13 Jun 2024 21:37:11 +0800
From: Zheng Yejian <zhengyejian1@...wei.com>
To: <rostedt@...dmis.org>, <mhiramat@...nel.org>, <mark.rutland@....com>,
<mpe@...erman.id.au>, <npiggin@...il.com>, <christophe.leroy@...roup.eu>,
<naveen.n.rao@...ux.ibm.com>, <tglx@...utronix.de>, <mingo@...hat.com>,
<bp@...en8.de>, <dave.hansen@...ux.intel.com>, <x86@...nel.org>,
<hpa@...or.com>, <mcgrof@...nel.org>, <mathieu.desnoyers@...icios.com>,
<masahiroy@...nel.org>, <nathan@...nel.org>, <nicolas@...sle.eu>,
<kees@...nel.org>, <james.clark@....com>, <kent.overstreet@...ux.dev>,
<yhs@...com>, <jpoimboe@...nel.org>, <peterz@...radead.org>
CC: <zhengyejian1@...wei.com>, <linux-kernel@...r.kernel.org>,
<linux-trace-kernel@...r.kernel.org>, <linuxppc-dev@...ts.ozlabs.org>,
<linux-modules@...r.kernel.org>, <linux-kbuild@...r.kernel.org>,
<bpf@...r.kernel.org>
Subject: [PATCH 6/6] ftrace: Revert the FTRACE_MCOUNT_MAX_OFFSET workaround
After patch titled "ftrace: Skip invalid __fentry__ in
ftrace_process_locs()", __fentry__ locations in overridden weak function
have been checked and skipped, then all records in ftrace_pages are
valid, the FTRACE_MCOUNT_MAX_OFFSET workaround can be reverted, include:
1. commit b39181f7c690 ("ftrace: Add FTRACE_MCOUNT_MAX_OFFSET to avoid
adding weak function")
2. commit 7af82ff90a2b ("powerpc/ftrace: Ignore weak functions")
3. commit f6834c8c59a8 ("powerpc/ftrace: Fix dropping weak symbols with
older toolchains")
Signed-off-by: Zheng Yejian <zhengyejian1@...wei.com>
---
arch/powerpc/include/asm/ftrace.h | 7 --
arch/x86/include/asm/ftrace.h | 7 --
kernel/trace/ftrace.c | 141 +-----------------------------
3 files changed, 2 insertions(+), 153 deletions(-)
diff --git a/arch/powerpc/include/asm/ftrace.h b/arch/powerpc/include/asm/ftrace.h
index 107fc5a48456..d6ed058c8041 100644
--- a/arch/powerpc/include/asm/ftrace.h
+++ b/arch/powerpc/include/asm/ftrace.h
@@ -10,13 +10,6 @@
#define HAVE_FUNCTION_GRAPH_RET_ADDR_PTR
-/* Ignore unused weak functions which will have larger offsets */
-#if defined(CONFIG_MPROFILE_KERNEL) || defined(CONFIG_ARCH_USING_PATCHABLE_FUNCTION_ENTRY)
-#define FTRACE_MCOUNT_MAX_OFFSET 16
-#elif defined(CONFIG_PPC32)
-#define FTRACE_MCOUNT_MAX_OFFSET 8
-#endif
-
#ifndef __ASSEMBLY__
extern void _mcount(void);
diff --git a/arch/x86/include/asm/ftrace.h b/arch/x86/include/asm/ftrace.h
index 897cf02c20b1..7a147c9da08d 100644
--- a/arch/x86/include/asm/ftrace.h
+++ b/arch/x86/include/asm/ftrace.h
@@ -9,13 +9,6 @@
# define MCOUNT_ADDR ((unsigned long)(__fentry__))
#define MCOUNT_INSN_SIZE 5 /* sizeof mcount call */
-/* Ignore unused weak functions which will have non zero offsets */
-#ifdef CONFIG_HAVE_FENTRY
-# include <asm/ibt.h>
-/* Add offset for endbr64 if IBT enabled */
-# define FTRACE_MCOUNT_MAX_OFFSET ENDBR_INSN_SIZE
-#endif
-
#ifdef CONFIG_DYNAMIC_FTRACE
#define ARCH_SUPPORTS_FTRACE_OPS 1
#endif
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index c46c35ac9b42..1d60dc9a850b 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -49,8 +49,6 @@
#define FTRACE_NOCLEAR_FLAGS (FTRACE_FL_DISABLED | FTRACE_FL_TOUCHED | \
FTRACE_FL_MODIFIED)
-#define FTRACE_INVALID_FUNCTION "__ftrace_invalid_address__"
-
#define FTRACE_WARN_ON(cond) \
({ \
int ___r = cond; \
@@ -3709,105 +3707,6 @@ static void add_trampoline_func(struct seq_file *m, struct ftrace_ops *ops,
seq_printf(m, " ->%pS", ptr);
}
-#ifdef FTRACE_MCOUNT_MAX_OFFSET
-/*
- * Weak functions can still have an mcount/fentry that is saved in
- * the __mcount_loc section. These can be detected by having a
- * symbol offset of greater than FTRACE_MCOUNT_MAX_OFFSET, as the
- * symbol found by kallsyms is not the function that the mcount/fentry
- * is part of. The offset is much greater in these cases.
- *
- * Test the record to make sure that the ip points to a valid kallsyms
- * and if not, mark it disabled.
- */
-static int test_for_valid_rec(struct dyn_ftrace *rec)
-{
- char str[KSYM_SYMBOL_LEN];
- unsigned long offset;
- const char *ret;
-
- ret = kallsyms_lookup(rec->ip, NULL, &offset, NULL, str);
-
- /* Weak functions can cause invalid addresses */
- if (!ret || offset > FTRACE_MCOUNT_MAX_OFFSET) {
- rec->flags |= FTRACE_FL_DISABLED;
- return 0;
- }
- return 1;
-}
-
-static struct workqueue_struct *ftrace_check_wq __initdata;
-static struct work_struct ftrace_check_work __initdata;
-
-/*
- * Scan all the mcount/fentry entries to make sure they are valid.
- */
-static __init void ftrace_check_work_func(struct work_struct *work)
-{
- struct ftrace_page *pg;
- struct dyn_ftrace *rec;
-
- mutex_lock(&ftrace_lock);
- do_for_each_ftrace_rec(pg, rec) {
- test_for_valid_rec(rec);
- } while_for_each_ftrace_rec();
- mutex_unlock(&ftrace_lock);
-}
-
-static int __init ftrace_check_for_weak_functions(void)
-{
- INIT_WORK(&ftrace_check_work, ftrace_check_work_func);
-
- ftrace_check_wq = alloc_workqueue("ftrace_check_wq", WQ_UNBOUND, 0);
-
- queue_work(ftrace_check_wq, &ftrace_check_work);
- return 0;
-}
-
-static int __init ftrace_check_sync(void)
-{
- /* Make sure the ftrace_check updates are finished */
- if (ftrace_check_wq)
- destroy_workqueue(ftrace_check_wq);
- return 0;
-}
-
-late_initcall_sync(ftrace_check_sync);
-subsys_initcall(ftrace_check_for_weak_functions);
-
-static int print_rec(struct seq_file *m, unsigned long ip)
-{
- unsigned long offset;
- char str[KSYM_SYMBOL_LEN];
- char *modname;
- const char *ret;
-
- ret = kallsyms_lookup(ip, NULL, &offset, &modname, str);
- /* Weak functions can cause invalid addresses */
- if (!ret || offset > FTRACE_MCOUNT_MAX_OFFSET) {
- snprintf(str, KSYM_SYMBOL_LEN, "%s_%ld",
- FTRACE_INVALID_FUNCTION, offset);
- ret = NULL;
- }
-
- seq_puts(m, str);
- if (modname)
- seq_printf(m, " [%s]", modname);
- return ret == NULL ? -1 : 0;
-}
-#else
-static inline int test_for_valid_rec(struct dyn_ftrace *rec)
-{
- return 1;
-}
-
-static inline int print_rec(struct seq_file *m, unsigned long ip)
-{
- seq_printf(m, "%ps", (void *)ip);
- return 0;
-}
-#endif
-
static int t_show(struct seq_file *m, void *v)
{
struct ftrace_iterator *iter = m->private;
@@ -3835,13 +3734,7 @@ static int t_show(struct seq_file *m, void *v)
if (iter->flags & FTRACE_ITER_ADDRS)
seq_printf(m, "%lx ", rec->ip);
- if (print_rec(m, rec->ip)) {
- /* This should only happen when a rec is disabled */
- WARN_ON_ONCE(!(rec->flags & FTRACE_FL_DISABLED));
- seq_putc(m, '\n');
- return 0;
- }
-
+ seq_printf(m, "%ps", (void *)rec->ip);
if (iter->flags & (FTRACE_ITER_ENABLED | FTRACE_ITER_TOUCHED)) {
struct ftrace_ops *ops;
@@ -4221,24 +4114,6 @@ add_rec_by_index(struct ftrace_hash *hash, struct ftrace_glob *func_g,
return 0;
}
-#ifdef FTRACE_MCOUNT_MAX_OFFSET
-static int lookup_ip(unsigned long ip, char **modname, char *str)
-{
- unsigned long offset;
-
- kallsyms_lookup(ip, NULL, &offset, modname, str);
- if (offset > FTRACE_MCOUNT_MAX_OFFSET)
- return -1;
- return 0;
-}
-#else
-static int lookup_ip(unsigned long ip, char **modname, char *str)
-{
- kallsyms_lookup(ip, NULL, NULL, modname, str);
- return 0;
-}
-#endif
-
static int
ftrace_match_record(struct dyn_ftrace *rec, struct ftrace_glob *func_g,
struct ftrace_glob *mod_g, int exclude_mod)
@@ -4246,12 +4121,7 @@ ftrace_match_record(struct dyn_ftrace *rec, struct ftrace_glob *func_g,
char str[KSYM_SYMBOL_LEN];
char *modname;
- if (lookup_ip(rec->ip, &modname, str)) {
- /* This should only happen when a rec is disabled */
- WARN_ON_ONCE(system_state == SYSTEM_RUNNING &&
- !(rec->flags & FTRACE_FL_DISABLED));
- return 0;
- }
+ kallsyms_lookup(rec->ip, NULL, NULL, &modname, str);
if (mod_g) {
int mod_matches = (modname) ? ftrace_match(modname, mod_g) : 0;
@@ -6887,13 +6757,6 @@ void ftrace_module_enable(struct module *mod)
if (!within_module(rec->ip, mod))
break;
- /* Weak functions should still be ignored */
- if (!test_for_valid_rec(rec)) {
- /* Clear all other flags. Should not be enabled anyway */
- rec->flags = FTRACE_FL_DISABLED;
- continue;
- }
-
cnt = 0;
/*
--
2.25.1
Powered by blists - more mailing lists