linux-kernel - Re: [RFC][PATCH 0/3] ftrace: Add dynamically allocated trampolines

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, 23 Jul 2014 19:05:52 +0200
From:	Oleg Nesterov <oleg@...hat.com>
To:	Steven Rostedt <rostedt@...dmis.org>
Cc:	linux-kernel@...r.kernel.org, Ingo Molnar <mingo@...nel.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>,
	Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>,
	Namhyung Kim <namhyung@...nel.org>,
	"H. Peter Anvin" <hpa@...or.com>,
	Josh Poimboeuf <jpoimboe@...hat.com>,
	Jiri Kosina <jkosina@...e.cz>,
	Seth Jennings <sjenning@...hat.com>,
	Jiri Slaby <jslaby@...e.cz>
Subject: Re: [RFC][PATCH 0/3] ftrace: Add dynamically allocated trampolines

On 07/23, Steven Rostedt wrote:
>
> On Wed, 23 Jul 2014 14:08:05 +0200
> Oleg Nesterov <oleg@...hat.com> wrote:
>
> > With this stupid patch
> >
> > 	--- a/kernel/trace/ftrace.c
> > 	+++ b/kernel/trace/ftrace.c
> > 	@@ -4464,6 +4464,7 @@ __ftrace_ops_list_func(unsigned long ip, unsigned long parent_ip,
> > 					printk("op=%p %pS\n", op, op);
> > 					goto out;
> > 				}
> > 	+			pr_crit("LIST_FUNC -> %pf()\n",  op->func);
> > 				op->func(ip, parent_ip, op, regs);
> > 			}
> > 		} while_for_each_ftrace_op(op);
> >
> > I do
> > 	# cd /sys/kernel/debug/tracing/
> > 	# echo "p:xx SyS_prctl+0x1c" >| kprobe_events
> > 	# cat ../kprobes/list
> > 	ffffffff81056c4c  k  SyS_prctl+0x1c    [DISABLED][FTRACE]
> > 	# echo 1 >| events/kprobes/xx/enable
> > 	#
> > 	# perl -e 'syscall 157,-1'
> > 	# dmesg
> > 	LIST_FUNC -> kprobe_ftrace_handler()
> >
> > so it is really called by the loop test code.
> >
> > And I guess that after your patches kprobe_ftrace_handler() should be called
> > from the trampoline in this case.
>
> No it wont be. Not until we have Paul McKenney's task_rcu code that
> will return after all tasks have either gone through userspace or a
> schedule. Hmm, maybe on a !CONFIG_PREEMPT it will be OK. Oh, I can have
> that be OK now on !CONFIG_PREEMPT. Maybe I'll do that too.
>
> kprobe ftrace_ops are allocated which sets the FTRACE_OPS_FL_DYNAMIC
> flag. You'll see that flag checked in update_ftrace_function(), and if
> it is set, it forces the ftrace_ops_list_func() to be used.

No? __register_ftrace_function() sets if !core_kernel_data(ops), and
kprobe_ftrace_ops is not dynamic?

> Why?
>
> [...snip..]

Yes, thanks, I understand why, at least to some degree.

> 	foo()
> 	  [mcount called --> ftrace_caller trampoline]
>            ftrace_caller
>              load ftrace_ops into parameter
>              <interrupt>
>              preempt_schedule()
>        [new task]
>        kfree(kprobe ftrace_ops);

see above.

And to be sure, I compiled your rfc/trampoline kernel which I pulled
yesterday with the same patch and did the same test. __ftrace_ops_list_func()
prints nothing.

So I also added WARN_ON(1) into kprobe_ftrace_handler() to ensure that
it is actually called, and yes, dmesg reports

	WARNING: ... kprobe_ftrace_handler+0x38/0x140()
	...
	Call Trace:
	 [<ffffffff8136a3eb>] dump_stack+0x5b/0xa8
	 [<ffffffff810423ec>] warn_slowpath_common+0x8c/0xc0
	 [<ffffffff8105772c>] ? SyS_prctl+0x1c/0x730
	 [<ffffffff8104243a>] warn_slowpath_null+0x1a/0x20
	 [<ffffffff810325c8>] kprobe_ftrace_handler+0x38/0x140
	 [<ffffffff8137148a>] ? retint_swapgs+0xe/0x13
	 [<ffffffff81057731>] ? SyS_prctl+0x21/0x730
	 [<ffffffff81057731>] ? SyS_prctl+0x21/0x730
	 [<ffffffff8122424e>] ? trace_hardirqs_on_thunk+0x3a/0x3f
	 [<ffffffff81370912>] ? system_call_fastpath+0x16/0x1b

after "perl -e 'syscall 157,-1'".

and, as expected, if I do "echo SyS_prctl >| set_ftrace_filter" and
"echo function >| current_tracer", then the command above also triggers
2 printk's in __ftrace_ops_list_func() :

	LIST_FUNC -> function_trace_call()
	LIST_FUNC -> kprobe_ftrace_handler()

so it seems that your patches can potentially buy more than you think ;)

Oleg.

--
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/