public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Steven Rostedt <rostedt@goodmis.org>
To: linux-kernel@vger.kernel.org
Cc: Ingo Molnar <mingo@kernel.org>,
	Frederic Weisbecker <fweisbec@gmail.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Petr Mladek <pmladek@suse.cz>
Subject: [for-next][PATCH 20/20] ftrace/x86: BUG when ftrace recovery fails
Date: Fri, 07 Mar 2014 10:09:40 -0500	[thread overview]
Message-ID: <20140307151204.171814170@goodmis.org> (raw)
In-Reply-To: 20140307150920.881849073@goodmis.org

[-- Attachment #1: 0020-ftrace-x86-BUG-when-ftrace-recovery-fails.patch --]
[-- Type: text/plain, Size: 2293 bytes --]

From: Petr Mladek <pmladek@suse.cz>

Ftrace modifies function calls using Int3 breakpoints on x86.
The breakpoints are handled only when the patching is in progress.
If something goes wrong, there is a recovery code that removes
the breakpoints. If this fails, the system might get silently
rebooted when a remaining break is not handled or an invalid
instruction is proceed.

We should BUG() when the breakpoint could not be removed. Otherwise,
the system silently crashes when the function finishes the Int3
handler is disabled.

Note that we need to modify remove_breakpoint() to return non-zero
value only when there is an error. The return value was ignored before,
so it does not cause any troubles.

Link: http://lkml.kernel.org/r/1393258342-29978-4-git-send-email-pmladek@suse.cz

Signed-off-by: Petr Mladek <pmladek@suse.cz>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 arch/x86/kernel/ftrace.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
index 4b66adf..52819e8 100644
--- a/arch/x86/kernel/ftrace.c
+++ b/arch/x86/kernel/ftrace.c
@@ -425,7 +425,7 @@ static int remove_breakpoint(struct dyn_ftrace *rec)
 
 	/* If this does not have a breakpoint, we are done */
 	if (ins[0] != brk)
-		return -1;
+		return 0;
 
 	nop = ftrace_nop_replace();
 
@@ -625,7 +625,12 @@ void ftrace_replace_code(int enable)
 	printk(KERN_WARNING "Failed on %s (%d):\n", report, count);
 	for_ftrace_rec_iter(iter) {
 		rec = ftrace_rec_iter_record(iter);
-		remove_breakpoint(rec);
+		/*
+		 * Breakpoints are handled only when this function is in
+		 * progress. The system could not work with them.
+		 */
+		if (remove_breakpoint(rec))
+			BUG();
 	}
 	run_sync();
 }
@@ -649,12 +654,19 @@ ftrace_modify_code(unsigned long ip, unsigned const char *old_code,
 	run_sync();
 
 	ret = ftrace_write(ip, new_code, 1);
+	/*
+	 * The breakpoint is handled only when this function is in progress.
+	 * The system could not work if we could not remove it.
+	 */
+	BUG_ON(ret);
  out:
 	run_sync();
 	return ret;
 
  fail_update:
-	ftrace_write(ip, old_code, 1);
+	/* Also here the system could not work with the breakpoint */
+	if (ftrace_write(ip, old_code, 1))
+		BUG();
 	goto out;
 }
 
-- 
1.8.5.3



      parent reply	other threads:[~2014-03-07 15:13 UTC|newest]

