linux-kernel - [PATCH 1/2 v2] ftrace: don't allow IPMODIFY without proper compiler support

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]

Date:	Wed, 14 Jan 2015 09:47:50 +0100 (CET)
From:	Jiri Kosina <jkosina@...e.cz>
To:	Steven Rostedt <rostedt@...dmis.org>,
	Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>
cc:	Andrew Morton <akpm@...ux-foundation.org>,
	Stephen Rothwell <sfr@...b.auug.org.au>,
	Josh Poimboeuf <jpoimboe@...hat.com>,
	Christoph Hellwig <hch@...radead.org>,
	linux-kernel@...r.kernel.org, live-patching@...r.kernel.org,
	linux-next@...r.kernel.org
Subject: [PATCH 1/2 v2] ftrace: don't allow IPMODIFY without proper compiler
 support

Using IPMODIFY needs to be allowed only with compilers which are 
guaranteed to generate function prologues compatible with function 
redirection through changing instruction pointer in saved regs.

For example changing regs->ip on x86_64 in cases when gcc is using mcount 
(and not fentry) is not allowed, because at the time mcount call is 
issued, the original function's prologue has already been executed, which 
leads to all kinds of inconsistent havoc.

There is currently no way to express dependency on gcc features in 
Kconfig, (CC_USING_FENTRY is defined only during build, so it's not 
possible for Kconfig symbol to depend on it) so this needs to be checked 
in runtime.

Mark x86_64 with fentry supported for now. Other archs can be added
gradually.

Signed-off-by: Jiri Kosina <jkosina@...e.cz>
---

v1 -> v2: turn macro function into a plain macro

 arch/x86/include/asm/ftrace.h | 2 ++
 include/linux/ftrace.h        | 5 +++++
 kernel/trace/ftrace.c         | 5 +++++
 3 files changed, 12 insertions(+)

diff --git a/arch/x86/include/asm/ftrace.h b/arch/x86/include/asm/ftrace.h
index f45acad..f84eaaf 100644
--- a/arch/x86/include/asm/ftrace.h
+++ b/arch/x86/include/asm/ftrace.h
@@ -4,8 +4,10 @@
 #ifdef CONFIG_FUNCTION_TRACER
 #ifdef CC_USING_FENTRY
 # define MCOUNT_ADDR		((long)(__fentry__))
+# define ftrace_ipmodify_supported 1
 #else
 # define MCOUNT_ADDR		((long)(mcount))
+# define ftrace_ipmodify_supported 0
 #endif
 #define MCOUNT_INSN_SIZE	5 /* sizeof mcount call */
 
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index 1da6029..837153a 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -244,6 +244,11 @@ static inline int ftrace_function_local_disabled(struct ftrace_ops *ops)
 extern void ftrace_stub(unsigned long a0, unsigned long a1,
 			struct ftrace_ops *op, struct pt_regs *regs);
 
+#ifndef ftrace_ipmodify_supported
+/* let's not make any implicit assumptions about profiling call placement */
+# define ftrace_ipmodify_supported 0
+
+#endif
 #else /* !CONFIG_FUNCTION_TRACER */
 /*
  * (un)register_ftrace_function must be a macro since the ops parameter
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 929a733..05af4cd 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -1809,6 +1809,11 @@ static int __ftrace_hash_update_ipmodify(struct ftrace_ops *ops,
 	if (!(ops->flags & FTRACE_OPS_FL_IPMODIFY))
 		return 0;
 
+	if (!ftrace_ipmodify_supported) {
+		WARN(1, "Your compiler doesn't support features necessary for IPMODIFY");
+		return 0;
+	}
+
 	/*
 	 * Since the IPMODIFY is a very address sensitive action, we do not
 	 * allow ftrace_ops to set all functions to new hash.

-- 
Jiri Kosina
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/