[lxc-devel] [lxc/master] commands: fix state socket implementation
brauner on Github
lxc-bot at linuxcontainers.org
Mon Nov 20 22:05:01 UTC 2017
A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 1715 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20171120/81196514/attachment.bin>
-------------- next part --------------
From 9dfa4041c7039c64052a988d85d2b9291a87f2d3 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Mon, 20 Nov 2017 16:49:30 +0100
Subject: [PATCH 1/3] commands: non-functional changes
Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
src/lxc/commands.c | 11 +++++------
src/lxc/commands.h | 3 +--
src/lxc/commands_utils.c | 12 +++++-------
src/lxc/commands_utils.h | 2 +-
src/lxc/start.c | 19 ++++++++++---------
5 files changed, 22 insertions(+), 25 deletions(-)
diff --git a/src/lxc/commands.c b/src/lxc/commands.c
index f0f3c076a..3420c2e77 100644
--- a/src/lxc/commands.c
+++ b/src/lxc/commands.c
@@ -229,7 +229,7 @@ static int lxc_cmd_send(const char *name, struct lxc_cmd_rr *cmd,
int client_fd;
ssize_t ret = -1;
- client_fd = lxc_cmd_connect(name, lxcpath, hashed_sock_name);
+ client_fd = lxc_cmd_connect(name, lxcpath, hashed_sock_name, "command");
if (client_fd < 0) {
if (client_fd == -ECONNREFUSED)
return -ECONNREFUSED;
@@ -1284,8 +1284,7 @@ static int lxc_cmd_accept(int fd, uint32_t events, void *data,
goto out;
}
-int lxc_cmd_init(const char *name, struct lxc_handler *handler,
- const char *lxcpath)
+int lxc_cmd_init(const char *name, const char *lxcpath, const char *suffix)
{
int fd, len, ret;
char path[sizeof(((struct sockaddr_un *)0)->sun_path)] = {0};
@@ -1297,9 +1296,10 @@ int lxc_cmd_init(const char *name, struct lxc_handler *handler,
* because we print the sockname out sometimes.
*/
len = sizeof(path) - 2;
- ret = lxc_make_abstract_socket_name(offset, len, name, lxcpath, NULL, "command");
+ ret = lxc_make_abstract_socket_name(offset, len, name, lxcpath, NULL, suffix);
if (ret < 0)
return -1;
+ TRACE("Creating abstract unix socket \"%s\"", offset);
fd = lxc_abstract_unix_open(path, SOCK_STREAM, 0);
if (fd < 0) {
@@ -1317,8 +1317,7 @@ int lxc_cmd_init(const char *name, struct lxc_handler *handler,
return -1;
}
- handler->conf->maincmd_fd = fd;
- return 0;
+ return fd;
}
int lxc_cmd_mainloop_add(const char *name, struct lxc_epoll_descr *descr,
diff --git a/src/lxc/commands.h b/src/lxc/commands.h
index 7e197b98d..aeb34eeee 100644
--- a/src/lxc/commands.h
+++ b/src/lxc/commands.h
@@ -126,8 +126,7 @@ extern int lxc_cmd_add_state_client(const char *name, const char *lxcpath,
struct lxc_epoll_descr;
struct lxc_handler;
-extern int lxc_cmd_init(const char *name, struct lxc_handler *handler,
- const char *lxcpath);
+extern int lxc_cmd_init(const char *name, const char *lxcpath, const char *suffix);
extern int lxc_cmd_mainloop_add(const char *name, struct lxc_epoll_descr *descr,
struct lxc_handler *handler);
extern int lxc_try_cmd(const char *name, const char *lxcpath);
diff --git a/src/lxc/commands_utils.c b/src/lxc/commands_utils.c
index 23a0b9504..30c224519 100644
--- a/src/lxc/commands_utils.c
+++ b/src/lxc/commands_utils.c
@@ -68,16 +68,14 @@ int lxc_cmd_sock_rcv_state(int state_client_fd, int timeout)
goto again;
}
- ERROR("failed to receive message: %s", strerror(errno));
+ ERROR("Failed to receive message: %s", strerror(errno));
return -1;
}
- if (ret == 0) {
- ERROR("length of message was 0");
+ if (ret < 0)
return -1;
- }
- TRACE("received state %s from state client %d",
+ TRACE("Received state %s from state client %d",
lxc_state2str(msg.value), state_client_fd);
return msg.value;
@@ -163,7 +161,7 @@ int lxc_make_abstract_socket_name(char *path, int len, const char *lxcname,
}
int lxc_cmd_connect(const char *name, const char *lxcpath,
- const char *hashed_sock_name)
+ const char *hashed_sock_name, const char *suffix)
{
int ret, client_fd;
char path[sizeof(((struct sockaddr_un *)0)->sun_path)] = {0};
@@ -176,7 +174,7 @@ int lxc_cmd_connect(const char *name, const char *lxcpath,
*/
size_t len = sizeof(path) - 2;
ret = lxc_make_abstract_socket_name(offset, len, name, lxcpath,
- hashed_sock_name, "command");
+ hashed_sock_name, suffix);
if (ret < 0)
return -1;
diff --git a/src/lxc/commands_utils.h b/src/lxc/commands_utils.h
index 134a0d73a..d54cb11f3 100644
--- a/src/lxc/commands_utils.h
+++ b/src/lxc/commands_utils.h
@@ -79,6 +79,6 @@ extern int lxc_add_state_client(int state_client_fd,
* >= 0 client fd
*/
extern int lxc_cmd_connect(const char *name, const char *lxcpath,
- const char *hashed_sock_name);
+ const char *hashed_sock_name, const char *suffix);
#endif /* __LXC_COMMANDS_UTILS_H */
diff --git a/src/lxc/start.c b/src/lxc/start.c
index 41035a11b..4d583125b 100644
--- a/src/lxc/start.c
+++ b/src/lxc/start.c
@@ -347,10 +347,10 @@ static int lxc_serve_state_clients(const char *name,
* lxc_cmd_add_state_client() to miss a state.
*/
handler->state = state;
- TRACE("set container state to %s", lxc_state2str(state));
+ TRACE("Set container state to %s", lxc_state2str(state));
if (lxc_list_empty(&handler->state_clients)) {
- TRACE("no state clients registered");
+ TRACE("No state clients registered");
process_unlock();
lxc_monitor_send_state(name, state, handler->lxcpath);
return 0;
@@ -363,12 +363,12 @@ static int lxc_serve_state_clients(const char *name,
client = cur->elem;
if (!client->states[state]) {
- TRACE("state %s not registered for state client %d",
+ TRACE("State %s not registered for state client %d",
lxc_state2str(state), client->clientfd);
continue;
}
- TRACE("sending state %s to state client %d",
+ TRACE("Sending state %s to state client %d",
lxc_state2str(state), client->clientfd);
again:
@@ -379,7 +379,8 @@ static int lxc_serve_state_clients(const char *name,
goto again;
}
- ERROR("failed to send message to client");
+ ERROR("%s - Failed to send message to client",
+ strerror(errno));
}
/* kick client from list */
@@ -553,12 +554,12 @@ struct lxc_handler *lxc_init_handler(const char *name, struct lxc_conf *conf,
handler->state_socket_pair[1]);
}
- if (lxc_cmd_init(name, handler, lxcpath)) {
- ERROR("failed to set up command socket");
+ handler->conf->maincmd_fd = lxc_cmd_init(name, lxcpath, "command");
+ if (handler->conf->maincmd_fd < 0) {
+ ERROR("Failed to set up command socket");
goto on_error;
}
-
- TRACE("unix domain socket %d for command server is ready",
+ TRACE("Unix domain socket %d for command server is ready",
handler->conf->maincmd_fd);
return handler;
From c1a3e54736744b0a403f3fcda4f3466c727d6629 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Mon, 20 Nov 2017 16:50:00 +0100
Subject: [PATCH 2/3] lxccontainer: non-functional changes
Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
src/lxc/lxccontainer.c | 26 ++++++++++++++------------
1 file changed, 14 insertions(+), 12 deletions(-)
diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c
index ab2a5fc73..413dd375b 100644
--- a/src/lxc/lxccontainer.c
+++ b/src/lxc/lxccontainer.c
@@ -801,6 +801,7 @@ 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)
@@ -921,11 +922,11 @@ static bool do_lxcapi_start(struct lxc_container *c, int useinit, char * const a
exit(EXIT_FAILURE);
}
- ret = lxc_check_inherited(conf, true,
- (int[]){handler->conf->maincmd_fd,
- handler->state_socket_pair[0],
- handler->state_socket_pair[1]},
- 3);
+ 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]));
if (ret < 0)
exit(EXIT_FAILURE);
@@ -1010,11 +1011,11 @@ static bool do_lxcapi_start(struct lxc_container *c, int useinit, char * const a
}
}
- ret = lxc_check_inherited(conf, daemonize,
- (int[]){handler->conf->maincmd_fd,
- handler->state_socket_pair[0],
- handler->state_socket_pair[1]},
- 3);
+ 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, daemonize, keepfds,
+ sizeof(keepfds) / sizeof(keepfds[0]));
if (ret < 0) {
lxc_free_handler(handler);
ret = 1;
@@ -1816,7 +1817,7 @@ static bool do_lxcapi_shutdown(struct lxc_container *c, int timeout)
if (c->lxc_conf && c->lxc_conf->haltsignal)
haltsignal = c->lxc_conf->haltsignal;
- INFO("Using signal number '%d' as halt signal.", haltsignal);
+ INFO("Using signal number '%d' as halt signal", haltsignal);
/* Add a new state client before sending the shutdown signal so that we
* don't miss a state.
@@ -1827,13 +1828,14 @@ static bool do_lxcapi_shutdown(struct lxc_container *c, int timeout)
/* Send shutdown signal to container. */
if (kill(pid, haltsignal) < 0)
- WARN("Could not send signal %d to pid %d.", haltsignal, pid);
+ WARN("Could not send signal %d to pid %d", haltsignal, pid);
/* Retrieve the state. */
if (state_client_fd >= 0) {
int state;
state = lxc_cmd_sock_rcv_state(state_client_fd, timeout);
close(state_client_fd);
+ TRACE("Received state \"%s\"", lxc_state2str(state));
if (state != STOPPED)
return false;
retv = true;
From f6fc156515a0368659c957831bd7acdad1ee95a3 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Mon, 20 Nov 2017 22:16:40 +0100
Subject: [PATCH 3/3] commands: fix state socket implementation
Remove dead state clients from state client list. Consider the following
scenario:
01 start container
02 issue shutdown request
03 state_client_fd is added to lxc_handler
03 container doesn't respond to shutdown request
04 user aborts shutdown request
05 lxc_cmd_fd_cleanup() removes state_client_fd from lxc_mainloop
06 invalid state_client_fd is still recorded in the lxc_handler
07 user issues lxc_cmd_stop() request via SIGKILL
08 container reaches STOPPED state and sends message to state_client_fd
09 state_client_fd number has been reused by lxc_cmd_stop_callback()
10 invalid data gets dumped to lxc_cmd_stop()
Reproducer:
Set an invalid shutdown signal to which the init system does not respond with a
shutdown via lxc.signal.halt e.g. "lxc.signal.halt = SIGUSR1". Then do:
1. start container
root at conventiont|~
> lxc-start -n a1
2. try to shutdown container
root at conventiont|~
> lxc-stop -n a1
3. abort shutdown
^C
4. SIGKILL the container (lxc.signal.stop = SIGKILL)
root at conventiont|~
> lxc-stop -n a1 -k
lxc-stop: a1: commands.c: lxc_cmd_rsp_recv: 165 File too large - Response data for command "stop" is too long: 12641 bytes > 8192
To not let this happen we remove the state_client_fd from the lxc_handler when
we detect a cleanup event in lxc_cmd_fd_cleanup().
Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
src/lxc/commands.c | 27 ++++++++++++++++++++++++---
1 file changed, 24 insertions(+), 3 deletions(-)
diff --git a/src/lxc/commands.c b/src/lxc/commands.c
index 3420c2e77..553382993 100644
--- a/src/lxc/commands.c
+++ b/src/lxc/commands.c
@@ -1152,11 +1152,32 @@ static int lxc_cmd_process(int fd, struct lxc_cmd_req *req,
}
static void lxc_cmd_fd_cleanup(int fd, struct lxc_handler *handler,
- struct lxc_epoll_descr *descr)
+ struct lxc_epoll_descr *descr,
+ const lxc_cmd_t cmd)
{
+ struct state_client *client;
+ struct lxc_list *cur, *next;
+
lxc_console_free(handler->conf, fd);
lxc_mainloop_del_handler(descr, fd);
- close(fd);
+ if (cmd != LXC_CMD_ADD_STATE_CLIENT) {
+ close(fd);
+ return;
+ }
+
+ process_lock();
+ lxc_list_for_each_safe(cur, &handler->state_clients, next) {
+ client = cur->elem;
+ if (client->clientfd != fd)
+ continue;
+
+ /* kick client from list */
+ close(client->clientfd);
+ lxc_list_del(cur);
+ free(cur->elem);
+ free(cur);
+ }
+ process_unlock();
}
static int lxc_cmd_handler(int fd, uint32_t events, void *data,
@@ -1242,7 +1263,7 @@ static int lxc_cmd_handler(int fd, uint32_t events, void *data,
return ret;
out_close:
- lxc_cmd_fd_cleanup(fd, handler, descr);
+ lxc_cmd_fd_cleanup(fd, handler, descr, req.cmd);
goto out;
}
More information about the lxc-devel
mailing list