af_unix: fix return value & cleanups

Signed-off-by: 2xsec <dh48.jeong@samsung.com>

diff --git a/src/lxc/af_unix.c b/src/lxc/af_unix.c
index 86f1fe9fdf62864cb72753701fff226939262622..0afc6590cc99c52d9429428ba7cd6dfc8fe8abb9 100644 (file)
--- 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 9dfccd16e96acd6d05dc578d8d31b6f6f5c5e6a9..f2c2fdcc6eeae2d8f1b77aaeae7b66e60e2a9252 100644 (file)
--- 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 8ac83a6c9121c1334a2761fb80809ef76d8f533c..9bc3e23d19540ae3a630690028751cf97c23a4b2 100644 (file)
--- 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 298f431efb054e171f47ee1732db7c30ee8983fa..38eee0a9bb1297140b398f9bcc2cfe53b778d88a 100644 (file)
--- 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 9bf96fa8d437eea6f23d34524463aea3b35ec67d..30d6b6047aae2304a16464af2fe9037241585589 100644 (file)
--- 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 180a37ab44d838ac93bca8d19ae9ca27e62a8172..a970d3c154cbf6efb1d69ae4a8548a750732dbf2 100644 (file)
--- 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");