From 6dd32d35f9bbc39cc727fe32f7f7e5872ea4120d Mon Sep 17 00:00:00 2001 From: 2xsec Date: Fri, 12 Oct 2018 10:36:42 +0900 Subject: [PATCH 1/4] monitor: log cleanups Signed-off-by: 2xsec --- src/lxc/monitor.c | 40 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/src/lxc/monitor.c b/src/lxc/monitor.c index 4a5b9a985..ef21457c3 100644 --- a/src/lxc/monitor.c +++ b/src/lxc/monitor.c @@ -73,7 +73,7 @@ int lxc_monitor_fifo_name(const char *lxcpath, char *fifo_path, size_t fifo_path if (do_mkdirp) { ret = snprintf(fifo_path, fifo_path_sz, "%s/lxc/%s", rundir, lxcpath); if (ret < 0 || (size_t)ret >= fifo_path_sz) { - ERROR("rundir/lxcpath (%s/%s) too long for monitor fifo.", rundir, lxcpath); + ERROR("rundir/lxcpath (%s/%s) too long for monitor fifo", rundir, lxcpath); free(rundir); return -1; } @@ -86,7 +86,7 @@ int lxc_monitor_fifo_name(const char *lxcpath, char *fifo_path, size_t fifo_path } ret = snprintf(fifo_path, fifo_path_sz, "%s/lxc/%s/monitor-fifo", rundir, lxcpath); if (ret < 0 || (size_t)ret >= fifo_path_sz) { - ERROR("rundir/lxcpath (%s/%s) too long for monitor fifo.", rundir, lxcpath); + ERROR("rundir/lxcpath (%s/%s) too long for monitor fifo", rundir, lxcpath); free(rundir); return -1; } @@ -128,7 +128,7 @@ static void lxc_monitor_fifo_send(struct lxc_msg *msg, const char *lxcpath) ret = lxc_write_nointr(fd, msg, sizeof(*msg)); if (ret != sizeof(*msg)) { close(fd); - SYSERROR("Failed to write to monitor fifo \"%s\".", fifo_path); + SYSERROR("Failed to write to monitor fifo \"%s\"", fifo_path); return; } @@ -185,7 +185,7 @@ int lxc_monitor_sock_name(const char *lxcpath, struct sockaddr_un *addr) { path = alloca(len); ret = snprintf(path, len, "lxc/%s/monitor-sock", lxcpath); if (ret < 0 || (size_t)ret >= len) { - ERROR("failed to create name for monitor socket"); + ERROR("Failed to create name for monitor socket"); return -1; } @@ -198,13 +198,13 @@ int lxc_monitor_sock_name(const char *lxcpath, struct sockaddr_un *addr) { hash = fnv_64a_buf(path, ret, FNV1A_64_INIT); ret = snprintf(addr->sun_path, len, "@lxc/%016" PRIx64 "/%s", hash, lxcpath); if (ret < 0) { - ERROR("failed to create hashed name for monitor socket"); + ERROR("Failed to create hashed name for monitor socket"); return -1; } /* replace @ with \0 */ addr->sun_path[0] = '\0'; - INFO("using monitor socket name \"%s\" (length of socket name %zu must be <= %zu)", &addr->sun_path[1], strlen(&addr->sun_path[1]), sizeof(addr->sun_path) - 3); + INFO("Using monitor socket name \"%s\" (length of socket name %zu must be <= %zu)", &addr->sun_path[1], strlen(&addr->sun_path[1]), sizeof(addr->sun_path) - 3); return 0; } @@ -221,7 +221,7 @@ int lxc_monitor_open(const char *lxcpath) return -1; len = strlen(&addr.sun_path[1]); - DEBUG("opening monitor socket %s with len %zu", &addr.sun_path[1], len); + DEBUG("Opening monitor socket %s with len %zu", &addr.sun_path[1], len); if (len >= sizeof(addr.sun_path) - 1) { errno = ENAMETOOLONG; SYSERROR("The name of monitor socket too long (%zu bytes)", len); @@ -272,7 +272,7 @@ int lxc_monitor_read_fdset(struct pollfd *fds, nfds_t nfds, struct lxc_msg *msg, } } - SYSERROR("No ready fd found."); + SYSERROR("No ready fd found"); return -1; } @@ -315,33 +315,33 @@ int lxc_monitord_spawn(const char *lxcpath) /* double fork to avoid zombies when monitord exits */ pid1 = fork(); if (pid1 < 0) { - SYSERROR("Failed to fork()."); + SYSERROR("Failed to fork()"); return -1; } if (pid1) { - DEBUG("Going to wait for pid %d.", pid1); + DEBUG("Going to wait for pid %d", pid1); if (waitpid(pid1, NULL, 0) != pid1) return -1; - DEBUG("Finished waiting on pid %d.", pid1); + DEBUG("Finished waiting on pid %d", pid1); return 0; } if (pipe(pipefd) < 0) { - SYSERROR("Failed to create pipe."); + SYSERROR("Failed to create pipe"); _exit(EXIT_FAILURE); } pid2 = fork(); if (pid2 < 0) { - SYSERROR("Failed to fork()."); + SYSERROR("Failed to fork()"); _exit(EXIT_FAILURE); } if (pid2) { - DEBUG("Trying to sync with child process."); + DEBUG("Trying to sync with child process"); char c; /* Wait for daemon to create socket. */ close(pipefd[1]); @@ -356,18 +356,18 @@ int lxc_monitord_spawn(const char *lxcpath) close(pipefd[0]); - DEBUG("Successfully synced with child process."); + DEBUG("Successfully synced with child process"); _exit(EXIT_SUCCESS); } if (setsid() < 0) { - SYSERROR("Failed to setsid()."); + SYSERROR("Failed to setsid()"); _exit(EXIT_FAILURE); } lxc_check_inherited(NULL, true, &pipefd[1], 1); if (null_stdfds() < 0) { - SYSERROR("Failed to dup2() standard file descriptors to /dev/null."); + SYSERROR("Failed to dup2() standard file descriptors to /dev/null"); _exit(EXIT_FAILURE); } @@ -375,14 +375,14 @@ int lxc_monitord_spawn(const char *lxcpath) ret = snprintf(pipefd_str, sizeof(pipefd_str), "%d", pipefd[1]); if (ret < 0 || ret >= sizeof(pipefd_str)) { - ERROR("Failed to create pid argument to pass to monitord."); + ERROR("Failed to create pid argument to pass to monitord"); _exit(EXIT_FAILURE); } - DEBUG("Using pipe file descriptor %d for monitord.", pipefd[1]); + DEBUG("Using pipe file descriptor %d for monitord", pipefd[1]); execvp(args[0], args); - SYSERROR("failed to exec lxc-monitord"); + SYSERROR("Failed to exec lxc-monitord"); _exit(EXIT_FAILURE); } From 2f1264995f17ddb206164b31c1b2b222e4913f3a Mon Sep 17 00:00:00 2001 From: 2xsec Date: Fri, 12 Oct 2018 11:19:04 +0900 Subject: [PATCH 2/4] monitor: checking name too long to make monitor sock name Signed-off-by: 2xsec --- src/lxc/monitor.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/lxc/monitor.c b/src/lxc/monitor.c index ef21457c3..64dd3511e 100644 --- a/src/lxc/monitor.c +++ b/src/lxc/monitor.c @@ -199,7 +199,11 @@ int lxc_monitor_sock_name(const char *lxcpath, struct sockaddr_un *addr) { ret = snprintf(addr->sun_path, len, "@lxc/%016" PRIx64 "/%s", hash, lxcpath); if (ret < 0) { ERROR("Failed to create hashed name for monitor socket"); - return -1; + goto on_error; + } else if ((size_t)ret >= len) { + errno = ENAMETOOLONG; + SYSERROR("The name of monitor socket too long (%d bytes)", ret); + goto on_error; } /* replace @ with \0 */ @@ -207,6 +211,9 @@ int lxc_monitor_sock_name(const char *lxcpath, struct sockaddr_un *addr) { INFO("Using monitor socket name \"%s\" (length of socket name %zu must be <= %zu)", &addr->sun_path[1], strlen(&addr->sun_path[1]), sizeof(addr->sun_path) - 3); return 0; + +on_error: + return -1; } int lxc_monitor_open(const char *lxcpath) @@ -214,19 +221,12 @@ int lxc_monitor_open(const char *lxcpath) struct sockaddr_un addr; int fd; size_t retry; - size_t len; int backoff_ms[] = {10, 50, 100}; if (lxc_monitor_sock_name(lxcpath, &addr) < 0) return -1; - len = strlen(&addr.sun_path[1]); - DEBUG("Opening monitor socket %s with len %zu", &addr.sun_path[1], len); - if (len >= sizeof(addr.sun_path) - 1) { - errno = ENAMETOOLONG; - SYSERROR("The name of monitor socket too long (%zu bytes)", len); - return -1; - } + DEBUG("Opening monitor socket %s with len %zu", &addr.sun_path[1], strlen(&addr.sun_path[1])); for (retry = 0; retry < sizeof(backoff_ms) / sizeof(backoff_ms[0]); retry++) { fd = lxc_abstract_unix_connect(addr.sun_path); From 5b46db1a63dd0b663cce2cb8015c2a5aa05453a1 Mon Sep 17 00:00:00 2001 From: 2xsec Date: Fri, 12 Oct 2018 15:05:43 +0900 Subject: [PATCH 3/4] commands_utils: improve code redundancy to make abstract unix socket name Signed-off-by: 2xsec --- src/lxc/commands.c | 15 ++++----------- src/lxc/commands_utils.c | 33 ++++++++++++++++++++------------- src/lxc/commands_utils.h | 3 ++- 3 files changed, 26 insertions(+), 25 deletions(-) diff --git a/src/lxc/commands.c b/src/lxc/commands.c index 4a8c24a37..ffa8c3537 100644 --- a/src/lxc/commands.c +++ b/src/lxc/commands.c @@ -1244,24 +1244,17 @@ out_close: int lxc_cmd_init(const char *name, const char *lxcpath, const char *suffix) { - int fd, len, ret; + int fd, ret; char path[LXC_AUDS_ADDR_LEN] = {0}; - char *offset = &path[1]; - /* -2 here because this is an abstract unix socket so it needs a - * leading \0, and we null terminate, so it needs a trailing \0. - * Although null termination isn't required by the API, we do it anyway - * because we print the sockname out sometimes. - */ - len = sizeof(path) - 2; - ret = lxc_make_abstract_socket_name(offset, len, name, lxcpath, NULL, suffix); + ret = lxc_make_abstract_socket_name(path, sizeof(path), name, lxcpath, NULL, suffix); if (ret < 0) return -1; - TRACE("Creating abstract unix socket \"%s\"", offset); + TRACE("Creating abstract unix socket \"%s\"", &path[1]); fd = lxc_abstract_unix_open(path, SOCK_STREAM, 0); if (fd < 0) { - SYSERROR("Failed to create command socket %s", offset); + SYSERROR("Failed to create command socket %s", &path[1]); if (errno == EADDRINUSE) ERROR("Container \"%s\" appears to be already running", name); diff --git a/src/lxc/commands_utils.c b/src/lxc/commands_utils.c index 5d5f55f79..812f5ceeb 100644 --- a/src/lxc/commands_utils.c +++ b/src/lxc/commands_utils.c @@ -96,24 +96,38 @@ int lxc_cmd_sock_get_state(const char *name, const char *lxcpath, return ret; } -int lxc_make_abstract_socket_name(char *path, int len, const char *lxcname, +int lxc_make_abstract_socket_name(char *path, size_t pathlen, + const char *lxcname, const char *lxcpath, const char *hashed_sock_name, const char *suffix) { const char *name; + char *offset; char *tmppath; + size_t len; size_t tmplen; uint64_t hash; int ret; + if (!path) + return -1; + + offset = &path[1]; + + /* -2 here because this is an abstract unix socket so it needs a + * leading \0, and we null terminate, so it needs a trailing \0. + * Although null termination isn't required by the API, we do it anyway + * because we print the sockname out sometimes. + */ + len = pathlen -2; + name = lxcname; if (!name) name = ""; if (hashed_sock_name != NULL) { - ret = - snprintf(path, len, "lxc/%s/%s", hashed_sock_name, suffix); + ret = snprintf(offset, len, "lxc/%s/%s", hashed_sock_name, suffix); if (ret < 0 || ret >= len) { ERROR("Failed to create abstract socket name"); return -1; @@ -129,7 +143,7 @@ int lxc_make_abstract_socket_name(char *path, int len, const char *lxcname, } } - ret = snprintf(path, len, "%s/%s/%s", lxcpath, name, suffix); + ret = snprintf(offset, len, "%s/%s/%s", lxcpath, name, suffix); if (ret < 0) { ERROR("Failed to create abstract socket name"); return -1; @@ -147,7 +161,7 @@ int lxc_make_abstract_socket_name(char *path, int len, const char *lxcname, } hash = fnv_64a_buf(tmppath, ret, FNV1A_64_INIT); - ret = snprintf(path, len, "lxc/%016" PRIx64 "/%s", hash, suffix); + ret = snprintf(offset, len, "lxc/%016" PRIx64 "/%s", hash, suffix); if (ret < 0 || ret >= len) { ERROR("Failed to create abstract socket name"); return -1; @@ -161,15 +175,8 @@ int lxc_cmd_connect(const char *name, const char *lxcpath, { int ret, client_fd; char path[LXC_AUDS_ADDR_LEN] = {0}; - char *offset = &path[1]; - /* -2 here because this is an abstract unix socket so it needs a - * leading \0, and we null terminate, so it needs a trailing \0. - * Although null termination isn't required by the API, we do it anyway - * because we print the sockname out sometimes. - */ - size_t len = sizeof(path) - 2; - ret = lxc_make_abstract_socket_name(offset, len, name, lxcpath, + ret = lxc_make_abstract_socket_name(path, sizeof(path), name, lxcpath, hashed_sock_name, suffix); if (ret < 0) return -1; diff --git a/src/lxc/commands_utils.h b/src/lxc/commands_utils.h index d54cb11f3..c1583b785 100644 --- a/src/lxc/commands_utils.h +++ b/src/lxc/commands_utils.h @@ -25,7 +25,8 @@ #include "state.h" #include "commands.h" -int lxc_make_abstract_socket_name(char *path, int len, const char *lxcname, +int lxc_make_abstract_socket_name(char *path, size_t pathlen, + const char *lxcname, const char *lxcpath, const char *hashed_sock_name, const char *suffix); From 95e523c8565b323973c50ee9c402f855cb78fcd5 Mon Sep 17 00:00:00 2001 From: 2xsec Date: Fri, 12 Oct 2018 16:05:31 +0900 Subject: [PATCH 4/4] monitor: fix coding standard Signed-off-by: 2xsec --- src/lxc/monitor.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/lxc/monitor.c b/src/lxc/monitor.c index 64dd3511e..f8b7a8b7c 100644 --- a/src/lxc/monitor.c +++ b/src/lxc/monitor.c @@ -168,7 +168,8 @@ int lxc_monitor_close(int fd) * have a maximum of 106 chars. But to not break backwards compatibility we keep * the limit at 105. */ -int lxc_monitor_sock_name(const char *lxcpath, struct sockaddr_un *addr) { +int lxc_monitor_sock_name(const char *lxcpath, struct sockaddr_un *addr) +{ size_t len; int ret; char *path;