From: Earl Chew <earl_chew@yahoo.com>
Date: Tue, 8 Mar 2022 04:41:24 +0000 (-0800)
Subject: unshare: Fix PDEATHSIG race for --kill-child with --pid
X-Git-Tag: v2.39-rc1~744
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=1f248f73d851518fb84d8dd4a1b9b15619454c4b;p=thirdparty%2Futil-linux.git

unshare: Fix PDEATHSIG race for --kill-child with --pid

Exit the child explicitly should the parent terminate
just before the child invokes prctl(PR_SET_PDEATHSIG).

Note that if the child is created as pid 1 in a
new pid namespace, getppid(2) will return 0 because
the parent resides in a different namespace, and
SIGKILL will only be delivered if sent from the
ancestor namespace.

Instead of checking the pid of the parent, use a pidfd
instance to detect the termination of the parent.

The issue can be reproduced using the following script
which must be run as root to permite the --pid option.
The script will sometimes print 0..9 despite the parent
being killed:

while : ; do
cat /proc/uptime
sh -c '
  unshare --kill-child --pid --fork bash -c "
    for X in {0..9} ; do echo \$X || break ; sleep 1 ; done" &
  sleep 0 # sleep 0.00$RANDOM
  kill -9 $! 2>/dev/null' |
cat
done

Signed-off-by: Earl Chew <earl_chew@yahoo.com>
---

diff --git a/sys-utils/unshare.c b/sys-utils/unshare.c
index 5effff6b18..d35f1df274 100644
--- a/sys-utils/unshare.c
+++ b/sys-utils/unshare.c
@@ -20,6 +20,7 @@
 
 #include <errno.h>
 #include <getopt.h>
+#include <poll.h>
 #include <sched.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -40,6 +41,7 @@
 #include "caputils.h"
 #include "closestream.h"
 #include "namespace.h"
+#include "pidfd-utils.h"
 #include "exec_shell.h"
 #include "xalloc.h"
 #include "pathnames.h"
@@ -764,6 +766,7 @@ int main(int argc, char *argv[])
 	const char *newdir = NULL;
 	pid_t pid_bind = 0, pid_idmap = 0;
 	pid_t pid = 0;
+	int fd_parent_pid = -1;
 	int fd_idmap, fd_bind = -1;
 	sigset_t sigset, oldsigset;
 	int status;
@@ -956,6 +959,10 @@ int main(int argc, char *argv[])
 
 		/* force child forking before mountspace binding
 		 * so pid_for_children is populated */
+		fd_parent_pid = pidfd_open(getpid(), 0);
+		if (0 > fd_parent_pid)
+			err(EXIT_FAILURE, _("pidfd_open failed"));
+
 		pid = fork();
 
 		switch(pid) {
@@ -1007,8 +1014,31 @@ int main(int argc, char *argv[])
 		err(EXIT_FAILURE, _("child exit failed"));
 	}
 
-	if (kill_child_signo != 0 && prctl(PR_SET_PDEATHSIG, kill_child_signo) < 0)
-		err(EXIT_FAILURE, "prctl failed");
+	if (kill_child_signo != 0) {
+		if (prctl(PR_SET_PDEATHSIG, kill_child_signo) < 0)
+			err(EXIT_FAILURE, "prctl failed");
+
+		struct pollfd pollfds[1] = {
+			{
+				.fd = fd_parent_pid,
+				.events = POLLIN,
+			}
+                };
+
+		int nfds = poll(pollfds, 1, 0);
+		if (0 > nfds)
+			err(EXIT_FAILURE, "poll parent pidfd failed");
+
+		/* If the child was re-parented before prctl(2) was called,
+		 * the new parent will likely not be interested in the
+                 * precise exit status of the orphan.
+		 */
+		if (nfds)
+			exit(EXIT_FAILURE);
+	}
+
+	close(fd_parent_pid);
+	fd_parent_pid = -1;
 
         if (mapuser != (uid_t) -1 && !usermap)
 		map_id(_PATH_PROC_UIDMAP, mapuser, real_euid);