[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Fri, 26 Jun 2020 13:42:26 +0200
From: Peter Zijlstra <peterz@...radead.org>
To: Sami Tolvanen <samitolvanen@...gle.com>
Cc: Steven Rostedt <rostedt@...dmis.org>,
Masahiro Yamada <masahiroy@...nel.org>,
Will Deacon <will@...nel.org>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
"Paul E. McKenney" <paulmck@...nel.org>,
Kees Cook <keescook@...omium.org>,
Nick Desaulniers <ndesaulniers@...gle.com>,
clang-built-linux@...glegroups.com,
kernel-hardening@...ts.openwall.com, linux-arch@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, linux-kbuild@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-pci@...r.kernel.org,
x86@...nel.org, Josh Poimboeuf <jpoimboe@...hat.com>,
mhelsley@...are.com
Subject: Re: [RFC][PATCH] objtool,x86_64: Replace recordmcount with objtool
On Fri, Jun 26, 2020 at 01:29:31PM +0200, Peter Zijlstra wrote:
> On Thu, Jun 25, 2020 at 03:40:42PM -0700, Sami Tolvanen wrote:
> > Anyway, since objtool is run before recordmcount, I just left this unchanged
> > for testing and ignored the recordmcount warnings about __mcount_loc already
> > existing. Something is a bit off still though, I see this at boot:
> >
> > ------------[ ftrace bug ]------------
> > ftrace failed to modify
> > [<ffffffff81000660>] __tracepoint_iter_initcall_level+0x0/0x40
> > actual: 0f:1f:44:00:00
> > Initializing ftrace call sites
> > ftrace record flags: 0
> > (0)
> > expected tramp: ffffffff81056500
> > ------------[ cut here ]------------
> >
> > Otherwise, this looks pretty good.
>
> Ha! it is trying to convert the "CALL __fentry__" into a NOP and not
> finding the CALL -- because objtool already made it a NOP...
>
> Weird, I thought recordmcount would also write NOPs, it certainly has
> code for that. I suppose we can use CC_USING_NOP_MCOUNT to avoid those,
> but I'd rather Steve explain this before I wreck things further.
Something like so would ignore whatever text is there and rewrite it
with ideal_nop.
---
diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
index c84d28e90a58..98a6a93d7615 100644
--- a/arch/x86/kernel/ftrace.c
+++ b/arch/x86/kernel/ftrace.c
@@ -109,9 +109,11 @@ static int __ref
ftrace_modify_code_direct(unsigned long ip, const char *old_code,
const char *new_code)
{
- int ret = ftrace_verify_code(ip, old_code);
- if (ret)
- return ret;
+ if (old_code) {
+ int ret = ftrace_verify_code(ip, old_code);
+ if (ret)
+ return ret;
+ }
/* replace the text with the new text */
if (ftrace_poke_late)
@@ -124,9 +126,8 @@ ftrace_modify_code_direct(unsigned long ip, const char *old_code,
int ftrace_make_nop(struct module *mod, struct dyn_ftrace *rec, unsigned long addr)
{
unsigned long ip = rec->ip;
- const char *new, *old;
+ const char *new;
- old = ftrace_call_replace(ip, addr);
new = ftrace_nop_replace();
/*
@@ -138,7 +139,7 @@ int ftrace_make_nop(struct module *mod, struct dyn_ftrace *rec, unsigned long ad
* just modify the code directly.
*/
if (addr == MCOUNT_ADDR)
- return ftrace_modify_code_direct(ip, old, new);
+ return ftrace_modify_code_direct(ip, NULL, new);
/*
* x86 overrides ftrace_replace_code -- this function will never be used
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
