lkml.org 
[lkml]   [2014]   [Feb]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH 0/4] x86: Fix ftrace recovery when code modification failed
On Mon, 17 Feb 2014 16:22:49 +0100
Petr Mladek <pmladek@suse.cz> wrote:

> Ftrace modifies function calls using Int3 breakpoints on x86. It patches
> all functions in parallel to reduce the number of sync() calls. There is
> a code that removes pending Int3 breakpoints when something goes wrong.
>
> The recovery does not work on x86_64. I simulated an error in
> ftrace_replace_code() and the system got rebooted.

Thanks for the report, I just did the same and it caused a reboot too.

>
> This patch set fixes the recovery code, improves debugging of this
> type of problem, and does some clean up.
>
> BTW: It is an echo from the patch set that tried to use text_poke_bp()
> instead of the ftrace-specific Int3 based patching code. Ftrace has
> some specific restrictions. I did not find a way how to make the
> universal text_poke_bp effective, safe, and clean to be usable
> there.
>
> The patch set is against linux/tip. Last commit is a5b3cca53c43c3ba7
> (Merge tag 'v3.14-rc3')
>
> Petr Mladek (4):
> x86: Clean up remove_breakpoint() in ftrace code
> x86: Fix ftrace patching recovery code to work on x86_64
> x86: BUG when ftrace patching recovery fails
> x86: Fix order of warning messages when ftrace modifies code
>
> arch/x86/kernel/ftrace.c | 67 ++++++++++++++++++++++++------------------------
> 1 file changed, 34 insertions(+), 33 deletions(-)
>

That's quite a bit of changes. I did a quick debug analysis of it, and
found two bugs.

One, I shouldn't be using direct access to the ip with
probe_kernel_write(), I need to do the within() magic (also have a
ftrace_write() wrapper now).

The second bug is that I need to do another run_sync() after the fix
up. Can you see if this patch fixes the issue?

-- Steve

ftrace.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
index e625319..6b566c8 100644
--- a/arch/x86/kernel/ftrace.c
+++ b/arch/x86/kernel/ftrace.c
@@ -455,7 +455,7 @@ static int remove_breakpoint(struct dyn_ftrace *rec)
}

update:
- return probe_kernel_write((void *)ip, &nop[0], 1);
+ return ftrace_write(ip, nop, 1);
}

static int add_update_code(unsigned long ip, unsigned const char *new)
@@ -634,6 +634,7 @@ void ftrace_replace_code(int enable)
rec = ftrace_rec_iter_record(iter);
remove_breakpoint(rec);
}
+ run_sync();
}

static int
@@ -664,7 +665,7 @@ ftrace_modify_code(unsigned long ip, unsigned const
char *old_code, return ret;

fail_update:
- probe_kernel_write((void *)ip, &old_code[0], 1);
+ ftrace_write(ip, old_code, 1);
goto out;
}



\
 
 \ /
  Last update: 2014-02-21 06:01    [W:1.608 / U:0.028 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site