Prev: weird umem vs nfsd regression
Next: [GIT PULL 0/2] perf/urgent fixes for 2.6.35
From: Arnaldo Carvalho de Melo on 17 Jun 2010 16:40
From: Ian Munsie <imunsie(a)au1.ibm.com>

At exit, perf record will kill the process it was profiling by sending a
SIGTERM to child_pid (if it had been initialised), but in certain situations
child_pid may be 0 and perf would mistakenly kill more processes than intended.

child_pid is set to the return of fork() to either 0 or the pid of the child.
Ordinarily this would not present an issue as the child calls execvp to spawn
the process to be profiled and would therefore never run it's sig_atexit and
never attempt to kill pid 0.

However, if a nonexistant binary had been passed in to perf record the call to
execvp would fail and child_pid would be left set to 0. The child would then
exit and it's atexit handler, finding that child_pid was initialised to 0,
would call kill(0, SIGTERM), resulting in every process within it's process
group being killed.

In the case that perf was being run directly from the shell this typically
would not be an issue as the shell isolates the process. However, if perf was
being called from another program it could kill unexpected processes, which may
even include X.

This patch changes the logic of the test for whether child_pid was initialised
to only consider positive pids as valid, thereby never attempting to kill pid
0.

Cc: David S. Miller <davem(a)davemloft.net>
Cc: Frédéric Weisbecker <fweisbec(a)gmail.com>
Cc: Ingo Molnar <mingo(a)elte.hu>
Cc: Paul Mackerras <paulus(a)samba.org>
Cc: Peter Zijlstra <peterz(a)infradead.org>
Cc: Tom Zanussi <tzanussi(a)gmail.com>
LKML-Reference: <1276072680-17378-1-git-send-email-imunsie(a)au1.ibm.com>
Signed-off-by: Ian Munsie <imunsie(a)au1.ibm.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme(a)redhat.com>
---
tools/perf/builtin-record.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index dc3435e..711745f 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -193,7 +193,7 @@ static void sig_handler(int sig)

static void sig_atexit(void)
{
- if (child_pid != -1)
+ if (child_pid > 0)
kill(child_pid, SIGTERM);

if (signr == -1)
--
1.6.2.5

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
 | 
Pages: 1
Prev: weird umem vs nfsd regression
Next: [GIT PULL 0/2] perf/urgent fixes for 2.6.35