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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20191018074634.858645375@infradead.org>
Date:   Fri, 18 Oct 2019 09:35:41 +0200
From:   Peter Zijlstra <peterz@...radead.org>
To:     x86@...nel.org
Cc:     peterz@...radead.org, linux-kernel@...r.kernel.org,
        rostedt@...dmis.org, mhiramat@...nel.org, bristot@...hat.com,
        jbaron@...mai.com, torvalds@...ux-foundation.org,
        tglx@...utronix.de, mingo@...nel.org, namit@...are.com,
        hpa@...or.com, luto@...nel.org, ard.biesheuvel@...aro.org,
        jpoimboe@...hat.com, jeyu@...nel.org
Subject: [PATCH v4 16/16] ftrace: Merge ftrace_module_{init,enable}()

Because of how some architectures used set_all_modules_text_*() there
was a dependency between the module state and its memory protection
state. This then required ftrace to be split into two functions, see
commit:

  a949ae560a51 ("ftrace/module: Hardcode ftrace_module_init() call into load_module()")

Now that set_all_modules_text_*() is dead and burried, this is no
longer relevant and we can merge the ftrace_module hooks again.

Signed-off-by: Peter Zijlstra (Intel) <peterz@...radead.org>
Cc: Steven Rostedt <rostedt@...dmis.org>
---
 include/linux/ftrace.h |    2 
 kernel/module.c        |    3 -
 kernel/trace/ftrace.c  |  124 +++++++++++++++++++------------------------------
 3 files changed, 49 insertions(+), 80 deletions(-)

--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -579,7 +579,6 @@ static inline int ftrace_modify_call(str
 extern int ftrace_arch_read_dyn_info(char *buf, int size);
 
 extern int skip_trace(unsigned long ip);
-extern void ftrace_module_init(struct module *mod);
 extern void ftrace_module_enable(struct module *mod);
 extern void ftrace_release_mod(struct module *mod);
 
@@ -590,7 +589,6 @@ static inline int skip_trace(unsigned lo
 static inline int ftrace_force_update(void) { return 0; }
 static inline void ftrace_disable_daemon(void) { }
 static inline void ftrace_enable_daemon(void) { }
-static inline void ftrace_module_init(struct module *mod) { }
 static inline void ftrace_module_enable(struct module *mod) { }
 static inline void ftrace_release_mod(struct module *mod) { }
 static inline int ftrace_text_reserved(const void *start, const void *end)
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -3836,9 +3836,6 @@ static int load_module(struct load_info
 
 	dynamic_debug_setup(mod, info->debug, info->num_debug);
 
-	/* Ftrace init must be called in the MODULE_STATE_UNFORMED state */
-	ftrace_module_init(mod);
-
 	/* Finally it's fully formed, ready to start executing. */
 	err = complete_formation(mod, info);
 	if (err)
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -5564,6 +5564,8 @@ static int ftrace_cmp_ips(const void *a,
 	return 0;
 }
 
+static int referenced_filters(struct dyn_ftrace *rec);
+
 static int ftrace_process_locs(struct module *mod,
 			       unsigned long *start,
 			       unsigned long *end)
@@ -5656,6 +5658,49 @@ static int ftrace_process_locs(struct mo
 	ftrace_update_code(mod, start_pg);
 	if (!mod)
 		local_irq_restore(flags);
+
+#ifdef CONFIG_MODULES
+	if (ftrace_disabled || !mod)
+		goto out_loop;
+
+	do_for_each_ftrace_rec(pg, rec) {
+		int cnt;
+		/*
+		 * do_for_each_ftrace_rec() is a double loop.
+		 * module text shares the pg. If a record is
+		 * not part of this module, then skip this pg,
+		 * which the "break" will do.
+		 */
+		if (!within_module_core(rec->ip, mod) &&
+		    !within_module_init(rec->ip, mod))
+			break;
+
+		cnt = 0;
+
+		/*
+		 * When adding a module, we need to check if tracers are
+		 * currently enabled and if they are, and can trace this record,
+		 * we need to enable the module functions as well as update the
+		 * reference counts for those function records.
+		 */
+		if (ftrace_start_up)
+			cnt += referenced_filters(rec);
+
+		/* This clears FTRACE_FL_DISABLED */
+		rec->flags = cnt;
+
+		if (ftrace_start_up && cnt) {
+			int failed = __ftrace_replace_code(rec, 1);
+			if (failed) {
+				ftrace_bug(failed, rec);
+				goto out_loop;
+			}
+		}
+
+	} while_for_each_ftrace_rec();
+
+ out_loop:
+#endif
 	ret = 0;
  out:
 	mutex_unlock(&ftrace_lock);
@@ -5823,85 +5868,14 @@ void ftrace_release_mod(struct module *m
 
 void ftrace_module_enable(struct module *mod)
 {
-	struct dyn_ftrace *rec;
-	struct ftrace_page *pg;
-
-	mutex_lock(&ftrace_lock);
-
-	if (ftrace_disabled)
-		goto out_unlock;
-
-	/*
-	 * If the tracing is enabled, go ahead and enable the record.
-	 *
-	 * The reason not to enable the record immediately is the
-	 * inherent check of ftrace_make_nop/ftrace_make_call for
-	 * correct previous instructions.  Making first the NOP
-	 * conversion puts the module to the correct state, thus
-	 * passing the ftrace_make_call check.
-	 *
-	 * We also delay this to after the module code already set the
-	 * text to read-only, as we now need to set it back to read-write
-	 * so that we can modify the text.
-	 */
-	if (ftrace_start_up)
-		ftrace_arch_code_modify_prepare();
-
-	do_for_each_ftrace_rec(pg, rec) {
-		int cnt;
-		/*
-		 * do_for_each_ftrace_rec() is a double loop.
-		 * module text shares the pg. If a record is
-		 * not part of this module, then skip this pg,
-		 * which the "break" will do.
-		 */
-		if (!within_module_core(rec->ip, mod) &&
-		    !within_module_init(rec->ip, mod))
-			break;
-
-		cnt = 0;
-
-		/*
-		 * When adding a module, we need to check if tracers are
-		 * currently enabled and if they are, and can trace this record,
-		 * we need to enable the module functions as well as update the
-		 * reference counts for those function records.
-		 */
-		if (ftrace_start_up)
-			cnt += referenced_filters(rec);
-
-		/* This clears FTRACE_FL_DISABLED */
-		rec->flags = cnt;
-
-		if (ftrace_start_up && cnt) {
-			int failed = __ftrace_replace_code(rec, 1);
-			if (failed) {
-				ftrace_bug(failed, rec);
-				goto out_loop;
-			}
-		}
-
-	} while_for_each_ftrace_rec();
-
- out_loop:
-	if (ftrace_start_up)
-		ftrace_arch_code_modify_post_process();
-
- out_unlock:
-	mutex_unlock(&ftrace_lock);
+	if (!(ftrace_disabled || !mod->num_ftrace_callsites)) {
+		ftrace_process_locs(mod, mod->ftrace_callsites,
+				    mod->ftrace_callsites + mod->num_ftrace_callsites);
+	}
 
 	process_cached_mods(mod->name);
 }
 
-void ftrace_module_init(struct module *mod)
-{
-	if (ftrace_disabled || !mod->num_ftrace_callsites)
-		return;
-
-	ftrace_process_locs(mod, mod->ftrace_callsites,
-			    mod->ftrace_callsites + mod->num_ftrace_callsites);
-}
-
 static void save_ftrace_mod_rec(struct ftrace_mod_map *mod_map,
 				struct dyn_ftrace *rec)
 {


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