Merge pull request #3410 from brauner/2020-05-13/fixes

src/lxc/criu.c

@@ -942,7 +942,7 @@ static void do_restore(struct lxc_container *c, int status_pipe, struct migrate_
 		close(fd);
 	}
 
-	handler = lxc_init_handler(c->name, c->lxc_conf, c->config_path, false);
+	handler = lxc_init_handler(NULL, c->name, c->lxc_conf, c->config_path, false);
 	if (!handler)
 		goto out;
 

src/lxc/log.c

@@ -44,7 +44,7 @@
 #define LXC_LOG_TIME_SIZE ((INTTYPE_TO_STRLEN(uint64_t)) * 2)
 
 int lxc_log_fd = -EBADF;
-static int syslog_enable = 0;
+static bool wants_syslog = false;
 int lxc_quiet_specified;
 int lxc_log_use_global_fd;
 static int lxc_loglevel_specified;
@@ -128,7 +128,7 @@ static int log_append_syslog(const struct lxc_log_appender *appender,
 	__do_free char *msg = NULL;
 	const char *log_container_name;
 
-	if (!syslog_enable)
+	if (!wants_syslog)
 		return 0;
 
 	log_container_name = lxc_log_get_container_name();
@@ -738,9 +738,14 @@ int lxc_log_syslog(int facility)
 	return 0;
 }
 
-inline void lxc_log_enable_syslog(void)
+void lxc_log_syslog_enable(void)
 {
-	syslog_enable = 1;
+	wants_syslog = true;
+}
+
+void lxc_log_syslog_disable(void)
+{
+	wants_syslog = false;
 }
 
 /*

src/lxc/log.h

@@ -563,7 +563,8 @@ __lxc_unused static inline void LXC_##LEVEL(struct lxc_log_locinfo* locinfo,	\
 extern int lxc_log_fd;
 
 extern int lxc_log_syslog(int facility);
-extern void lxc_log_enable_syslog(void);
+extern void lxc_log_syslog_enable(void);
+extern void lxc_log_syslog_disable(void);
 extern int lxc_log_set_level(int *dest, int level);
 extern int lxc_log_get_level(void);
 extern bool lxc_log_has_valid_level(void);

src/lxc/lxccontainer.c

@@ -830,14 +830,12 @@ static bool wait_on_daemonized_start(struct lxc_handler *handler, int pid)
 		DEBUG("First child %d exited", pid);
 
 	/* Close write end of the socket pair. */
-	close(handler->state_socket_pair[1]);
-	handler->state_socket_pair[1] = -1;
+	close_prot_errno_disarm(handler->state_socket_pair[1]);
 
 	state = lxc_rcv_status(handler->state_socket_pair[0]);
 
 	/* Close read end of the socket pair. */
