[lxc-devel] [lxc/master] start: handle setting pdeath signal in new pidns

brauner on Github lxc-bot at linuxcontainers.org
Sat Apr 13 14:46:29 UTC 2019


A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 1165 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20190413/bff7a716/attachment.bin>
-------------- next part --------------
From 37f9b1ff6d499b4fca2453f1cf1cf04cc15b3b60 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Sat, 13 Apr 2019 16:41:30 +0200
Subject: [PATCH] start: handle setting pdeath signal in new pidns

In the usual case the child runs in a separate pid namespace. So far we haven't
been able to reliably set the pdeath signal. When we set the pdeath signal we
need to verify that we haven't lost a race whereby we have been orphaned and
though we have set a pdeath signal it won't help us since, well, the parent is
dead.
We were able to correctly handle this case when we were in the same pidns since
getppid() will return a valid pid. When we are in a separate pidns 0 will be
returned since the parent doesn't exist in our pidns.
A while back, while Jann (Horn) and I were discussing other things he came up
with a nifty idea: simply pass an fd for the parent's status file and check the
"State:" field. This is the implementation of that idea.

Suggested-by: Jann Horn <jann at thejh.net>
Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
 src/lxc/start.c | 20 ++++++++++++++---
 src/lxc/start.h |  2 ++
 src/lxc/utils.c | 57 +++++++++++++++++++++++++++++++++++++++++++------
 src/lxc/utils.h |  2 +-
 4 files changed, 71 insertions(+), 10 deletions(-)

diff --git a/src/lxc/start.c b/src/lxc/start.c
index 46a92d3d46..6b8d918f86 100644
--- a/src/lxc/start.c
+++ b/src/lxc/start.c
@@ -681,6 +681,9 @@ void lxc_free_handler(struct lxc_handler *handler)
 		if (handler->conf->maincmd_fd >= 0)
 			lxc_abstract_unix_close(handler->conf->maincmd_fd);
 
+	if (handler->monitor_status_fd >= 0)
+		close(handler->monitor_status_fd);
+
 	if (handler->state_socket_pair[0] >= 0)
 		close(handler->state_socket_pair[0]);
 
