[gentoo-portage-dev] [PATCH] pid-ns-init: fix child process signal disposition (bug 675828)

[gentoo-portage-dev] [PATCH] pid-ns-init: fix child process signal disposition (bug 675828)

[Zac Medico]

Use subprocess.Popen to correctly configure the signal disposition
of the child process, since os.fork leaves the signal disposition
in a state which may be inappropriate for various signals including
SIGPIPE, SIGQUIT, SIGTERM, and SIGINT. For python implementations
other that CPython >= 3, use preexec_fn to manually configure the
signal disposition (I have found that this is necessary for CPython
2.7 and all PyPy versions tested, including PyPy3).

Bug: https://bugs.gentoo.org/675828
Signed-off-by: Zac Medico <zmedico@gentoo.org>
---
 bin/pid-ns-init | 31 ++++++++++++++++++++++++++++---
 1 file changed, 28 insertions(+), 3 deletions(-)

diff --git a/bin/pid-ns-init b/bin/pid-ns-init
index 182d00a43..86029f983 100644
--- a/bin/pid-ns-init
+++ b/bin/pid-ns-init
@@ -2,18 +2,37 @@
 # Copyright 2018-2019 Gentoo Authors
 # Distributed under the terms of the GNU General Public License v2
 
+import errno
 import functools
 import os
+import platform
 import signal
+import subprocess
 import sys
 
 
+if sys.version_info.major < 3 or platform.python_implementation() != 'CPython':
+	def signal_disposition_preexec():
+		for signum in (
+			signal.SIGHUP,
+			signal.SIGINT,
+			signal.SIGPIPE,
+			signal.SIGQUIT,
+			signal.SIGTERM,
+			):
+			signal.signal(signum, signal.SIG_DFL)
+else:
+	# CPython >= 3 subprocess.Popen handles this internally.
+	signal_disposition_preexec = None
+
+
 KILL_SIGNALS = (
 	signal.SIGINT,
 	signal.SIGTERM,
 	signal.SIGHUP,
 )
 
+
 def forward_kill_signal(main_child_pid, signum, frame):
 	os.kill(main_child_pid, signum)
 
@@ -28,14 +47,15 @@ def main(argv):
 		# (forwarding signals to init and forwarding exit status to the parent
 		# process).
 		main_child_pid = int(argv[1])
+		proc = None
 	else:
 		# The current process is init (pid 1) in a child pid namespace.
 		binary = argv[1]
 		args = argv[2:]
 
-		main_child_pid = os.fork()
-		if main_child_pid == 0:
-			os.execv(binary, args)
+		proc = subprocess.Popen(args, executable=binary,
+			preexec_fn=signal_disposition_preexec)
+		main_child_pid = proc.pid
 
 	sig_handler = functools.partial(forward_kill_signal, main_child_pid)
 	for signum in KILL_SIGNALS:
@@ -50,6 +70,11 @@ def main(argv):
 				continue
 			raise
 		if pid == main_child_pid:
+			if proc is not None:
+				# Suppress warning messages like this:
+				# ResourceWarning: subprocess 1234 is still running
+				proc.returncode = 0
+
 			if os.WIFEXITED(status):
 				return os.WEXITSTATUS(status)
 			elif os.WIFSIGNALED(status):
-- 
2.18.1

[gentoo-portage-dev] [PATCH v2] pid-ns-init: fix child process signal disposition (bug 675828)

[Zac Medico]

Use subprocess.Popen to correctly configure the signal disposition
of the child process, since os.fork leaves the signal disposition
in a state which may be inappropriate for various signals including
SIGPIPE, SIGQUIT, SIGTERM, and SIGINT. For python implementations
other that CPython >= 3, use preexec_fn to manually configure the
signal disposition (I have found that this is necessary for CPython
2.7 and all PyPy versions tested, including PyPy3).

