tracing: Last minute updates for 4.5

git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git
for-next

Head SHA1: 5156dca34a3e1e1edac2d0dabf43d8632909b7aa

Li Bin (1):
x86: ftrace: Fix the comments for ftrace_modify_code_direct()

Qiu Peiyang (1):
ftrace: Fix the race between ftrace and insmod

Steven Rostedt (Red Hat) (1):
ftrace: Add infrastructure for delayed enabling of module functions

----

 arch/x86/kernel/ftrace.c |  12 ++--
 include/linux/ftrace.h   |   6 +-
 kernel/trace/ftrace.c    | 184 +++++++++++++++++++++++++++++++----------------
 3 files changed, 130 insertions(+), 72 deletions(-)

From: Li Bin <[email protected]>

There is no need to worry about module and __init text disappearing case, because that ftrace has a module notifier that is called when a module is being unloaded and before the text goes away and this code grabs the ftrace_lock mutex and removes the module functions from the ftrace list, such that it will no longer do any modifications to that module's text, the update to make functions be traced or not is done under the ftrace_lock mutex as well. And by now, __init section codes should not been modified by ftrace, because it is black listed in recordmcount.c and ignored by ftrace.

---
 arch/x86/kernel/ftrace.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

index 909da012406d..29408d6d6626 100644
@@ -108,13 +108,11 @@ ftrace_modify_code_direct(unsigned long ip, unsigned const char *old_code,
 	ftrace_expected = old_code;
 
 	/*
-	 * Note: Due to modules and __init, code can
-	 *  disappear and change, we need to protect against faulting
-	 *  as well as code changing. We do this by using the
-	 *  probe_kernel_* functions.
-	 *
-	 * No real locking needed, this code is run through
-	 * kstop_machine, or before SMP starts.
+	 * Note:
+	 * We are paranoid about modifying text, as if a bug was to happen, it
+	 * could cause us to read or write to someplace that could cause harm.
+	 * Carefully read and modify the code with probe_kernel_*(), and make
+	 * sure what we read is what we expected it to be before modifying it.
 	 */
 
 	/* read the text we want to modify */

2.6.2

From: "Steven Rostedt (Red Hat)" <[email protected]>

Qiu Peiyang pointed out that there's a race when enabling function tracing and loading a module. In order to make the modifications of converting nops in the prologue of functions into callbacks, the text needs to be converted from read-only to read-write. When enabling function tracing, the text permission is updated, the functions are modified, and then they are put back.

When loading a module, the updates to convert function calls to mcount is done before the module text is set to read-only. But after it is done, the module text is visible by the function tracer. Thus we have the following race:

	CPU 0			CPU 1
	-----			-----

start function tracing
set text to read-write
load_module
add functions to ftrace set module text read-only

update all functions to callbacks
modify module functions too
< Can't it's read-only >

When this happens, ftrace detects the issue and disables itself till the next reboot.

To fix this, a new DISABLED flag is added for ftrace records, which all module functions get when they are added. Then later, after the module code is all set, the records will have the DISABLED flag cleared, and they will be enabled if any callback wants all functions to be traced.

Note, this doesn't add the delay to later. It simply changes the ftrace_module_init() to do both the setting of DISABLED records, and then immediately calls the enable code. This helps with testing this new code as it has the same behavior as previously. Another change will come after this to have the ftrace_module_enable() called after the text is set to read-only.

---
 include/linux/ftrace.h |   6 +-
 kernel/trace/ftrace.c  | 161 ++++++++++++++++++++++++++++++++-----------------
 2 files changed, 110 insertions(+), 57 deletions(-)

index 4736a826baf5..660e7c698f3b 100644
@@ -357,6 +357,7 @@ bool is_ftrace_trampoline(unsigned long addr);
  *  REGS    - the record wants the function to save regs
  *  REGS_EN - the function is set up to save regs.
  *  IPMODIFY - the record allows for the IP address to be changed.
+ *  DISABLED - the record is not ready to be touched yet
  *
  * When a new ftrace_ops is registered and wants a function to save
  * pt_regs, the rec->flag REGS is set. When the function has been

@@ -371,10 +372,11 @@ enum {
 	FTRACE_FL_TRAMP		= (1UL << 28),
 	FTRACE_FL_TRAMP_EN	= (1UL << 27),
 	FTRACE_FL_IPMODIFY	= (1UL << 26),
+	FTRACE_FL_DISABLED	= (1UL << 25),
 };
 
-#define FTRACE_REF_MAX_SHIFT	26
-#define FTRACE_FL_BITS		6
+#define FTRACE_REF_MAX_SHIFT	25
+#define FTRACE_FL_BITS		7
 #define FTRACE_FL_MASKED_BITS	((1UL << FTRACE_FL_BITS) - 1)
 #define FTRACE_FL_MASK		(FTRACE_FL_MASKED_BITS << FTRACE_REF_MAX_SHIFT)
 #define FTRACE_REF_MAX		((1UL << FTRACE_REF_MAX_SHIFT) - 1)

2.6.2

From: Qiu Peiyang <[email protected]>

We hit ftrace_bug report when booting Android on a 64bit ATOM SOC chip. Basically, there is a race between insmod and ftrace_run_update_code.

After load_module=>ftrace_module_init, another thread jumps in to call ftrace_run_update_code=>ftrace_arch_code_modify_prepare
=>set_all_modules_text_rw, to change all modules
as RW. Since the new module is at MODULE_STATE_UNFORMED, the text attribute is not changed. Then, the 2nd thread goes ahead to change codes. However, load_module continues to call complete_formation=>set_section_ro_nx, then 2nd thread would fail when probing the module's TEXT.

The patch fixes it by using notifier to delay the enabling of ftrace records to the time when module is at state MODULE_STATE_COMING.

---
 kernel/trace/ftrace.c | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

index 23683b06b18c..eca592f977b2 100644
@@ -5037,29 +5037,36 @@ void ftrace_module_init(struct module *mod)
 
 	ftrace_process_locs(mod, mod->ftrace_callsites,
 			    mod->ftrace_callsites + mod->num_ftrace_callsites);
-	ftrace_module_enable(mod);
 }
 
