* [PATCH] perf stat: avoid sending SIGTERM to random processes
@ 2013-06-04 15:44 Stephane Eranian
2013-06-19 6:46 ` Stephane Eranian
2013-07-12 8:50 ` [tip:perf/urgent] perf stat: Avoid " tip-bot for Stephane Eranian
0 siblings, 2 replies; 3+ messages in thread
From: Stephane Eranian @ 2013-06-04 15:44 UTC (permalink / raw)
To: linux-kernel; +Cc: peterz, mingo, acme, jolsa, namhyung
This patch fixes a problem with perf stat whereby on termination
it may send a SIGTERM signal to random processes on systems
with high PID recycling. I got some actual bug reports on this.
There is race between the SIGCHLD and sig_atexit() handlers.
This patch addresses this problem by clearing child_pid in the
SIGCHLD handler.
Signed-off-by: Stephane Eranian <eranian@google.com>
---
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 7e910ba..95768af 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -87,7 +87,7 @@ static int run_count = 1;
static bool no_inherit = false;
static bool scale = true;
static enum aggr_mode aggr_mode = AGGR_GLOBAL;
-static pid_t child_pid = -1;
+static volatile pid_t child_pid = -1;
static bool null_run = false;
static int detailed_run = 0;
static bool big_num = true;
@@ -1148,13 +1148,34 @@ static void skip_signal(int signo)
done = 1;
signr = signo;
+ /*
+ * render child_pid harmless
+ * won't send SIGTERM to a random
+ * process in case of race condition
+ * and fast PID recycling
+ */
+ child_pid = -1;
}
static void sig_atexit(void)
{
+ sigset_t set, oset;
+
+ /*
+ * avoid race condition with SIGCHLD handler
+ * in skip_signal() which is modifying child_pid
+ * goal is to avoid send SIGTERM to a random
+ * process
+ */
+ sigemptyset(&set);
+ sigaddset(&set, SIGCHLD);
+ sigprocmask(SIG_BLOCK, &set, &oset);
+
if (child_pid != -1)
kill(child_pid, SIGTERM);
+ sigprocmask(SIG_SETMASK, &oset, NULL);
+
if (signr == -1)
return;
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] perf stat: avoid sending SIGTERM to random processes
2013-06-04 15:44 [PATCH] perf stat: avoid sending SIGTERM to random processes Stephane Eranian
@ 2013-06-19 6:46 ` Stephane Eranian
2013-07-12 8:50 ` [tip:perf/urgent] perf stat: Avoid " tip-bot for Stephane Eranian
1 sibling, 0 replies; 3+ messages in thread
From: Stephane Eranian @ 2013-06-19 6:46 UTC (permalink / raw)
To: LKML
Cc: Peter Zijlstra, mingo@elte.hu, Arnaldo Carvalho de Melo,
Jiri Olsa, Namhyung Kim
Any comment on this patch Arnaldo?
On Tue, Jun 4, 2013 at 5:44 PM, Stephane Eranian <eranian@google.com> wrote:
>
> This patch fixes a problem with perf stat whereby on termination
> it may send a SIGTERM signal to random processes on systems
> with high PID recycling. I got some actual bug reports on this.
>
> There is race between the SIGCHLD and sig_atexit() handlers.
> This patch addresses this problem by clearing child_pid in the
> SIGCHLD handler.
>
> Signed-off-by: Stephane Eranian <eranian@google.com>
> ---
>
> diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
> index 7e910ba..95768af 100644
> --- a/tools/perf/builtin-stat.c
> +++ b/tools/perf/builtin-stat.c
> @@ -87,7 +87,7 @@ static int run_count = 1;
> static bool no_inherit = false;
> static bool scale = true;
> static enum aggr_mode aggr_mode = AGGR_GLOBAL;
> -static pid_t child_pid = -1;
> +static volatile pid_t child_pid = -1;
> static bool null_run = false;
> static int detailed_run = 0;
> static bool big_num = true;
> @@ -1148,13 +1148,34 @@ static void skip_signal(int signo)
> done = 1;
>
> signr = signo;
> + /*
> + * render child_pid harmless
> + * won't send SIGTERM to a random
> + * process in case of race condition
> + * and fast PID recycling
> + */
> + child_pid = -1;
> }
>
> static void sig_atexit(void)
> {
> + sigset_t set, oset;
> +
> + /*
> + * avoid race condition with SIGCHLD handler
> + * in skip_signal() which is modifying child_pid
> + * goal is to avoid send SIGTERM to a random
> + * process
> + */
> + sigemptyset(&set);
> + sigaddset(&set, SIGCHLD);
> + sigprocmask(SIG_BLOCK, &set, &oset);
> +
> if (child_pid != -1)
> kill(child_pid, SIGTERM);
>
> + sigprocmask(SIG_SETMASK, &oset, NULL);
> +
> if (signr == -1)
> return;
>
^ permalink raw reply [flat|nested] 3+ messages in thread* [tip:perf/urgent] perf stat: Avoid sending SIGTERM to random processes
2013-06-04 15:44 [PATCH] perf stat: avoid sending SIGTERM to random processes Stephane Eranian
2013-06-19 6:46 ` Stephane Eranian
@ 2013-07-12 8:50 ` tip-bot for Stephane Eranian
1 sibling, 0 replies; 3+ messages in thread
From: tip-bot for Stephane Eranian @ 2013-07-12 8:50 UTC (permalink / raw)
To: linux-tip-commits
Cc: acme, linux-kernel, eranian, hpa, mingo, peterz, namhyung, jolsa,
tglx, mingo
Commit-ID: d07f0b120642f442d81a61f68a9325fb7717004f
Gitweb: http://git.kernel.org/tip/d07f0b120642f442d81a61f68a9325fb7717004f
Author: Stephane Eranian <eranian@google.com>
AuthorDate: Tue, 4 Jun 2013 17:44:26 +0200
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 8 Jul 2013 17:36:33 -0300
perf stat: Avoid sending SIGTERM to random processes
This patch fixes a problem with perf stat whereby on termination it may
send a SIGTERM signal to random processes on systems with high PID
recycling. I got some actual bug reports on this.
There is race between the SIGCHLD and sig_atexit() handlers. This patch
addresses this problem by clearing child_pid in the SIGCHLD handler.
Signed-off-by: Stephane Eranian <eranian@google.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/20130604154426.GA2928@quad
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/builtin-stat.c | 23 ++++++++++++++++++++++-
1 file changed, 22 insertions(+), 1 deletion(-)
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 7e910ba..95768af 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -87,7 +87,7 @@ static int run_count = 1;
static bool no_inherit = false;
static bool scale = true;
static enum aggr_mode aggr_mode = AGGR_GLOBAL;
-static pid_t child_pid = -1;
+static volatile pid_t child_pid = -1;
static bool null_run = false;
static int detailed_run = 0;
static bool big_num = true;
@@ -1148,13 +1148,34 @@ static void skip_signal(int signo)
done = 1;
signr = signo;
+ /*
+ * render child_pid harmless
+ * won't send SIGTERM to a random
+ * process in case of race condition
+ * and fast PID recycling
+ */
+ child_pid = -1;
}
static void sig_atexit(void)
{
+ sigset_t set, oset;
+
+ /*
+ * avoid race condition with SIGCHLD handler
+ * in skip_signal() which is modifying child_pid
+ * goal is to avoid send SIGTERM to a random
+ * process
+ */
+ sigemptyset(&set);
+ sigaddset(&set, SIGCHLD);
+ sigprocmask(SIG_BLOCK, &set, &oset);
+
if (child_pid != -1)
kill(child_pid, SIGTERM);
+ sigprocmask(SIG_SETMASK, &oset, NULL);
+
if (signr == -1)
return;
^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2013-07-12 8:51 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-06-04 15:44 [PATCH] perf stat: avoid sending SIGTERM to random processes Stephane Eranian
2013-06-19 6:46 ` Stephane Eranian
2013-07-12 8:50 ` [tip:perf/urgent] perf stat: Avoid " tip-bot for Stephane Eranian
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.