The following pull request was submitted through Github.
It can be accessed and reviewed at: https://github.com/lxc/lxc/pull/3290
This e-mail was sent by the LXC bot, direct replies will not reach the author
unless they happen to be subscribed to this list.
=== Description (from pull-request) ===
Signed-off-by: Christian Brauner <christian.brau...@ubuntu.com>
From ea2a070bc5666ef7b3fc23023c3a5214b0dafc9c Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brau...@ubuntu.com>
Date: Wed, 11 Mar 2020 12:02:10 +0100
Subject: [PATCH] commands: add ability to audit fd connection and cleanup path
Signed-off-by: Christian Brauner <christian.brau...@ubuntu.com>
---
src/lxc/commands.c | 145 +++++++++++++++++++++------------------
src/lxc/commands.h | 7 ++
src/lxc/commands_utils.c | 3 +-
src/lxc/lxccontainer.c | 14 ++--
4 files changed, 94 insertions(+), 75 deletions(-)
diff --git a/src/lxc/commands.c b/src/lxc/commands.c
index 1feae25aab..b11c3c7fc8 100644
--- a/src/lxc/commands.c
+++ b/src/lxc/commands.c
@@ -284,6 +284,9 @@ static int lxc_cmd(const char *name, struct lxc_cmd_rr
*cmd, int *stopped,
if (ret < 0 && errno == ECONNRESET)
*stopped = 1;
+ TRACE("Opened new command socket connection fd %d for command \"%s\"",
+ client_fd, lxc_cmd_str(cmd->req.cmd));
+
if (stay_connected && ret > 0)
cmd->rsp.ret = move_fd(client_fd);
@@ -357,11 +360,16 @@ static int lxc_cmd_get_init_pid_callback(int fd, struct
lxc_cmd_req *req,
struct lxc_handler *handler,
struct lxc_epoll_descr *descr)
{
+ int ret;
struct lxc_cmd_rsp rsp = {
.data = PID_TO_PTR(handler->pid)
};
- return lxc_cmd_rsp_send(fd, &rsp);
+ ret = lxc_cmd_rsp_send(fd, &rsp);
+ if (ret < 0)
+ return LXC_CMD_REAP_CLIENT_FD;
+
+ return 0;
}
/*
@@ -392,11 +400,16 @@ static int lxc_cmd_get_clone_flags_callback(int fd,
struct lxc_cmd_req *req,
struct lxc_handler *handler,
struct lxc_epoll_descr *descr)
{
+ int ret;
struct lxc_cmd_rsp rsp = {
.data = INT_TO_PTR(handler->ns_clone_flags),
};
- return lxc_cmd_rsp_send(fd, &rsp);
+ ret = lxc_cmd_rsp_send(fd, &rsp);
+ if (ret < 0)
+ return LXC_CMD_REAP_CLIENT_FD;
+
+ return 0;
}
/*
@@ -445,6 +458,7 @@ static int lxc_cmd_get_cgroup_callback(int fd, struct
lxc_cmd_req *req,
struct lxc_handler *handler,
struct lxc_epoll_descr *descr)
{
+ int ret;
const char *path;
struct lxc_cmd_rsp rsp;
struct cgroup_ops *cgroup_ops = handler->cgroup_ops;
@@ -460,7 +474,11 @@ static int lxc_cmd_get_cgroup_callback(int fd, struct
lxc_cmd_req *req,
rsp.datalen = strlen(path) + 1;
rsp.data = (char *)path;
- return lxc_cmd_rsp_send(fd, &rsp);
+ ret = lxc_cmd_rsp_send(fd, &rsp);
+ if (ret < 0)
+ return LXC_CMD_REAP_CLIENT_FD;
+
+ return 0;
}
/*
@@ -525,7 +543,11 @@ static int lxc_cmd_get_config_item_callback(int fd, struct
lxc_cmd_req *req,
err1:
rsp.ret = -1;
out:
- return lxc_cmd_rsp_send(fd, &rsp);
+ cilen = lxc_cmd_rsp_send(fd, &rsp);
+ if (cilen < 0)
+ return LXC_CMD_REAP_CLIENT_FD;
+
+ return 0;
}
/*
@@ -564,11 +586,16 @@ static int lxc_cmd_get_state_callback(int fd, struct
lxc_cmd_req *req,
struct lxc_handler *handler,
struct lxc_epoll_descr *descr)
{
+ int ret;
struct lxc_cmd_rsp rsp = {
.data = INT_TO_PTR(handler->state),
};
- return lxc_cmd_rsp_send(fd, &rsp);
+ ret = lxc_cmd_rsp_send(fd, &rsp);
+ if (ret < 0)
+ return LXC_CMD_REAP_CLIENT_FD;
+
+ return 0;
}
/*
@@ -613,6 +640,7 @@ static int lxc_cmd_stop_callback(int fd, struct lxc_cmd_req
*req,
struct lxc_cmd_rsp rsp;
int stopsignal = SIGKILL;
struct cgroup_ops *cgroup_ops = handler->cgroup_ops;
+ int ret;
if (handler->conf->stopsignal)
stopsignal = handler->conf->stopsignal;
@@ -629,7 +657,11 @@ static int lxc_cmd_stop_callback(int fd, struct
lxc_cmd_req *req,
rsp.ret = -errno;
}
- return lxc_cmd_rsp_send(fd, &rsp);
+ ret = lxc_cmd_rsp_send(fd, &rsp);
+ if (ret < 0)
+ return LXC_CMD_REAP_CLIENT_FD;
+
+ return 0;
}
/*
@@ -706,24 +738,18 @@ static int lxc_cmd_console_callback(int fd, struct
lxc_cmd_req *req,
masterfd = lxc_terminal_allocate(handler->conf, fd, &ttynum);
if (masterfd < 0)
- goto out_close;
+ return LXC_CMD_REAP_CLIENT_FD;
memset(&rsp, 0, sizeof(rsp));
rsp.data = INT_TO_PTR(ttynum);
ret = lxc_abstract_unix_send_fds(fd, &masterfd, 1, &rsp, sizeof(rsp));
if (ret < 0) {
- SYSERROR("Failed to send tty to client");
lxc_terminal_free(handler->conf, fd);
- goto out_close;
+ return log_error_errno(LXC_CMD_REAP_CLIENT_FD, errno,
+ "Failed to send tty to client");
}
return 0;
-
-out_close:
- /* Special indicator to lxc_cmd_handler() to close the fd and do
- * related cleanup.
- */
- return 1;
}
/*
@@ -756,6 +782,7 @@ static int lxc_cmd_get_name_callback(int fd, struct
lxc_cmd_req *req,
struct lxc_handler *handler,
struct lxc_epoll_descr *descr)
{
+ int ret;
struct lxc_cmd_rsp rsp;
memset(&rsp, 0, sizeof(rsp));
@@ -764,7 +791,11 @@ static int lxc_cmd_get_name_callback(int fd, struct
lxc_cmd_req *req,
rsp.datalen = strlen(handler->name) + 1;
rsp.ret = 0;
- return lxc_cmd_rsp_send(fd, &rsp);
+ ret = lxc_cmd_rsp_send(fd, &rsp);
+ if (ret < 0)
+ return LXC_CMD_REAP_CLIENT_FD;
+
+ return 0;
}
/*
@@ -797,13 +828,18 @@ static int lxc_cmd_get_lxcpath_callback(int fd, struct
lxc_cmd_req *req,
struct lxc_handler *handler,
struct lxc_epoll_descr *descr)
{
+ int ret;
struct lxc_cmd_rsp rsp = {
.ret = 0,
.data = (char *)handler->lxcpath,
.datalen = strlen(handler->lxcpath) + 1,
};
- return lxc_cmd_rsp_send(fd, &rsp);
+ ret = lxc_cmd_rsp_send(fd, &rsp);
+ if (ret < 0)
+ return LXC_CMD_REAP_CLIENT_FD;
+
+ return 0;
}
int lxc_cmd_add_state_client(const char *name, const char *lxcpath,
@@ -844,7 +880,8 @@ int lxc_cmd_add_state_client(const char *name, const char
*lxcpath,
return log_trace(state, "Container is already in requested
state %s", lxc_state2str(state));
*state_client_fd = move_fd(clientfd);
- return log_trace(MAX_STATE, "Added state client %d to state client
list", *state_client_fd);
+ TRACE("State connection fd %d ready to listen for container state
changes", *state_client_fd);
+ return MAX_STATE;
}
static int lxc_cmd_add_state_client_callback(__owns int fd, struct lxc_cmd_req
*req,
@@ -855,31 +892,25 @@ static int lxc_cmd_add_state_client_callback(__owns int
fd, struct lxc_cmd_req *
struct lxc_cmd_rsp rsp = {0};
if (req->datalen < 0)
- goto reap_client_fd;
+ return LXC_CMD_REAP_CLIENT_FD;
if (req->datalen != (sizeof(lxc_state_t) * MAX_STATE))
- goto reap_client_fd;
+ return LXC_CMD_REAP_CLIENT_FD;
if (!req->data)
- goto reap_client_fd;
+ return LXC_CMD_REAP_CLIENT_FD;
rsp.ret = lxc_add_state_client(fd, handler, (lxc_state_t *)req->data);
if (rsp.ret < 0)
- goto reap_client_fd;
+ return LXC_CMD_REAP_CLIENT_FD;
rsp.data = INT_TO_PTR(rsp.ret);
ret = lxc_cmd_rsp_send(fd, &rsp);
if (ret < 0)
- goto reap_client_fd;
+ return LXC_CMD_REAP_CLIENT_FD;
return 0;
-
-reap_client_fd:
- /* Special indicator to lxc_cmd_handler() to close the fd and do related
- * cleanup.
- */
- return 1;
}
int lxc_cmd_add_bpf_device_cgroup(const char *name, const char *lxcpath,
@@ -925,13 +956,13 @@ static int lxc_cmd_add_bpf_device_cgroup_callback(int fd,
struct lxc_cmd_req *re
struct bpf_program *devices_old;
if (req->datalen <= 0)
- goto reap_client_fd;
+ return LXC_CMD_REAP_CLIENT_FD;
if (req->datalen != sizeof(struct device_item))
- goto reap_client_fd;
+ return LXC_CMD_REAP_CLIENT_FD;
if (!req->data)
- goto reap_client_fd;
+ return LXC_CMD_REAP_CLIENT_FD;
device = (struct device_item *)req->data;
rsp.ret = -1;
@@ -978,15 +1009,9 @@ static int lxc_cmd_add_bpf_device_cgroup_callback(int fd,
struct lxc_cmd_req *re
respond:
ret = lxc_cmd_rsp_send(fd, &rsp);
if (ret < 0)
- goto reap_client_fd;
+ return LXC_CMD_REAP_CLIENT_FD;
return 0;
-
-reap_client_fd:
- /* Special indicator to lxc_cmd_handler() to close the fd and do related
- * cleanup.
- */
- return 1;
#else
return ret_set_errno(-1, ENOSYS);
#endif
@@ -1097,20 +1122,13 @@ static int lxc_cmd_serve_state_clients_callback(int fd,
struct lxc_cmd_req *req,
ret = lxc_serve_state_clients(handler->name, handler, state);
if (ret < 0)
- goto reap_client_fd;
+ return LXC_CMD_REAP_CLIENT_FD;
ret = lxc_cmd_rsp_send(fd, &rsp);
if (ret < 0)
- goto reap_client_fd;
+ return LXC_CMD_REAP_CLIENT_FD;
return 0;
-
-reap_client_fd:
- /*
- * Special indicator to lxc_cmd_handler() to close the fd and do
- * related cleanup.
- */
- return 1;
}
int lxc_cmd_seccomp_notify_add_listener(const char *name, const char *lxcpath,
@@ -1347,7 +1365,7 @@ static void lxc_cmd_fd_cleanup(int fd, struct lxc_handler
*handler,
* No need to walk the whole list. If we found the state
* client fd there can't be a second one.
*/
- TRACE("Found state client fd %d in state client list",
fd);
+ TRACE("Found state client fd %d in state client list
for command \"%s\"", fd, lxc_cmd_str(cmd));
break;
}
@@ -1357,9 +1375,10 @@ static void lxc_cmd_fd_cleanup(int fd, struct
lxc_handler *handler,
* was already reached by the time we were ready to add it. So
* fallthrough and clean it up.
*/
- TRACE("Closing state client fd %d", fd);
+ TRACE("Closing state client fd %d for command \"%s\"", fd,
lxc_cmd_str(cmd));
}
+ TRACE("Closing client fd %d for command \"%s\"", fd, lxc_cmd_str(cmd));
close(fd);
}
@@ -1373,12 +1392,13 @@ static int lxc_cmd_handler(int fd, uint32_t events,
void *data,
ret = lxc_abstract_unix_rcv_credential(fd, &req, sizeof(req));
if (ret < 0) {
- SYSERROR("Failed to receive data on command socket for command "
- "\"%s\"", lxc_cmd_str(req.cmd));
+ SYSERROR("Failed to receive data on command socket for command
\"%s\"", lxc_cmd_str(req.cmd));
if (errno == EACCES) {
/* We don't care for the peer, just send and close. */
- struct lxc_cmd_rsp rsp = {.ret = ret};
+ struct lxc_cmd_rsp rsp = {
+ .ret = ret,
+ };
lxc_cmd_rsp_send(fd, &rsp);
}
@@ -1390,16 +1410,13 @@ static int lxc_cmd_handler(int fd, uint32_t events,
void *data,
goto out_close;
if (ret != sizeof(req)) {
- WARN("Failed to receive full command request. Ignoring request "
- "for \"%s\"", lxc_cmd_str(req.cmd));
+ WARN("Failed to receive full command request. Ignoring request
for \"%s\"", lxc_cmd_str(req.cmd));
ret = -1;
goto out_close;
}
- if ((req.datalen > LXC_CMD_DATA_MAX) &&
- (req.cmd != LXC_CMD_CONSOLE_LOG)) {
- ERROR("Received command data length %d is too large for "
- "command \"%s\"", req.datalen, lxc_cmd_str(req.cmd));
+ if ((req.datalen > LXC_CMD_DATA_MAX) && (req.cmd !=
LXC_CMD_CONSOLE_LOG)) {
+ ERROR("Received command data length %d is too large for command
\"%s\"", req.datalen, lxc_cmd_str(req.cmd));
errno = EFBIG;
ret = -EFBIG;
goto out_close;
@@ -1409,8 +1426,7 @@ static int lxc_cmd_handler(int fd, uint32_t events, void
*data,
reqdata = must_realloc(NULL, req.datalen);
ret = lxc_recv_nointr(fd, reqdata, req.datalen, 0);
if (ret != req.datalen) {
- WARN("Failed to receive full command request. Ignoring "
- "request for \"%s\"", lxc_cmd_str(req.cmd));
+ WARN("Failed to receive full command request. Ignoring
request for \"%s\"", lxc_cmd_str(req.cmd));
ret = LXC_MAINLOOP_ERROR;
goto out_close;
}
@@ -1455,6 +1471,7 @@ static int lxc_cmd_accept(int fd, uint32_t events, void
*data,
if (ret)
return log_error(ret, "Failed to add command handler");
+ TRACE("Accepted new client as fd %d on command server fd %d",
connection, fd);
move_fd(connection);
return ret;
}
@@ -1487,13 +1504,11 @@ int lxc_cmd_init(const char *name, const char *lxcpath,
const char *suffix)
int lxc_cmd_mainloop_add(const char *name, struct lxc_epoll_descr *descr,
struct lxc_handler *handler)
{
- __do_close_prot_errno int fd = handler->conf->maincmd_fd;
int ret;
- ret = lxc_mainloop_add_handler(descr, fd, lxc_cmd_accept, handler);
+ ret = lxc_mainloop_add_handler(descr, handler->conf->maincmd_fd,
lxc_cmd_accept, handler);
if (ret < 0)
- return log_error(ret, "Failed to add handler for command
socket");
+ return log_error(ret, "Failed to add handler for command socket
fd %d", handler->conf->maincmd_fd);
- move_fd(fd);
return ret;
}
diff --git a/src/lxc/commands.h b/src/lxc/commands.h
index c386c21bef..cd78a61a21 100644
--- a/src/lxc/commands.h
+++ b/src/lxc/commands.h
@@ -11,6 +11,13 @@
#include "macro.h"
#include "state.h"
+/*
+ * Value command callbacks should return when they want the client fd to be
+ * cleaned up by the main loop. This is most certainly what you want unless you
+ * have specific reasons to keep the file descriptor alive.
+ */
+#define LXC_CMD_REAP_CLIENT_FD 1
+
typedef enum {
LXC_CMD_CONSOLE,
LXC_CMD_TERMINAL_WINCH,
diff --git a/src/lxc/commands_utils.c b/src/lxc/commands_utils.c
index 884a18ea1c..dbc06bf334 100644
--- a/src/lxc/commands_utils.c
+++ b/src/lxc/commands_utils.c
@@ -185,5 +185,6 @@ int lxc_add_state_client(int state_client_fd, struct
lxc_handler *handler,
move_ptr(newclient);
move_ptr(tmplist);
- return log_trace(MAX_STATE, "Added state client %d to state client
list", state_client_fd);
+ TRACE("Added state client fd %d to state client list", state_client_fd);
+ return MAX_STATE;
}
diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c
index 68e5b3400a..bf0c44d217 100644
--- a/src/lxc/lxccontainer.c
+++ b/src/lxc/lxccontainer.c
@@ -2064,10 +2064,11 @@ WRAP_API_1(bool, lxcapi_reboot2, int)
static bool do_lxcapi_shutdown(struct lxc_container *c, int timeout)
{
+ __do_close_prot_errno int state_client_fd = -EBADF;
+ int haltsignal = SIGPWR;
+ lxc_state_t states[MAX_STATE] = {0};
int killret, ret;
pid_t pid;
- int haltsignal = SIGPWR, state_client_fd = -EBADF;
- lxc_state_t states[MAX_STATE] = {0};
if (!c)
return false;
@@ -2107,20 +2108,15 @@ static bool do_lxcapi_shutdown(struct lxc_container *c,
int timeout)
/* Send shutdown signal to container. */
killret = kill(pid, haltsignal);
- if (killret < 0) {
- if (state_client_fd >= 0)
- close(state_client_fd);
+ if (killret < 0)
+ return log_warn(false, "Failed to send signal %d to pid %d",
haltsignal, pid);
- WARN("Failed to send signal %d to pid %d", haltsignal, pid);
- return false;
- }
TRACE("Sent signal %d to pid %d", haltsignal, pid);
if (timeout == 0)
return true;
ret = lxc_cmd_sock_rcv_state(state_client_fd, timeout);
- close(state_client_fd);
if (ret < 0)
return false;
_______________________________________________
lxc-devel mailing list
lxc-devel@lists.linuxcontainers.org
http://lists.linuxcontainers.org/listinfo/lxc-devel
