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: <20150114130300.75c5d2ce@gandalf.local.home>
Date:	Wed, 14 Jan 2015 13:03:00 -0500
From:	Steven Rostedt <rostedt@...dmis.org>
To:	Michael Ellerman <mpe@...erman.id.au>
Cc:	<linux-kernel@...r.kernel.org>, mingo@...hat.com,
	paulmck@...ux.vnet.ibm.com, Andrew Morton <akpm@...l.org>,
	tglx@...utronix.de, mathieu.desnoyers@...icios.com,
	xiakaixu@...wei.com
Subject: Re: [PATCH] tracing: Allow raw_syscall tracepoints to work from
 boot

On Wed, 14 Jan 2015 09:35:17 +1100
Michael Ellerman <mpe@...erman.id.au> wrote:

> In commit 5f893b2639b2 "tracing: Move enabling tracepoints to just after
> rcu_init()", tracing was enabled earlier in boot.
> 
> This broke tracing of the raw_syscall tracepoints from boot using the
> trace_event kernel parameter.
> 
> When the registration function for the raw_syscall tracepoints runs, it
> iterates over all tasks setting TIF_SYSCALL_TRACEPOINT. However now that
> this happens earlier in boot, the loop has no effect because there are
> no tasks in existence other than init_task, which is skipped by the
> for_each_process_thread() macro.
> 
> We can fix it by explicitly setting TIF_SYSCALL_TRACEPOINT for the
> init_task. That way when pid 1 is cloned from init_task it will inherit
> TIF_SYSCALL_TRACEPOINT.
> 
> Fixes: 5f893b2639b2 ("tracing: Move enabling tracepoints to just after rcu_init()")

I don't like setting the swap task flag for syscall tracing, as nothing
will unset it.


> Signed-off-by: Michael Ellerman <mpe@...erman.id.au>
> ---
>  kernel/tracepoint.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> 
> It feels a bit naughty to be whacking init_task like this, but it also
> seems like the right fix?

No, I tried the following instead.

> 
> Should we also clear it in the unregfunc? I can't see how that would
> ever be needed in practice?

It just seems hacky to set swapper in the first place.

Try my patch and let me know if it works for you?

-- Steve

>From aae3b6410d536d65204cce7654887095dc41e1cf Mon Sep 17 00:00:00 2001
From: "Steven Rostedt (Red Hat)" <rostedt@...dmis.org>
Date: Wed, 14 Jan 2015 12:53:45 -0500
Subject: [PATCH] tracing: Fix enabling of syscall events on the command line

Commit 5f893b2639b2 "tracing: Move enabling tracepoints to just after
rcu_init()" broke the enabling of system call events from the command
line. The reason was that the enabling of command line trace events
was moved before PID 1 started, and the syscall tracepoints require
that all tasks have the TIF_SYSCALL_TRACEPOINT flag set. But the
swapper task (pid 0) is not part of that. Since the swapper task is the
only task that is running at this early in boot, no task gets the
flag set, and the tracepoint never gets reached.

Instead of setting the swapper task flag (there should be no reason to
do that), re-enabled trace events again after the init thread (PID 1)
has been started. It requires disabling all command line events and
re-enabling them, as just enabling them again will not reset the logic
to set the TIF_SYSCALL_TRACEPOINT flag, as the syscall tracepoint will
be fooled into thinking that it was already set, and wont try setting
it again. For this reason, we must first disable it and re-enable it.

Link: http://lkml.kernel.org/r/1421188517-18312-1-git-send-email-mpe@ellerman.id.au

Reported-by: Michael Ellerman <mpe@...erman.id.au>
Signed-off-by: Steven Rostedt <rostedt@...dmis.org>
---
 kernel/trace/trace_events.c | 69 ++++++++++++++++++++++++++++++++++++---------
 1 file changed, 55 insertions(+), 14 deletions(-)

diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index 366a78a3e61e..b03a0ea77b99 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -2429,12 +2429,39 @@ static __init int event_trace_memsetup(void)
 	return 0;
 }
 
+static __init void
+early_enable_events(struct trace_array *tr, bool disable_first)
+{
+	char *buf = bootup_event_buf;
+	char *token;
+	int ret;
+
+	while (true) {
+		token = strsep(&buf, ",");
+
+		if (!token)
+			break;
+		if (!*token)
+			continue;
+
+		/* Restarting syscalls requires that we stop them first */
+		if (disable_first)
+			ftrace_set_clr_event(tr, token, 0);
+
+		ret = ftrace_set_clr_event(tr, token, 1);
+		if (ret)
+			pr_warn("Failed to enable trace event: %s\n", token);
+
+		/* Put back the comma to allow this to be called again */
+		if (buf)
+			*(buf - 1) = ',';
+	}
+}
+
 static __init int event_trace_enable(void)
 {
 	struct trace_array *tr = top_trace_array();
 	struct ftrace_event_call **iter, *call;
-	char *buf = bootup_event_buf;
-	char *token;
 	int ret;
 
 	if (!tr)
@@ -2456,18 +2483,7 @@ static __init int event_trace_enable(void)
 	 */
 	__trace_early_add_events(tr);
 
-	while (true) {
-		token = strsep(&buf, ",");
-
-		if (!token)
-			break;
-		if (!*token)
-			continue;
-
-		ret = ftrace_set_clr_event(tr, token, 1);
-		if (ret)
-			pr_warn("Failed to enable trace event: %s\n", token);
-	}
+	early_enable_events(tr, false);
 
 	trace_printk_start_comm();
 
@@ -2478,6 +2494,31 @@ static __init int event_trace_enable(void)
 	return 0;
 }
 
+/*
+ * event_trace_enable() is called from trace_event_init() first to
+ * initialize events and perhaps start any events that are on the
+ * command line. Unfortunately, there are some events that will not
+ * start this early, like the system call tracepoints that need
+ * to set the TIF_SYSCALL_TRACEPOINT flag of pid 1. But event_trace_enable()
+ * is called before pid 1 starts, and this flag is never set, making
+ * the syscall tracepoint never get reached, but the event is enabled
+ * regardless (and not doing anything).
+ */
+static __init int event_trace_enable_again(void)
+{
+	struct trace_array *tr;
+
+	tr = top_trace_array();
+	if (!tr)
+		return -ENODEV;
+
+	early_enable_events(tr, true);
+
+	return 0;
+}
+
+early_initcall(event_trace_enable_again);
+
 static __init int event_trace_init(void)
 {
 	struct trace_array *tr;
-- 
1.8.1.4

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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