Thread overview: 54+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-03-07 15:09 [for-next][PATCH 00/20] tracing: linux-next updates Steven Rostedt
2014-03-07 15:09 ` [for-next][PATCH 01/20] ftrace/x86: Run a sync after fixup on failure Steven Rostedt
2014-03-07 15:09 ` [for-next][PATCH 02/20] ftrace/x86: One more missing sync after fixup of function modification failure Steven Rostedt
2014-03-07 15:09 ` [for-next][PATCH 03/20] tracepoint: Do not waste memory on mods with no tracepoints Steven Rostedt
2014-03-07 15:09 ` [for-next][PATCH 04/20] ftrace/x86: Have ftrace_write() return -EPERM and clean up callers Steven Rostedt
2014-03-07 15:09 ` [for-next][PATCH 05/20] tracing: Move raw output code from macro to standalone function Steven Rostedt
2014-03-07 15:09 ` [for-next][PATCH 06/20] tracing: Move event storage for array " Steven Rostedt
2014-03-07 15:09 ` [for-next][PATCH 07/20] tracing: Use helper functions in event assignment to shrink macro size Steven Rostedt
2014-03-07 15:09 ` [for-next][PATCH 08/20] tracing: Warn if a tracepoint is not set via debugfs Steven Rostedt
2014-03-10 20:01   ` Mathieu Desnoyers
2014-03-10 20:19     ` Steven Rostedt
2014-03-10 20:55       ` Mathieu Desnoyers
2014-03-11  2:41     ` Frank Ch. Eigler
2014-03-11  2:58       ` Steven Rostedt
2014-03-11  4:08         ` Mathieu Desnoyers
2014-03-11 14:46           ` Steven Rostedt
2014-03-11 14:26         ` Frank Ch. Eigler
2014-03-11 15:06           ` Mathieu Desnoyers
2014-03-11 15:40             ` Steven Rostedt
2014-03-11 17:34               ` Mathieu Desnoyers
2014-03-11 19:13                 ` Steven Rostedt
2014-03-12 14:24                   ` Mathieu Desnoyers
2014-03-12 15:11                     ` Steven Rostedt
2014-03-12 15:46                       ` Steven Rostedt
2014-03-12 16:05                         ` Mathieu Desnoyers
2014-03-12 16:18                           ` Steven Rostedt
2014-03-12 16:39                             ` Mathieu Desnoyers
2014-03-12 17:50                               ` Steven Rostedt
2014-03-12 18:47                                 ` Mathieu Desnoyers
2014-03-12 18:58                                   ` Steven Rostedt
2014-03-12 19:30                                     ` Steven Rostedt
2014-03-12 19:58                                       ` Mathieu Desnoyers
2014-03-12 19:51                                     ` Mathieu Desnoyers
2014-03-12 20:35                                       ` Andi Kleen
2014-03-12 20:47                                         ` Mathieu Desnoyers
2014-03-13  3:15                                           ` Andi Kleen
2014-03-13  3:21                                             ` Mathieu Desnoyers
2014-03-13  0:49                                       ` Steven Rostedt
2014-03-13  3:10                                         ` Mathieu Desnoyers
2014-03-13 15:24                                           ` Mathieu Desnoyers
2014-03-12 16:40                       ` Mathieu Desnoyers
2014-03-12 18:02                         ` Steven Rostedt
2014-03-07 15:09 ` [for-next][PATCH 09/20] tracing: Fix event header writeback.h to include tracepoint.h Steven Rostedt
2014-03-07 15:09 ` [for-next][PATCH 10/20] tracing: Fix event header migrate.h " Steven Rostedt
2014-03-07 15:09 ` [for-next][PATCH 11/20] tracing/module: Replace include of tracepoint.h with jump_label.h in module.h Steven Rostedt
2014-03-07 15:09 ` [for-next][PATCH 12/20] tracing: Correctly expand len expressions from __dynamic_array macro Steven Rostedt
2014-03-07 15:09 ` [for-next][PATCH 13/20] tracing: Evaluate len expression only once in " Steven Rostedt
2014-03-07 15:09 ` [for-next][PATCH 14/20] ftrace: Cleanup of global variables ftrace_new_pgs and ftrace_update_cnt Steven Rostedt
2014-03-07 15:09 ` [for-next][PATCH 15/20] ftrace: Inline the code from ftrace_dyn_table_alloc() Steven Rostedt
2014-03-07 15:09 ` [for-next][PATCH 16/20] ftrace: Pass retval through return in ftrace_dyn_arch_init() Steven Rostedt
2014-03-07 15:09 ` [for-next][PATCH 17/20] ftrace: Do not pass data to ftrace_dyn_arch_init Steven Rostedt
2014-03-07 15:09 ` [for-next][PATCH 18/20] ftrace: Remove freelist from struct dyn_ftrace Steven Rostedt
2014-03-07 15:09 ` [for-next][PATCH 19/20] ftrace: Warn on error when modifying ftrace function Steven Rostedt
2014-03-07 15:09 ` Steven Rostedt [this message]

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=20140307151204.171814170@goodmis.org \
    --to=rostedt@goodmis.org \
    --cc=akpm@linux-foundation.org \
    --cc=fweisbec@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=pmladek@suse.cz \
    /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