diff --git a/src/lxc/af_unix.c b/src/lxc/af_unix.c index 46d8e50fc..ac839943e 100644 --- a/src/lxc/af_unix.c +++ b/src/lxc/af_unix.c @@ -55,8 +55,9 @@ int lxc_abstract_unix_open(const char *path, int type, int flags) addr.sun_family = AF_UNIX; - len = strlen(&path[1]) + 1; - if (len >= sizeof(addr.sun_path) - 1) { + len = strlen(&path[1]); + /* do not enforce \0-termination */ + if (len >= sizeof(addr.sun_path)) { close(fd); errno = ENAMETOOLONG; return -1; @@ -64,7 +65,7 @@ int lxc_abstract_unix_open(const char *path, int type, int flags) /* addr.sun_path[0] has already been set to 0 by memset() */ strncpy(&addr.sun_path[1], &path[1], strlen(&path[1])); - if (bind(fd, (struct sockaddr *)&addr, offsetof(struct sockaddr_un, sun_path) + len)) { + if (bind(fd, (struct sockaddr *)&addr, offsetof(struct sockaddr_un, sun_path) + len + 1)) { int tmp = errno; close(fd); errno = tmp; @@ -109,8 +110,9 @@ int lxc_abstract_unix_connect(const char *path) addr.sun_family = AF_UNIX; - len = strlen(&path[1]) + 1; - if (len >= sizeof(addr.sun_path) - 1) { + len = strlen(&path[1]); + /* do not enforce \0-termination */ + if (len >= sizeof(addr.sun_path)) { close(fd); errno = ENAMETOOLONG; return -1; @@ -118,7 +120,7 @@ int lxc_abstract_unix_connect(const char *path) /* addr.sun_path[0] has already been set to 0 by memset() */ strncpy(&addr.sun_path[1], &path[1], strlen(&path[1])); - if (connect(fd, (struct sockaddr *)&addr, offsetof(struct sockaddr_un, sun_path) + len)) { + if (connect(fd, (struct sockaddr *)&addr, offsetof(struct sockaddr_un, sun_path) + len + 1)) { int tmp = errno; /* special case to connect to older containers */ if (connect(fd, (struct sockaddr *)&addr, sizeof(addr)) == 0) @@ -136,8 +138,8 @@ int lxc_abstract_unix_send_fd(int fd, int sendfd, void *data, size_t size) struct msghdr msg = { 0 }; struct iovec iov; struct cmsghdr *cmsg; - char cmsgbuf[CMSG_SPACE(sizeof(int))]; - char buf[1]; + char cmsgbuf[CMSG_SPACE(sizeof(int))] = {0}; + char buf[1] = {0}; int *val; msg.msg_control = cmsgbuf; @@ -166,9 +168,9 @@ int lxc_abstract_unix_recv_fd(int fd, int *recvfd, void *data, size_t size) struct msghdr msg = { 0 }; struct iovec iov; struct cmsghdr *cmsg; - char cmsgbuf[CMSG_SPACE(sizeof(int))]; - char buf[1]; int ret, *val; + char cmsgbuf[CMSG_SPACE(sizeof(int))] = {0}; + char buf[1] = {0}; msg.msg_name = NULL; msg.msg_namelen = 0; @@ -210,8 +212,8 @@ int lxc_abstract_unix_send_credential(int fd, void *data, size_t size) .uid = getuid(), .gid = getgid(), }; - char cmsgbuf[CMSG_SPACE(sizeof(cred))]; - char buf[1]; + char cmsgbuf[CMSG_SPACE(sizeof(cred))] = {0}; + char buf[1] = {0}; msg.msg_control = cmsgbuf; msg.msg_controllen = sizeof(cmsgbuf); @@ -239,9 +241,9 @@ int lxc_abstract_unix_rcv_credential(int fd, void *data, size_t size) struct iovec iov; struct cmsghdr *cmsg; struct ucred cred; - char cmsgbuf[CMSG_SPACE(sizeof(cred))]; - char buf[1]; int ret; + char cmsgbuf[CMSG_SPACE(sizeof(cred))] = {0}; + char buf[1] = {0}; msg.msg_name = NULL; msg.msg_namelen = 0; diff --git a/src/lxc/af_unix.h b/src/lxc/af_unix.h index 3f5d01fe1..d25a2118a 100644 --- a/src/lxc/af_unix.h +++ b/src/lxc/af_unix.h @@ -24,8 +24,10 @@ #ifndef __LXC_AF_UNIX_H #define __LXC_AF_UNIX_H +/* does not enforce \0-termination */ extern int lxc_abstract_unix_open(const char *path, int type, int flags); extern int lxc_abstract_unix_close(int fd); +/* does not enforce \0-termination */ extern int lxc_abstract_unix_connect(const char *path); extern int lxc_abstract_unix_send_fd(int fd, int sendfd, void *data, size_t size); extern int lxc_abstract_unix_recv_fd(int fd, int *recvfd, void *data, size_t size); diff --git a/src/lxc/commands.c b/src/lxc/commands.c index b17879b54..27c8c084f 100644 --- a/src/lxc/commands.c +++ b/src/lxc/commands.c @@ -74,14 +74,19 @@ lxc_log_define(lxc_commands, lxc); -static int fill_sock_name(char *path, int len, const char *name, +static int fill_sock_name(char *path, int len, const char *lxcname, const char *lxcpath, const char *hashed_sock_name) { + const char *name; char *tmppath; size_t tmplen; uint64_t hash; int ret; + name = lxcname; + if (!name) + name = ""; + if (hashed_sock_name != NULL) { ret = snprintf(path, len, "lxc/%s/command", hashed_sock_name); if (ret < 0 || ret >= len) { @@ -193,8 +198,11 @@ static int lxc_cmd_rsp_recv(int sock, struct lxc_cmd_rr *cmd) rsp->data = rspdata; } - if (rsp->datalen == 0) + if (rsp->datalen == 0) { + DEBUG("command %s response data length is 0", + lxc_cmd_str(cmd->req.cmd)); return ret; + } if (rsp->datalen > LXC_CMD_DATA_MAX) { ERROR("Command %s response data %d too long.", lxc_cmd_str(cmd->req.cmd), rsp->datalen); @@ -274,7 +282,7 @@ static int lxc_cmd(const char *name, struct lxc_cmd_rr *cmd, int *stopped, int sock, ret = -1; char path[sizeof(((struct sockaddr_un *)0)->sun_path)] = { 0 }; char *offset = &path[1]; - int len; + size_t len; int stay_connected = cmd->req.cmd == LXC_CMD_CONSOLE; *stopped = 0; @@ -982,7 +990,7 @@ int lxc_cmd_init(const char *name, struct lxc_handler *handler, * Although null termination isn't required by the API, we do it anyway * because we print the sockname out sometimes. */ - len = sizeof(path)-2; + len = sizeof(path) - 2; if (fill_sock_name(offset, len, name, lxcpath, NULL)) return -1; diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c index 3cee18ca7..ebbcfe86c 100644 --- a/src/lxc/lxccontainer.c +++ b/src/lxc/lxccontainer.c @@ -4370,7 +4370,10 @@ int list_active_containers(const char *lxcpath, char ***nret, *p2 = '\0'; if (is_hashed) { - if (strncmp(lxcpath, lxc_cmd_get_lxcpath(p), lxcpath_len) != 0) + char *recvpath = lxc_cmd_get_lxcpath(p); + if (!recvpath) + continue; + if (strncmp(lxcpath, recvpath, lxcpath_len) != 0) continue; p = lxc_cmd_get_name(p); } diff --git a/src/lxc/monitor.c b/src/lxc/monitor.c index 410a0f45c..1758402a9 100644 --- a/src/lxc/monitor.c +++ b/src/lxc/monitor.c @@ -153,36 +153,52 @@ int lxc_monitor_close(int fd) return close(fd); } +/* Enforces \0-termination for the abstract unix socket. This is not required + * but allows us to print it out. + * + * Older version of liblxc only allowed for 105 bytes to be used for the + * abstract unix domain socket name because the code for our abstract unix + * socket handling performed invalid checks. Since we \0-terminate we could now + * 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) { size_t len; int ret; - char *sockname; char *path; uint64_t hash; /* addr.sun_path is only 108 bytes, so we hash the full name and * then append as much of the name as we can fit. */ - sockname = &addr->sun_path[1]; memset(addr, 0, sizeof(*addr)); addr->sun_family = AF_UNIX; + /* strlen("lxc/") + strlen("/monitor-sock") + 1 = 18 */ len = strlen(lxcpath) + 18; path = alloca(len); ret = snprintf(path, len, "lxc/%s/monitor-sock", lxcpath); if (ret < 0 || (size_t)ret >= len) { - ERROR("Failed to create path for monitor."); + ERROR("failed to create name for monitor socket"); return -1; } + /* Note: snprintf() will \0-terminate addr->sun_path on the 106th byte + * and so the abstract socket name has 105 "meaningful" characters. This + * is absolutely intentional. For further info read the comment for this + * function above! + */ len = sizeof(addr->sun_path) - 1; hash = fnv_64a_buf(path, ret, FNV1A_64_INIT); - ret = snprintf(sockname, len, "lxc/%016" PRIx64 "/%s", hash, lxcpath); - if (ret < 0) + 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; + } - sockname[sizeof(addr->sun_path)-3] = '\0'; - INFO("Using monitor socket name \"%s\".", sockname); + /* 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); return 0; } @@ -193,7 +209,8 @@ int lxc_monitor_open(const char *lxcpath) int fd; size_t retry; size_t len; - int ret = 0, backoff_ms[] = {10, 50, 100}; + int ret = -1; + int backoff_ms[] = {10, 50, 100}; if (lxc_monitor_sock_name(lxcpath, &addr) < 0) return -1; @@ -201,28 +218,32 @@ int lxc_monitor_open(const char *lxcpath) fd = socket(PF_UNIX, SOCK_STREAM, 0); if (fd < 0) { ERROR("Failed to create socket: %s.", strerror(errno)); - return -1; + return -errno; } - len = strlen(&addr.sun_path[1]) + 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) { - ret = -1; errno = ENAMETOOLONG; + ret = -errno; + ERROR("name of monitor socket too long (%zu bytes): %s", len, strerror(errno)); goto on_error; } for (retry = 0; retry < sizeof(backoff_ms) / sizeof(backoff_ms[0]); retry++) { - ret = connect(fd, (struct sockaddr *)&addr, offsetof(struct sockaddr_un, sun_path) + len); - if (ret == 0 || errno != ECONNREFUSED) + fd = lxc_abstract_unix_connect(addr.sun_path); + if (fd < 0 || errno != ECONNREFUSED) break; - ERROR("Failed to connect to monitor socket. Retrying in %d ms.", backoff_ms[retry]); + ERROR("Failed to connect to monitor socket. Retrying in %d ms: %s", backoff_ms[retry], strerror(errno)); usleep(backoff_ms[retry] * 1000); } - if (ret < 0) { + if (fd < 0) { + ret = -errno; ERROR("Failed to connect to monitor socket: %s.", strerror(errno)); goto on_error; } + ret = 0; return fd;