[lxc-devel] [lxc/master] commands: fix race when open()/close() cmd socket

[brauner on Github]

A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 674 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20171214/4f0cad4e/attachment.bin>
-------------- next part --------------
From 0f9a9692f39a7623bb26f4fca23848ec46ba09fa Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Thu, 14 Dec 2017 17:35:24 +0100
Subject: [PATCH] commands: fix race when open()/close() cmd socket

When we report STOPPED to a caller and then close the command socket it is
technically possible - and I've seen this happen on the test builders - that a
container start() right after a wait() will receive ECONNREFUSED because it
called open() before we close(). So let's lock setting the STOPPED state.

Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
 src/lxc/commands.c |  2 ++
 src/lxc/start.c    | 46 ++++++++++++++++++++++++++++++++++++----------
 2 files changed, 38 insertions(+), 10 deletions(-)

diff --git a/src/lxc/commands.c b/src/lxc/commands.c
index 4adede557..d1f726f60 100644
--- a/src/lxc/commands.c
+++ b/src/lxc/commands.c
@@ -1313,7 +1313,9 @@ int lxc_cmd_init(const char *name, const char *lxcpath, const char *suffix)
 		return -1;
 	TRACE("Creating abstract unix socket \"%s\"", offset);
 
+	process_lock();
 	fd = lxc_abstract_unix_open(path, SOCK_STREAM, 0);
+	process_unlock();
 	if (fd < 0) {
 		ERROR("%s - Failed to create command socket %s",
 		      strerror(errno), offset);
diff --git a/src/lxc/start.c b/src/lxc/start.c
index a144d6f8f..a4088922b 100644
--- a/src/lxc/start.c
+++ b/src/lxc/start.c
@@ -357,13 +357,11 @@ static int lxc_serve_state_clients(const char *name,
 	struct lxc_state_client *client;
 	struct lxc_msg msg = {.type = lxc_msg_state, .value = state};
 
-	process_lock();
 	handler->state = state;
 	TRACE("Set container state to %s", lxc_state2str(state));
 
 	if (lxc_list_empty(&handler->conf->state_clients)) {
 		TRACE("No state clients registered");
-		process_unlock();
 		lxc_monitor_send_state(name, state, handler->lxcpath);
 		return 0;
 	}
@@ -401,7 +399,6 @@ static int lxc_serve_state_clients(const char *name,
 		free(cur->elem);
 		free(cur);
 	}
-	process_unlock();
 
 	return 0;
 }
@@ -442,6 +439,39 @@ static int lxc_serve_state_socket_pair(const char *name,
 	return 0;
 }
 
+/* This locks the command socket so that when we inform a caller that the
+ * container is STOPPED the caller can actually immediately restart the
+ * container without racing against the close() call on the command socket
+ *fwithout racing against the close() call on the command socket fd.
+ */
+int lxc_set_state_stopped_close_cmd_socket(const char *name,
+					   struct lxc_handler *handler)
+{
+	int ret;
+
+	process_lock();
+	ret = lxc_serve_state_socket_pair(name, handler, STOPPED);
+	if (ret < 0) {
+		process_unlock();
+		ERROR("Failed to synchronize via anonymous pair of unix sockets");
+		return -1;
+	}
+
+	ret = lxc_serve_state_clients(name, handler, STOPPED);
+
+	if (handler->conf->reboot == 0) {
+		/* close command socket */
+		close(handler->conf->maincmd_fd);
+		handler->conf->maincmd_fd = -1;
+	}
+
+	process_unlock();
+	if (ret < 0)
+		return -1;
+
+	return 0;
+}
+
 int lxc_set_state(const char *name, struct lxc_handler *handler,
 		  lxc_state_t state)
 {
@@ -453,7 +483,9 @@ int lxc_set_state(const char *name, struct lxc_handler *handler,
 		return -1;
 	}
 
+	process_lock();
 	ret = lxc_serve_state_clients(name, handler, state);
+	process_unlock();
 	if (ret < 0)
 		return -1;
 
@@ -753,13 +785,7 @@ void lxc_fini(const char *name, struct lxc_handler *handler)
 
 	cgroup_destroy(handler);
 
-	lxc_set_state(name, handler, STOPPED);
-
-	if (handler->conf->reboot == 0) {
-		/* close command socket */
-		close(handler->conf->maincmd_fd);
-		handler->conf->maincmd_fd = -1;
-	}
+	lxc_set_state_stopped_close_cmd_socket(name, handler);
 
 	if (run_lxc_hooks(name, "post-stop", handler->conf, handler->lxcpath, NULL)) {
 		ERROR("Failed to run lxc.hook.post-stop for container \"%s\".", name);