ftrace: Fix function_graph tracer interaction with BPF trampoline

Depending on type of BPF programs served by BPF trampoline it can call original function. In such case the trampoline will skip one stack frame while returning. That will confuse function_graph tracer and will cause crashes with bad RIP. Teach graph tracer to skip functions that have BPF trampoline attached.

Alexei Starovoitov committed 6 years ago

Commit ff205761 parent a61f8104 files · +21 −26Browse files

Unified · Split

4 files

arch/x86/kernel/ftrace.c +0 −14
include/linux/ftrace.h +5 −0
kernel/trace/fgraph.c +9 −0
kernel/trace/ftrace.c +7 −12

arch/x86/kernel/ftrace.c+0−14

@@ -1042,20 +1042,6 @@ void prepare_ftrace_return(unsigned long self_addr, unsigned long *parent, · 1041 unmodified lines
1042	if (unlikely(atomic_read(&current->tracing_graph_pause)))	1042	if (unlikely(atomic_read(&current->tracing_graph_pause)))
1043	return;	1043	return;
1044		1044
1045	/*
1046	* If the return location is actually pointing directly to
1047	* the start of a direct trampoline (if we trace the trampoline
1048	* it will still be offset by MCOUNT_INSN_SIZE), then the
1049	* return address is actually off by one word, and we
1050	* need to adjust for that.
1051	*/
1052	if (ftrace_direct_func_count) {
1053	if (ftrace_find_direct_func(self_addr + MCOUNT_INSN_SIZE)) {
1054	self_addr = *parent;
1055	parent++;
1056	}
1057	}
1058
1059	/*	1045	/*
1060	* Protect against fault, even if it shouldn't	1046	* Protect against fault, even if it shouldn't
1061	* happen. This tool is too much intrusive to	1047	* happen. This tool is too much intrusive to

include/linux/ftrace.h+5−0

 				struct dyn_ftrace *rec,
 				unsigned long old_addr,
 				unsigned long new_addr);
+unsigned long ftrace_find_rec_direct(unsigned long ip);
 #else
 # define ftrace_direct_func_count 0
 static inline int register_ftrace_direct(unsigned long ip, unsigned long addr)
 {
 	return -ENODEV;
 }
+static inline unsigned long ftrace_find_rec_direct(unsigned long ip)
+{
+	return 0;
+}
 #endif /* CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS */
 #ifndef CONFIG_HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS

kernel/trace/fgraph.c+9−0

@@ -101,6 +101,15 @@ int function_graph_enter(unsigned long ret, unsigned long func, · 100 unmodified lines
101	{	101	{
102	struct ftrace_graph_ent trace;	102	struct ftrace_graph_ent trace;
103		103
		104	/*
		105	* Skip graph tracing if the return location is served by direct trampoline,
		106	* since call sequence and return addresses is unpredicatable anymore.
		107	* Ex: BPF trampoline may call original function and may skip frame
		108	* depending on type of BPF programs attached.
		109	*/
		110	if (ftrace_direct_func_count &&
		111	ftrace_find_rec_direct(ret - MCOUNT_INSN_SIZE))
		112	return -EBUSY;
104	trace.func = func;	113	trace.func = func;
105	trace.depth = ++current->curr_ret_depth;	114	trace.depth = ++current->curr_ret_depth;
106		115

kernel/trace/ftrace.c+7−12

  * Search the direct_functions hash to see if the given instruction pointer
  * has a direct caller attached to it.
  */
-static unsigned long find_rec_direct(unsigned long ip)
+unsigned long ftrace_find_rec_direct(unsigned long ip)
 {
 	struct ftrace_func_entry *entry;
 {
 	unsigned long addr;
-	addr = find_rec_direct(ip);
+	addr = ftrace_find_rec_direct(ip);
 	if (!addr)
 		return;
 			  | FTRACE_OPS_FL_DIRECT | FTRACE_OPS_FL_SAVE_REGS
 			  | FTRACE_OPS_FL_PERMANENT,
 };
-#else
-static inline unsigned long find_rec_direct(unsigned long ip)
-{
-	return 0;
-}
 #endif /* CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS */
 /**
 	if ((rec->flags & FTRACE_FL_DIRECT) &&
 	    (ftrace_rec_count(rec) == 1)) {
-		addr = find_rec_direct(rec->ip);
+		addr = ftrace_find_rec_direct(rec->ip);
 		if (addr)
 			return addr;
 		WARN_ON_ONCE(1);
 	/* Direct calls take precedence over trampolines */
 	if (rec->flags & FTRACE_FL_DIRECT_EN) {
-		addr = find_rec_direct(rec->ip);
+		addr = ftrace_find_rec_direct(rec->ip);
 		if (addr)
 			return addr;
 		WARN_ON_ONCE(1);
 		if (rec->flags & FTRACE_FL_DIRECT) {
 			unsigned long direct;
-			direct = find_rec_direct(rec->ip);
+			direct = ftrace_find_rec_direct(rec->ip);
 			if (direct)
 				seq_printf(m, "\n\tdirect-->%pS", (void *)direct);
 		}
 	mutex_lock(&direct_mutex);
 	/* See if there's a direct function at @ip already */
-	if (find_rec_direct(ip))
+	if (ftrace_find_rec_direct(ip))
 		goto out_unlock;
 	ret = -ENODEV;
 	if (ip != rec->ip) {
 		ip = rec->ip;
 		/* Need to check this ip for a direct. */
-		if (find_rec_direct(ip))
+		if (ftrace_find_rec_direct(ip))
 			goto out_unlock;
 	}

ftrace: Fix function_graph tracer interaction with BPF trampoline

Keyboard shortcuts

General

Issues and pull requests

Issues, pull requests and Inbox

Repository code