-static int ftrace_module_notify_exit(struct notifier_block *self,
-				     unsigned long val, void *data)
+static int ftrace_module_notify(struct notifier_block *self,
+				unsigned long val, void *data)
 {
 	struct module *mod = data;
 
-	if (val == MODULE_STATE_GOING)
+	switch (val) {
+	case MODULE_STATE_COMING:
+		ftrace_module_enable(mod);
+		break;
+	case MODULE_STATE_GOING:
 		ftrace_release_mod(mod);
+		break;
+	default:
+		break;
+	}
 
 	return 0;
 }
 #else
-static int ftrace_module_notify_exit(struct notifier_block *self,
-				     unsigned long val, void *data)
+static int ftrace_module_notify(struct notifier_block *self,
+				unsigned long val, void *data)
 {
 	return 0;
 }
 #endif /* CONFIG_MODULES */
 
-struct notifier_block ftrace_module_exit_nb = {
-	.notifier_call = ftrace_module_notify_exit,
+struct notifier_block ftrace_module_nb = {
+	.notifier_call = ftrace_module_notify,
 	.priority = INT_MIN,	/* Run after anything that can remove kprobes */
 };
 

@@ -5091,7 +5098,7 @@ void __init ftrace_init(void)
 				  __start_mcount_loc,
 				  __stop_mcount_loc);
 
-	ret = register_module_notifier(&ftrace_module_exit_nb);
+	ret = register_module_notifier(&ftrace_module_nb);
 	if (ret)
 		pr_warning("Failed to register trace ftrace module exit notifier\n");
 

2.6.2