-	close(handler->state_socket_pair[0]);
-	handler->state_socket_pair[0] = -1;
+	close_prot_errno_disarm(handler->state_socket_pair[0]);
 
 	if (state < 0) {
 		SYSERROR("Failed to receive the container state");
@@ -867,7 +865,6 @@ static bool do_lxcapi_start(struct lxc_container *c, int useinit, char * const a
 		NULL,
 	};
 	char **init_cmd = NULL;
-	int keepfds[3] = {-1, -1, -1};
 
 	/* container does exist */
 	if (!c)
@@ -901,7 +898,7 @@ static bool do_lxcapi_start(struct lxc_container *c, int useinit, char * const a
 	conf = c->lxc_conf;
 
 	/* initialize handler */
-	handler = lxc_init_handler(c->name, conf, c->config_path, c->daemonize);
+	handler = lxc_init_handler(NULL, c->name, conf, c->config_path, c->daemonize);
 
 	container_mem_unlock(c);
 	if (!handler)
@@ -918,7 +915,7 @@ static bool do_lxcapi_start(struct lxc_container *c, int useinit, char * const a
 	if (!argv) {
 		if (useinit) {
 			ERROR("No valid init detected");
-			lxc_free_handler(handler);
+			lxc_put_handler(handler);
 			return false;
 		}
 		argv = default_args;
@@ -936,7 +933,7 @@ static bool do_lxcapi_start(struct lxc_container *c, int useinit, char * const a
 		pid_first = fork();
 		if (pid_first < 0) {
 			free_init_cmd(init_cmd);
-			lxc_free_handler(handler);
+			lxc_put_handler(handler);
 			return false;
 		}
 
@@ -953,7 +950,7 @@ static bool do_lxcapi_start(struct lxc_container *c, int useinit, char * const a
 			started = wait_on_daemonized_start(handler, pid_first);
 
 			free_init_cmd(init_cmd);
-			lxc_free_handler(handler);
+			lxc_put_handler(handler);
 			return started;
 		}
 
@@ -985,7 +982,7 @@ static bool do_lxcapi_start(struct lxc_container *c, int useinit, char * const a
 		/* second parent */
 		if (pid_second != 0) {
 			free_init_cmd(init_cmd);
-			lxc_free_handler(handler);
+			lxc_put_handler(handler);
 			_exit(EXIT_SUCCESS);
 		}
 
@@ -998,11 +995,7 @@ static bool do_lxcapi_start(struct lxc_container *c, int useinit, char * const a
 			_exit(EXIT_FAILURE);
 		}
 
-		keepfds[0] = handler->conf->maincmd_fd;
-		keepfds[1] = handler->state_socket_pair[0];
-		keepfds[2] = handler->state_socket_pair[1];
-		ret = lxc_check_inherited(conf, true, keepfds,
-					  sizeof(keepfds) / sizeof(keepfds[0]));
+		ret = inherit_fds(handler, true);
 		if (ret < 0)
 			_exit(EXIT_FAILURE);
 
@@ -1020,7 +1013,7 @@ static bool do_lxcapi_start(struct lxc_container *c, int useinit, char * const a
 	} else if (!am_single_threaded()) {
 		ERROR("Cannot start non-daemonized container when threaded");
 		free_init_cmd(init_cmd);
-		lxc_free_handler(handler);
+		lxc_put_handler(handler);
 		return false;
 	}
 
@@ -1034,7 +1027,7 @@ static bool do_lxcapi_start(struct lxc_container *c, int useinit, char * const a
 		w = snprintf(pidstr, sizeof(pidstr), "%d", lxc_raw_getpid());
 		if (w < 0 || (size_t)w >= sizeof(pidstr)) {
 			free_init_cmd(init_cmd);
-			lxc_free_handler(handler);
+			lxc_put_handler(handler);
 
 			SYSERROR("Failed to write monitor pid to \"%s\"", c->pidfile);
 
@@ -1047,7 +1040,7 @@ static bool do_lxcapi_start(struct lxc_container *c, int useinit, char * const a
 		ret = lxc_write_to_file(c->pidfile, pidstr, w, false, 0600);
 		if (ret < 0) {
 			free_init_cmd(init_cmd);
-			lxc_free_handler(handler);
+			lxc_put_handler(handler);
 
 			SYSERROR("Failed to write monitor pid to \"%s\"", c->pidfile);
 
@@ -1065,7 +1058,7 @@ static bool do_lxcapi_start(struct lxc_container *c, int useinit, char * const a
 		ret = unshare(CLONE_NEWNS);
 		if (ret < 0) {
 			SYSERROR("Failed to unshare mount namespace");
-			lxc_free_handler(handler);
+			lxc_put_handler(handler);
 			ret = 1;
 			goto on_error;
 		}
@@ -1073,7 +1066,7 @@ static bool do_lxcapi_start(struct lxc_container *c, int useinit, char * const a
 		ret = mount(NULL, "/", NULL, MS_SLAVE|MS_REC, NULL);
 		if (ret < 0) {
 			SYSERROR("Failed to make / rslave at startup");
-			lxc_free_handler(handler);
+			lxc_put_handler(handler);
 			ret = 1;
 			goto on_error;
 		}
@@ -1082,20 +1075,16 @@ static bool do_lxcapi_start(struct lxc_container *c, int useinit, char * const a
 reboot:
 	if (conf->reboot == REBOOT_INIT) {
 		/* initialize handler */
-		handler = lxc_init_handler(c->name, conf, c->config_path, c->daemonize);
+		handler = lxc_init_handler(handler, c->name, conf, c->config_path, c->daemonize);
 		if (!handler) {
 			ret = 1;
 			goto on_error;
 		}
 	}
 
-	keepfds[0] = handler->conf->maincmd_fd;
-	keepfds[1] = handler->state_socket_pair[0];
-	keepfds[2] = handler->state_socket_pair[1];
-	ret = lxc_check_inherited(conf, c->daemonize, keepfds,
-				  sizeof(keepfds) / sizeof(keepfds[0]));
+	ret = inherit_fds(handler, c->daemonize);
 	if (ret < 0) {
-		lxc_free_handler(handler);
+		lxc_put_handler(handler);
 		ret = 1;
 		goto on_error;
 	}

src/lxc/start.c

@@ -212,6 +212,13 @@ int lxc_check_inherited(struct lxc_conf *conf, bool closeall,
 	if (conf && conf->close_all_fds)
 		closeall = true;
 
+	/*
+	 * Disable syslog at this point to avoid the above logging
+	 * function to open a new fd and make the check_inherited function
+	 * enter an infinite loop.
+	 */
+	lxc_log_syslog_disable();
+
 restart:
 	dir = opendir("/proc/self/fd");
 	if (!dir)
@@ -272,21 +279,24 @@ restart:
 
 #endif
 		if (closeall) {
-			close(fd);
-			closedir(dir);
+			if (close(fd))
+				SYSINFO("Closed inherited fd %d", fd);
+			else
 				INFO("Closed inherited fd %d", fd);
+			closedir(dir);
 			goto restart;
 		}
 		WARN("Inherited fd %d", fd);
 	}
+	closedir(dir);
 
-	/* Only enable syslog at this point to avoid the above logging function
-	 * to open a new fd and make the check_inherited function enter an
-	 * infinite loop.
+	/*
+	 * Only enable syslog at this point to avoid the above logging
+	 * function to open a new fd and make the check_inherited function
+	 * enter an infinite loop.
 	 */
-	lxc_log_enable_syslog();
+	lxc_log_syslog_enable();
 
-	closedir(dir); /* cannot fail */
 	return 0;
 }
 
@@ -605,32 +615,7 @@ out_sigfd:
 	return ret;
 }
 
-void lxc_zero_handler(struct lxc_handler *handler)
-{
-	memset(handler, 0, sizeof(struct lxc_handler));
-
-	handler->state = STOPPED;
-
-	handler->pinfd = -EBADF;
-
-	handler->pidfd = -EBADF;
-
-	handler->sigfd = -EBADF;
-
-	for (int i = 0; i < LXC_NS_MAX; i++)
-		handler->nsfd[i] = -EBADF;
-
-	handler->data_sock[0] = -EBADF;
-	handler->data_sock[1] = -EBADF;
-
-	handler->state_socket_pair[0] = -EBADF;
-	handler->state_socket_pair[1] = -EBADF;
-
-	handler->sync_sock[0] = -EBADF;
-	handler->sync_sock[1] = -EBADF;
-}
-
-void lxc_free_handler(struct lxc_handler *handler)
+void lxc_put_handler(struct lxc_handler *handler)
 {
 	close_prot_errno_disarm(handler->pinfd);
 	close_prot_errno_disarm(handler->pidfd);
@@ -642,22 +627,27 @@ void lxc_free_handler(struct lxc_handler *handler)
 	close_prot_errno_disarm(handler->state_socket_pair[0]);
 	close_prot_errno_disarm(handler->state_socket_pair[1]);
 	cgroup_exit(handler->cgroup_ops);
-	handler->conf = NULL;
+	if (handler->conf && handler->conf->reboot == REBOOT_NONE)
 		free_disarm(handler);
+	else
+		handler->conf = NULL;
 }
 
-struct lxc_handler *lxc_init_handler(const char *name, struct lxc_conf *conf,
+struct lxc_handler *lxc_init_handler(struct lxc_handler *old,
+				     const char *name, struct lxc_conf *conf,
 				     const char *lxcpath, bool daemonize)
 {
+	int nr_keep_fds = 0;
 	int ret;
 	struct lxc_handler *handler;
 
-	handler = malloc(sizeof(*handler));
+	if (!old)
+		handler = zalloc(sizeof(*handler));
+	else
+		handler = old;
 	if (!handler)
 		return NULL;
 
-	memset(handler, 0, sizeof(*handler));
-
 	/* Note that am_guest_unpriv() checks the effective uid. We
 	 * probably don't care if we are real root only if we are running
 	 * as root so this should be fine.
@@ -701,6 +691,8 @@ struct lxc_handler *lxc_init_handler(const char *name, struct lxc_conf *conf,
 		TRACE("Created anonymous pair {%d,%d} of unix sockets",
 		      handler->state_socket_pair[0],
 		      handler->state_socket_pair[1]);
+		handler->keep_fds[nr_keep_fds++] = handler->state_socket_pair[0];
+		handler->keep_fds[nr_keep_fds++] = handler->state_socket_pair[1];
 	}
 
 	if (handler->conf->reboot == REBOOT_NONE) {
@@ -709,6 +701,7 @@ struct lxc_handler *lxc_init_handler(const char *name, struct lxc_conf *conf,
 			ERROR("Failed to set up command socket");
 			goto on_error;
 		}
+		handler->keep_fds[nr_keep_fds++] = handler->conf->maincmd_fd;
 	}
 
 	TRACE("Unix domain socket %d for command server is ready",
@@ -717,7 +710,7 @@ struct lxc_handler *lxc_init_handler(const char *name, struct lxc_conf *conf,
 	return handler;
 
 on_error:
-	lxc_free_handler(handler);
+	lxc_put_handler(handler);
 
 	return NULL;
 }
@@ -1008,7 +1001,7 @@ void lxc_end(struct lxc_handler *handler)
 	if (handler->conf->ephemeral == 1 && handler->conf->reboot != REBOOT_REQ)
 		lxc_destroy_container_on_signal(handler, name);
 
-	lxc_free_handler(handler);
+	lxc_put_handler(handler);
 }
 
 void lxc_abort(struct lxc_handler *handler)

src/lxc/start.h

@@ -10,6 +10,7 @@
 #include <sys/un.h>
 
 #include "conf.h"
+#include "macro.h"
 #include "namespace.h"
 #include "state.h"
 
@@ -122,6 +123,9 @@ struct lxc_handler {
 	int exit_status;
 
 	struct cgroup_ops *cgroup_ops;
+
+	/* Internal fds that always need to stay open. */
+	int keep_fds[3];
 };
 
 struct execute_args {
@@ -143,12 +147,11 @@ extern int lxc_serve_state_clients(const char *name,
 				   struct lxc_handler *handler,
 				   lxc_state_t state);
 extern void lxc_abort(struct lxc_handler *handler);
-extern struct lxc_handler *lxc_init_handler(const char *name,
+extern struct lxc_handler *lxc_init_handler(struct lxc_handler *old,
+					    const char *name,
 					    struct lxc_conf *conf,
-					    const char *lxcpath,
-					    bool daemonize);
-extern void lxc_zero_handler(struct lxc_handler *handler);
-extern void lxc_free_handler(struct lxc_handler *handler);
+					    const char *lxcpath, bool daemonize);
+extern void lxc_put_handler(struct lxc_handler *handler);
 extern int lxc_init(const char *name, struct lxc_handler *handler);
 extern void lxc_end(struct lxc_handler *handler);
 
@@ -161,6 +164,11 @@ extern void lxc_end(struct lxc_handler *handler);
  */
 extern int lxc_check_inherited(struct lxc_conf *conf, bool closeall,
 			       int *fds_to_ignore, size_t len_fds);
+static inline int inherit_fds(struct lxc_handler *handler, bool closeall)
+{
+	return lxc_check_inherited(handler->conf, closeall, handler->keep_fds,
+				   ARRAY_SIZE(handler->keep_fds));
+}
 extern int __lxc_start(struct lxc_handler *, struct lxc_operations *, void *,
 		       const char *, bool, int *);