@@ -765,11 +768,18 @@ struct lxc_handler *lxc_init_handler(const char *name, struct lxc_conf *conf,
 
 int lxc_init(const char *name, struct lxc_handler *handler)
 {
+	__do_close_prot_errno int status_fd = -EBADF;
 	int ret;
 	const char *loglevel;
 	struct lxc_conf *conf = handler->conf;
 
 	handler->monitor_pid = lxc_raw_getpid();
+	status_fd = open("/proc/self/status", O_RDONLY | O_CLOEXEC);
+	if (status_fd < 0) {
+		SYSERROR("Failed to open monitor status fd");
+		goto out_close_maincmd_fd;
+	}
+	handler->monitor_status_fd = move_fd(status_fd);
 
 	lsm_init();
 	TRACE("Initialized LSM");
@@ -1112,7 +1122,8 @@ static int do_start(void *data)
 	 * exit before we set the pdeath signal leading to a unsupervized
 	 * container.
 	 */
-	ret = lxc_set_death_signal(SIGKILL, handler->monitor_pid);
+	ret = lxc_set_death_signal(SIGKILL, handler->monitor_pid,
+				   handler->monitor_status_fd);
 	if (ret < 0) {
 		SYSERROR("Failed to set PR_SET_PDEATHSIG to SIGKILL");
 		goto out_warn_father;
@@ -1190,7 +1201,8 @@ static int do_start(void *data)
 			goto out_warn_father;
 
 		/* set{g,u}id() clears deathsignal */
-		ret = lxc_set_death_signal(SIGKILL, handler->monitor_pid);
+		ret = lxc_set_death_signal(SIGKILL, handler->monitor_pid,
+					   handler->monitor_status_fd);
 		if (ret < 0) {
 			SYSERROR("Failed to set PR_SET_PDEATHSIG to SIGKILL");
 			goto out_warn_father;
@@ -1438,7 +1450,9 @@ static int do_start(void *data)
 	}
 
 	if (handler->conf->monitor_signal_pdeath != SIGKILL) {
-		ret = lxc_set_death_signal(handler->conf->monitor_signal_pdeath, handler->monitor_pid);
+		ret = lxc_set_death_signal(handler->conf->monitor_signal_pdeath,
+					   handler->monitor_pid,
+					   handler->monitor_status_fd);
 		if (ret < 0) {
 			SYSERROR("Failed to set PR_SET_PDEATHSIG to %d",
 				 handler->conf->monitor_signal_pdeath);
diff --git a/src/lxc/start.h b/src/lxc/start.h
index 60607ccc12..7c40f123e3 100644
--- a/src/lxc/start.h
+++ b/src/lxc/start.h
@@ -105,6 +105,8 @@ struct lxc_handler {
 	/* The monitor's pid. */
 	pid_t monitor_pid;
 
+	int monitor_status_fd;
+
 	/* Whether the child has already exited. */
 	bool init_died;
 
diff --git a/src/lxc/utils.c b/src/lxc/utils.c
index ea081c566c..83c3b8e05a 100644
--- a/src/lxc/utils.c
+++ b/src/lxc/utils.c
@@ -1711,7 +1711,48 @@ uint64_t lxc_find_next_power2(uint64_t n)
 	return n;
 }
 
-int lxc_set_death_signal(int signal, pid_t parent)
+static int process_dead(int status_fd)
+{
+	__do_close_prot_errno int dupfd = -EBADF;
+	__do_free char *line = NULL;
+	__do_fclose FILE *f = NULL;
+	int ret = 0;
+	size_t n = 0;
+
+	/*
+	 * Create a duplicate of the file descriptor sinc we use fdopen() on it
+	 * futher below. fdopen() transfers ownership of the fd to it's FILE *.
+	 */
+	dupfd = dup(status_fd);
+	if (dupfd < 0)
+		return -1;
+
+	if (fd_cloexec(status_fd, true) < 0)
+		return -1;
+
+	f = fdopen(status_fd, "r");
+	if (!f)
+		return -1;
+	move_fd(dupfd);
+
+	ret = 0;
+	while (getline(&line, &n, f) != -1) {
+		char *state;
+
+		if (strncmp(line, "State:", 6))
+			continue;
+
+		state = lxc_trim_whitespace_in_place(line + 6);
+		/* only check whether process is dead or zombie for now */
+		if (*state == 'X' || *state == 'Z')
+			ret = 1;
+		TRACE("TESTING: %s", state);
+	}
+
+	return ret;
+}
+
+int lxc_set_death_signal(int signal, pid_t parent, int parent_status_fd)
 {
 	int ret;
 	pid_t ppid;
@@ -1719,12 +1760,16 @@ int lxc_set_death_signal(int signal, pid_t parent)
 	ret = prctl(PR_SET_PDEATHSIG, prctl_arg(signal), prctl_arg(0),
 		    prctl_arg(0), prctl_arg(0));
 
-	/* If not in a PID namespace, check whether we have been orphaned. */
+	/* verify that we haven't been orphaned in the meantime */
 	ppid = (pid_t)syscall(SYS_getppid);
-	if (ppid && ppid != parent) {
-		ret = raise(SIGKILL);
-		if (ret < 0)
-			return -1;
+	if (ppid == 0) { /* parent outside our pidns */
+		if (parent_status_fd < 0)
+			return 0;
+
+		if (process_dead(parent_status_fd) == 1)
+			return raise(SIGKILL);
+	} else if (ppid != parent) {
+		return raise(SIGKILL);
 	}
 
 	if (ret < 0)
diff --git a/src/lxc/utils.h b/src/lxc/utils.h
index 747e14b6ef..13181729b7 100644
--- a/src/lxc/utils.h
+++ b/src/lxc/utils.h
@@ -253,7 +253,7 @@ static inline uint64_t lxc_getpagesize(void)
 extern uint64_t lxc_find_next_power2(uint64_t n);
 
 /* Set a signal the child process will receive after the parent has died. */
-extern int lxc_set_death_signal(int signal, pid_t parent);
+extern int lxc_set_death_signal(int signal, pid_t parent, int parent_status_fd);
 extern int fd_cloexec(int fd, bool cloexec);
 extern int recursive_destroy(char *dirname);
 extern int lxc_setup_keyring(void);


More information about the lxc-devel mailing list