af_unix: fix return value & cleanups

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;
 }

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,

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);
 		}
 

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);
 

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)) {
+	if (ret < 0 ) {
+		if (errno == EPIPE) {
 			close(client_fd);
-
-		if (errno == EPIPE)
 			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) {
+	if (ret < 0) {
+		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};
 
 			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));
 		goto out_close;
 	}
 

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");