Bug: https://bugs.gentoo.org/675828
Signed-off-by: Zac Medico <zmedico@gentoo.org>
---
[PATCH v2] fix to use Popen pass_fds parameter when appropriate

 bin/pid-ns-init        | 41 +++++++++++++++++++++++++++++++++++------
 lib/portage/process.py |  1 +
 2 files changed, 36 insertions(+), 6 deletions(-)

diff --git a/bin/pid-ns-init b/bin/pid-ns-init
index 182d00a43..271a32855 100644
--- a/bin/pid-ns-init
+++ b/bin/pid-ns-init
@@ -2,25 +2,44 @@
 # Copyright 2018-2019 Gentoo Authors
 # Distributed under the terms of the GNU General Public License v2
 
+import errno
 import functools
 import os
+import platform
 import signal
+import subprocess
 import sys
 
 
+if sys.version_info.major < 3 or platform.python_implementation() != 'CPython':
+	def signal_disposition_preexec():
+		for signum in (
+			signal.SIGHUP,
+			signal.SIGINT,
+			signal.SIGPIPE,
+			signal.SIGQUIT,
+			signal.SIGTERM,
+			):
+			signal.signal(signum, signal.SIG_DFL)
+else:
+	# CPython >= 3 subprocess.Popen handles this internally.
+	signal_disposition_preexec = None
+
+
 KILL_SIGNALS = (
 	signal.SIGINT,
 	signal.SIGTERM,
 	signal.SIGHUP,
 )
 
+
 def forward_kill_signal(main_child_pid, signum, frame):
 	os.kill(main_child_pid, signum)
 
 
 def main(argv):
 	if len(argv) < 2:
-		return 'Usage: {} <main-child-pid> or <binary> <argv0> [arg]..'.format(argv[0])
+		return 'Usage: {} <main-child-pid> or <pass_fds> <binary> <argv0> [arg]..'.format(argv[0])
 
 	if len(argv) == 2:
 		# The child process is init (pid 1) in a child pid namespace, and
@@ -28,14 +47,19 @@ def main(argv):
 		# (forwarding signals to init and forwarding exit status to the parent
 		# process).
 		main_child_pid = int(argv[1])
+		proc = None
 	else:
 		# The current process is init (pid 1) in a child pid namespace.
-		binary = argv[1]
-		args = argv[2:]
+		pass_fds = [int(fd) for fd in argv[1].split(',')]
+		binary = argv[2]
+		args = argv[3:]
 
-		main_child_pid = os.fork()
-		if main_child_pid == 0:
-			os.execv(binary, args)
+		popen_kwargs = {}
+		if sys.version_info.major > 2:
+			popen_kwargs['pass_fds'] = pass_fds
+		proc = subprocess.Popen(args, executable=binary,
+			preexec_fn=signal_disposition_preexec, **popen_kwargs)
+		main_child_pid = proc.pid
 
 	sig_handler = functools.partial(forward_kill_signal, main_child_pid)
 	for signum in KILL_SIGNALS:
@@ -50,6 +74,11 @@ def main(argv):
 				continue
 			raise
 		if pid == main_child_pid:
+			if proc is not None:
+				# Suppress warning messages like this:
+				# ResourceWarning: subprocess 1234 is still running
+				proc.returncode = 0
+
 			if os.WIFEXITED(status):
 				return os.WEXITSTATUS(status)
 			elif os.WIFSIGNALED(status):
diff --git a/lib/portage/process.py b/lib/portage/process.py
index 6af3ac37d..dd3d58ddc 100644
--- a/lib/portage/process.py
+++ b/lib/portage/process.py
@@ -571,6 +571,7 @@ def _exec(binary, mycommand, opt_name, fd_pipes,
 									portage._python_interpreter,
 									os.path.join(portage._bin_path,
 										'pid-ns-init'),
+									_unicode_encode(','.join(str(fd) for fd in fd_pipes)),
 									binary] + myargs
 							else:
 								# Execute a supervisor process which will forward
-- 
2.18.1