From: 2xsec Date: Fri, 20 Jul 2018 17:41:53 +0000 (+0900) Subject: af_unix: fix return value & cleanups X-Git-Tag: lxc-3.1.0~200^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=9044b79e191a09f9a6a41138e54a5fbf579a0a1a;p=thirdparty%2Flxc.git af_unix: fix return value & cleanups Signed-off-by: 2xsec --- diff --git a/src/lxc/af_unix.c b/src/lxc/af_unix.c index 86f1fe9fd..0afc6590c 100644 --- a/src/lxc/af_unix.c +++ b/src/lxc/af_unix.c @@ -74,30 +74,28 @@ int lxc_abstract_unix_open(const char *path, int type, int flags) ret = bind(fd, (struct sockaddr *)&addr, offsetof(struct sockaddr_un, sun_path) + len + 1); if (ret < 0) { - int tmp = errno; + int saved_erron = errno; close(fd); - errno = tmp; + errno = saved_erron; return -1; } if (type == SOCK_STREAM) { ret = listen(fd, 100); if (ret < 0) { - int tmp = errno; + int saved_erron = errno; close(fd); - errno = tmp; + errno = saved_erron; return -1; } - } return fd; } -int lxc_abstract_unix_close(int fd) +void lxc_abstract_unix_close(int fd) { close(fd); - return 0; } int lxc_abstract_unix_connect(const char *path) @@ -128,7 +126,9 @@ int lxc_abstract_unix_connect(const char *path) ret = connect(fd, (struct sockaddr *)&addr, offsetof(struct sockaddr_un, sun_path) + len + 1); if (ret < 0) { + int saved_errno = errno; close(fd); + errno = saved_errno; return -1; } @@ -150,8 +150,10 @@ int lxc_abstract_unix_send_fds(int fd, int *sendfds, int num_sendfds, memset(&iov, 0, sizeof(iov)); cmsgbuf = malloc(cmsgbufsize); - if (!cmsgbuf) + if (!cmsgbuf) { + errno = ENOMEM; return -1; + } msg.msg_control = cmsgbuf; msg.msg_controllen = cmsgbufsize; @@ -190,8 +192,10 @@ int lxc_abstract_unix_recv_fds(int fd, int *recvfds, int num_recvfds, memset(&iov, 0, sizeof(iov)); cmsgbuf = malloc(cmsgbufsize); - if (!cmsgbuf) + if (!cmsgbuf) { + errno = ENOMEM; return -1; + } msg.msg_control = cmsgbuf; msg.msg_controllen = cmsgbufsize; @@ -209,9 +213,8 @@ int lxc_abstract_unix_recv_fds(int fd, int *recvfds, int num_recvfds, memset(recvfds, -1, num_recvfds * sizeof(int)); if (cmsg && cmsg->cmsg_len == CMSG_LEN(num_recvfds * sizeof(int)) && - cmsg->cmsg_level == SOL_SOCKET && cmsg->cmsg_type == SCM_RIGHTS) { + cmsg->cmsg_level == SOL_SOCKET && cmsg->cmsg_type == SCM_RIGHTS) memcpy(recvfds, CMSG_DATA(cmsg), num_recvfds * sizeof(int)); - } out: free(cmsgbuf); @@ -281,10 +284,12 @@ int lxc_abstract_unix_rcv_credential(int fd, void *data, size_t size) memcpy(&cred, CMSG_DATA(cmsg), sizeof(cred)); if (cred.uid && (cred.uid != getuid() || cred.gid != getgid())) { - INFO("message denied for '%d/%d'", cred.uid, cred.gid); - return -EACCES; + INFO("Message denied for '%d/%d'", cred.uid, cred.gid); + errno = EACCES; + return -1; } } + out: return ret; } diff --git a/src/lxc/af_unix.h b/src/lxc/af_unix.h index 9dfccd16e..f2c2fdcc6 100644 --- a/src/lxc/af_unix.h +++ b/src/lxc/af_unix.h @@ -28,7 +28,7 @@ /* 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); +extern void 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_fds(int fd, int *sendfds, int num_sendfds, diff --git a/src/lxc/attach.c b/src/lxc/attach.c index 8ac83a6c9..9bc3e23d1 100644 --- a/src/lxc/attach.c +++ b/src/lxc/attach.c @@ -828,8 +828,13 @@ static int attach_child_main(struct attach_clone_payload *payload) */ if (needs_lsm) { ret = lxc_abstract_unix_recv_fds(payload->ipc_socket, &lsm_fd, 1, NULL, 0); - if (ret <= 0) + if (ret <= 0) { + if (ret < 0) + SYSERROR("Failed to receive lsm label fd"); + goto on_error; + } + TRACE("Received LSM label file descriptor %d from parent", lsm_fd); } @@ -1330,11 +1335,15 @@ int lxc_attach(const char *name, const char *lxcpath, /* Send child fd of the LSM security module to write to. */ ret = lxc_abstract_unix_send_fds(ipc_sockets[0], &labelfd, 1, NULL, 0); - close(labelfd); if (ret <= 0) { - SYSERROR("%d", (int)ret); + if (ret < 0) + SYSERROR("Failed to send lsm label fd"); + + close(labelfd); goto close_mainloop; } + + close(labelfd); TRACE("Sent LSM label file descriptor %d to child", labelfd); } diff --git a/src/lxc/cmd/lxc_monitord.c b/src/lxc/cmd/lxc_monitord.c index 298f431ef..38eee0a9b 100644 --- a/src/lxc/cmd/lxc_monitord.c +++ b/src/lxc/cmd/lxc_monitord.c @@ -97,8 +97,8 @@ static int lxc_monitord_fifo_create(struct lxc_monitor *mon) mon->fifofd = open(fifo_path, O_RDWR); if (mon->fifofd < 0) { + SYSERROR("Failed to open monitor fifo %s", fifo_path); unlink(fifo_path); - ERROR("Failed to open monitor fifo."); return -1; } @@ -106,12 +106,14 @@ static int lxc_monitord_fifo_create(struct lxc_monitor *mon) lk.l_whence = SEEK_SET; lk.l_start = 0; lk.l_len = 0; + if (fcntl(mon->fifofd, F_SETLK, &lk) != 0) { /* another lxc-monitord is already running, don't start up */ - DEBUG("lxc-monitord already running on lxcpath %s.", mon->lxcpath); + SYSDEBUG("lxc-monitord already running on lxcpath %s", mon->lxcpath); close(mon->fifofd); return -1; } + return 0; } @@ -132,15 +134,15 @@ static void lxc_monitord_sockfd_remove(struct lxc_monitor *mon, int fd) { int i; if (lxc_mainloop_del_handler(&mon->descr, fd)) - CRIT("File descriptor %d not found in mainloop.", fd); + CRIT("File descriptor %d not found in mainloop", fd); close(fd); - for (i = 0; i < mon->clientfds_cnt; i++) { + for (i = 0; i < mon->clientfds_cnt; i++) if (mon->clientfds[i] == fd) break; - } + if (i >= mon->clientfds_cnt) { - CRIT("File descriptor %d not found in clients array.", fd); + CRIT("File descriptor %d not found in clients array", fd); lxc_monitord_cleanup(); exit(EXIT_FAILURE); } @@ -166,6 +168,7 @@ static int lxc_monitord_sock_handler(int fd, uint32_t events, void *data, if (events & EPOLLHUP) lxc_monitord_sockfd_remove(mon, fd); + return quit; } @@ -180,53 +183,55 @@ static int lxc_monitord_sock_accept(int fd, uint32_t events, void *data, ret = LXC_MAINLOOP_ERROR; clientfd = accept(fd, NULL, 0); if (clientfd < 0) { - SYSERROR("Failed to accept connection for client file descriptor %d.", fd); + SYSERROR("Failed to accept connection for client file descriptor %d", fd); goto out; } if (fcntl(clientfd, F_SETFD, FD_CLOEXEC)) { - SYSERROR("Failed to set FD_CLOEXEC on client socket connection %d.", clientfd); + SYSERROR("Failed to set FD_CLOEXEC on client socket connection %d", clientfd); goto err1; } - if (getsockopt(clientfd, SOL_SOCKET, SO_PEERCRED, &cred, &credsz)) - { - ERROR("Failed to get credentials on client socket connection %d.", clientfd); + if (getsockopt(clientfd, SOL_SOCKET, SO_PEERCRED, &cred, &credsz)) { + SYSERROR("Failed to get credentials on client socket connection %d", clientfd); goto err1; } + if (cred.uid && cred.uid != geteuid()) { - WARN("Monitor denied for uid %d on client socket connection %d.", cred.uid, clientfd); - ret = -EACCES; + WARN("Monitor denied for uid %d on client socket connection %d", cred.uid, clientfd); goto err1; } if (mon->clientfds_cnt + 1 > mon->clientfds_size) { int *clientfds; + clientfds = realloc(mon->clientfds, (mon->clientfds_size + CLIENTFDS_CHUNK) * sizeof(mon->clientfds[0])); if (clientfds == NULL) { - ERROR("Failed to realloc memory for %d client file " - "descriptors.", + ERROR("Failed to realloc memory for %d client file descriptors", mon->clientfds_size + CLIENTFDS_CHUNK); goto err1; } + mon->clientfds = clientfds; mon->clientfds_size += CLIENTFDS_CHUNK; } ret = lxc_mainloop_add_handler(&mon->descr, clientfd, lxc_monitord_sock_handler, mon); - if (ret) { - ERROR("Failed to add socket handler."); + if (ret < 0) { + ERROR("Failed to add socket handler"); goto err1; } mon->clientfds[mon->clientfds_cnt++] = clientfd; - INFO("Accepted client file descriptor %d. Number of accepted file descriptors is now %d.", clientfd, mon->clientfds_cnt); + INFO("Accepted client file descriptor %d. Number of accepted file descriptors is now %d", + clientfd, mon->clientfds_cnt); goto out; err1: close(clientfd); + out: return ret; } @@ -255,8 +260,10 @@ static int lxc_monitord_sock_delete(struct lxc_monitor *mon) if (lxc_monitor_sock_name(mon->lxcpath, &addr) < 0) return -1; + if (addr.sun_path[0]) unlink(addr.sun_path); + return 0; } @@ -268,8 +275,7 @@ static int lxc_monitord_create(struct lxc_monitor *mon) if (ret < 0) return ret; - ret = lxc_monitord_sock_create(mon); - return ret; + return lxc_monitord_sock_create(mon); } static void lxc_monitord_delete(struct lxc_monitor *mon) @@ -277,7 +283,7 @@ static void lxc_monitord_delete(struct lxc_monitor *mon) int i; lxc_mainloop_del_handler(&mon->descr, mon->listenfd); - close(mon->listenfd); + lxc_abstract_unix_close(mon->listenfd); lxc_monitord_sock_delete(mon); lxc_mainloop_del_handler(&mon->descr, mon->fifofd); @@ -288,6 +294,7 @@ static void lxc_monitord_delete(struct lxc_monitor *mon) lxc_mainloop_del_handler(&mon->descr, mon->clientfds[i]); close(mon->clientfds[i]); } + mon->clientfds_cnt = 0; } @@ -321,14 +328,14 @@ static int lxc_monitord_mainloop_add(struct lxc_monitor *mon) ret = lxc_mainloop_add_handler(&mon->descr, mon->fifofd, lxc_monitord_fifo_handler, mon); if (ret < 0) { - ERROR("Failed to add to mainloop monitor handler for fifo."); + ERROR("Failed to add to mainloop monitor handler for fifo"); return -1; } ret = lxc_mainloop_add_handler(&mon->descr, mon->listenfd, lxc_monitord_sock_accept, mon); if (ret < 0) { - ERROR("Failed to add to mainloop monitor handler for listen socket."); + ERROR("Failed to add to mainloop monitor handler for listen socket"); return -1; } @@ -374,9 +381,10 @@ int main(int argc, char *argv[]) log.prefix = "lxc-monitord"; log.quiet = 0; log.lxcpath = lxcpath; + ret = lxc_log_init(&log); if (ret) - INFO("Failed to open log file %s, log will be lost.", lxcpath); + INFO("Failed to open log file %s, log will be lost", lxcpath); lxc_log_options_no_override(); if (lxc_safe_int(argv[2], &pipefd) < 0) @@ -388,7 +396,7 @@ int main(int argc, char *argv[]) sigdelset(&mask, SIGBUS) || sigdelset(&mask, SIGTERM) || pthread_sigmask(SIG_BLOCK, &mask, NULL)) { - SYSERROR("Failed to set signal mask."); + SYSERROR("Failed to set signal mask"); exit(EXIT_FAILURE); } @@ -401,10 +409,11 @@ int main(int argc, char *argv[]) goto on_signal; ret = EXIT_FAILURE; + memset(&mon, 0, sizeof(mon)); mon.lxcpath = lxcpath; if (lxc_mainloop_open(&mon.descr)) { - ERROR("Failed to create mainloop."); + ERROR("Failed to create mainloop"); goto on_error; } mainloop_opened = true; @@ -424,33 +433,38 @@ int main(int argc, char *argv[]) close(pipefd); if (lxc_monitord_mainloop_add(&mon)) { - ERROR("Failed to add mainloop handlers."); + ERROR("Failed to add mainloop handlers"); goto on_error; } - NOTICE("lxc-monitord with pid %d is now monitoring lxcpath %s.", + NOTICE("lxc-monitord with pid %d is now monitoring lxcpath %s", lxc_raw_getpid(), mon.lxcpath); + for (;;) { ret = lxc_mainloop(&mon.descr, 1000 * 30); if (ret) { ERROR("mainloop returned an error"); break; } + if (mon.clientfds_cnt <= 0) { - NOTICE("No remaining clients. lxc-monitord is exiting."); + NOTICE("No remaining clients. lxc-monitord is exiting"); break; } - if (quit == 1) { - NOTICE("got quit command. lxc-monitord is exitting."); + + if (quit == LXC_MAINLOOP_CLOSE) { + NOTICE("Got quit command. lxc-monitord is exitting"); break; } } on_signal: ret = EXIT_SUCCESS; + on_error: if (monitord_created) lxc_monitord_cleanup(); + if (mainloop_opened) lxc_mainloop_close(&mon.descr); diff --git a/src/lxc/commands.c b/src/lxc/commands.c index 9bf96fa8d..30d6b6047 100644 --- a/src/lxc/commands.c +++ b/src/lxc/commands.c @@ -240,18 +240,21 @@ static int lxc_cmd_send(const char *name, struct lxc_cmd_rr *cmd, ret = lxc_abstract_unix_send_credential(client_fd, &cmd->req, sizeof(cmd->req)); - if (ret < 0 || (size_t)ret != sizeof(cmd->req)) { - close(client_fd); - - if (errno == EPIPE) + if (ret < 0 ) { + if (errno == EPIPE) { + close(client_fd); return -EPIPE; - - if (ret >= 0) - return -EMSGSIZE; + } + close(client_fd); return -1; } + if ((size_t)ret != sizeof(cmd->req)) { + close(client_fd); + return -EMSGSIZE; + } + if (cmd->req.datalen <= 0) return client_fd; @@ -754,7 +757,7 @@ static int lxc_cmd_console_callback(int fd, struct lxc_cmd_req *req, rsp.data = INT_TO_PTR(ttynum); ret = lxc_abstract_unix_send_fds(fd, &masterfd, 1, &rsp, sizeof(rsp)); if (ret < 0) { - ERROR("Failed to send tty to client"); + SYSERROR("Failed to send tty to client"); lxc_terminal_free(handler->conf, fd); goto out_close; } @@ -1112,17 +1115,17 @@ static int lxc_cmd_handler(int fd, uint32_t events, void *data, struct lxc_handler *handler = data; ret = lxc_abstract_unix_rcv_credential(fd, &req, sizeof(req)); - if (ret == -EACCES) { - /* We don't care for the peer, just send and close. */ - struct lxc_cmd_rsp rsp = {.ret = ret}; - - lxc_cmd_rsp_send(fd, &rsp); - goto out_close; - } - if (ret < 0) { SYSERROR("Failed to receive data on command socket for command " - "\"%s\"", lxc_cmd_str(req.cmd)); + "\"%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}; + + lxc_cmd_rsp_send(fd, &rsp); + } + goto out_close; } diff --git a/src/lxc/start.c b/src/lxc/start.c index 180a37ab4..a970d3c15 100644 --- a/src/lxc/start.c +++ b/src/lxc/start.c @@ -490,11 +490,17 @@ static int lxc_serve_state_socket_pair(const char *name, again: ret = lxc_abstract_unix_send_credential(handler->state_socket_pair[1], &(int){state}, sizeof(int)); - if (ret != sizeof(int)) { + if (ret < 0) { + SYSERROR("Failed to send state to %d", handler->state_socket_pair[1]); + if (errno == EINTR) goto again; - SYSERROR("Failed to send state to %d", - handler->state_socket_pair[1]); + + return -1; + } + + if (ret != sizeof(int)) { + ERROR("Message too long : %d", handler->state_socket_pair[1]); return -1; } @@ -649,7 +655,7 @@ void lxc_free_handler(struct lxc_handler *handler) if (handler->conf && handler->conf->reboot == REBOOT_NONE) if (handler->conf->maincmd_fd >= 0) - close(handler->conf->maincmd_fd); + lxc_abstract_unix_close(handler->conf->maincmd_fd); if (handler->state_socket_pair[0] >= 0) close(handler->state_socket_pair[0]); @@ -671,6 +677,7 @@ struct lxc_handler *lxc_init_handler(const char *name, struct lxc_conf *conf, handler = malloc(sizeof(*handler)); if (!handler) return NULL; + memset(handler, 0, sizeof(*handler)); /* Note that am_guest_unpriv() checks the effective uid. We @@ -704,6 +711,7 @@ struct lxc_handler *lxc_init_handler(const char *name, struct lxc_conf *conf, ERROR("Failed to create anonymous pair of unix sockets"); goto on_error; } + TRACE("Created anonymous pair {%d,%d} of unix sockets", handler->state_socket_pair[0], handler->state_socket_pair[1]); @@ -716,6 +724,7 @@ struct lxc_handler *lxc_init_handler(const char *name, struct lxc_conf *conf, goto on_error; } } + TRACE("Unix domain socket %d for command server is ready", handler->conf->maincmd_fd); @@ -866,7 +875,7 @@ out_delete_tty: out_aborting: (void)lxc_set_state(name, handler, ABORTING); out_close_maincmd_fd: - close(conf->maincmd_fd); + lxc_abstract_unix_close(conf->maincmd_fd); conf->maincmd_fd = -1; return -1; } @@ -953,7 +962,7 @@ void lxc_fini(const char *name, struct lxc_handler *handler) * the command socket causing a new process to get ECONNREFUSED * because we haven't yet closed the command socket. */ - close(handler->conf->maincmd_fd); + lxc_abstract_unix_close(handler->conf->maincmd_fd); handler->conf->maincmd_fd = -1; TRACE("Closed command socket");