Merge pull request #2584 from brauner/2018-09-03/bugfixes

src/lxc/commands.c

@@ -97,7 +97,7 @@ static const char *lxc_cmd_str(lxc_cmd_t cmd)
 	};
 
 	if (cmd >= LXC_CMD_MAX)
-		return "Unknown cmd";
+		return "Invalid request";
 
 	return cmdname[cmd];
 }
@@ -130,7 +130,7 @@ static int lxc_cmd_rsp_recv(int sock, struct lxc_cmd_rr *cmd)
 		        lxc_cmd_str(cmd->req.cmd));
 
 		if (errno == ECONNRESET)
-			return -ECONNRESET;
+			return -1;
 
 		return -1;
 	}
@@ -147,10 +147,12 @@ static int lxc_cmd_rsp_recv(int sock, struct lxc_cmd_rr *cmd)
 
 		rspdata = malloc(sizeof(*rspdata));
 		if (!rspdata) {
+			errno = ENOMEM;
 			ERROR("Failed to allocate response buffer for command \"%s\"",
 			      lxc_cmd_str(cmd->req.cmd));
-			return -ENOMEM;
+			return -1;
 		}
+
 		rspdata->masterfd = rspfd;
 		rspdata->ttynum = PTR_TO_INT(rsp->data);
 		rsp->data = rspdata;
@@ -164,10 +166,9 @@ static int lxc_cmd_rsp_recv(int sock, struct lxc_cmd_rr *cmd)
 
 	if ((rsp->datalen > LXC_CMD_DATA_MAX) &&
 	    (cmd->req.cmd != LXC_CMD_CONSOLE_LOG)) {
-		errno = EFBIG;
-		SYSERROR("Response data for command \"%s\" is too long: %d bytes > %d",
-		         lxc_cmd_str(cmd->req.cmd), rsp->datalen, LXC_CMD_DATA_MAX);
-		return -EFBIG;
+		ERROR("Response data for command \"%s\" is too long: %d bytes > %d",
+		      lxc_cmd_str(cmd->req.cmd), rsp->datalen, LXC_CMD_DATA_MAX);
+		return -1;
 	}
 
 	if (cmd->req.cmd == LXC_CMD_CONSOLE_LOG) {
@@ -178,17 +179,16 @@ static int lxc_cmd_rsp_recv(int sock, struct lxc_cmd_rr *cmd)
 	}
 	if (!rsp->data) {
 		errno = ENOMEM;
-		SYSERROR("Failed to allocate response buffer for command \"%s\"",
-		         lxc_cmd_str(cmd->req.cmd));
-		return -ENOMEM;
+		ERROR("Failed to allocate response buffer for command \"%s\"",
+		      lxc_cmd_str(cmd->req.cmd));
+		return -1;
 	}
 
-	ret = recv(sock, rsp->data, rsp->datalen, 0);
+	ret = lxc_recv_nointr(sock, rsp->data, rsp->datalen, 0);
 	if (ret != rsp->datalen) {
 		SYSERROR("Failed to receive response data for command \"%s\"",
 		         lxc_cmd_str(cmd->req.cmd));
-		if (ret >= 0)
-			ret = -1;
+		return -1;
 	}
 
 	return ret;
@@ -206,7 +206,8 @@ static int lxc_cmd_rsp_send(int fd, struct lxc_cmd_rsp *rsp)
 {
 	ssize_t ret;
 
-	ret = send(fd, rsp, sizeof(*rsp), MSG_NOSIGNAL);
+	errno = EMSGSIZE;
+	ret = lxc_send_nointr(fd, rsp, sizeof(*rsp), MSG_NOSIGNAL);
 	if (ret < 0 || (size_t)ret != sizeof(*rsp)) {
 		SYSERROR("Failed to send command response %zd", ret);
 		return -1;
@@ -215,7 +216,8 @@ static int lxc_cmd_rsp_send(int fd, struct lxc_cmd_rsp *rsp)
 	if (!rsp->data || rsp->datalen <= 0)
 		return 0;
 
-	ret = send(fd, rsp->data, rsp->datalen, MSG_NOSIGNAL);
+	errno = EMSGSIZE;
+	ret = lxc_send_nointr(fd, rsp->data, rsp->datalen, MSG_NOSIGNAL);
 	if (ret < 0 || ret != (ssize_t)rsp->datalen) {
 		SYSWARN("Failed to send command response data %zd", ret);
 		return -1;
@@ -227,51 +229,35 @@ static int lxc_cmd_rsp_send(int fd, struct lxc_cmd_rsp *rsp)
 static int lxc_cmd_send(const char *name, struct lxc_cmd_rr *cmd,
 			const char *lxcpath, const char *hashed_sock_name)
 {
-	int client_fd;
+	int client_fd, saved_errno;
 	ssize_t ret = -1;
 
 	client_fd = lxc_cmd_connect(name, lxcpath, hashed_sock_name, "command");
-	if (client_fd < 0) {
-		if (client_fd == -ECONNREFUSED)
-			return -ECONNREFUSED;
-
+	if (client_fd < 0)
 		return -1;
-	}
 
 	ret = lxc_abstract_unix_send_credential(client_fd, &cmd->req,
 						sizeof(cmd->req));
-	if (ret < 0 ) {
-		if (errno == EPIPE) {
-			close(client_fd);
-			return -EPIPE;
-		}
-		close(client_fd);
-
-		return -1;
-	}
-
-	if ((size_t)ret != sizeof(cmd->req)) {
-		close(client_fd);
-		return -EMSGSIZE;
-	}
+	if (ret < 0 || (size_t)ret != sizeof(cmd->req))
+		goto on_error;
 
 	if (cmd->req.datalen <= 0)
 		return client_fd;
 
-	ret = send(client_fd, cmd->req.data, cmd->req.datalen, MSG_NOSIGNAL);
-	if (ret < 0 || ret != (ssize_t)cmd->req.datalen) {
-		close(client_fd);
-
-		if (errno == EPIPE)
-			return -EPIPE;
+	errno = EMSGSIZE;
+	ret = lxc_send_nointr(client_fd, (void *)cmd->req.data,
+			      cmd->req.datalen, MSG_NOSIGNAL);
+	if (ret < 0 || ret != (ssize_t)cmd->req.datalen)
+		goto on_error;
 
-		if (ret >= 0)
-			return -EMSGSIZE;
+	return client_fd;
 
-		return -1;
-	}
+on_error:
+	saved_errno = errno;
+	close(client_fd);
+	errno = saved_errno;
 
-	return client_fd;
+	return -1;
 }
 
 /*
@@ -296,7 +282,7 @@ static int lxc_cmd_send(const char *name, struct lxc_cmd_rr *cmd,
 static int lxc_cmd(const char *name, struct lxc_cmd_rr *cmd, int *stopped,
 		   const char *lxcpath, const char *hashed_sock_name)
 {
-	int client_fd;
+	int client_fd, saved_errno;
 	int ret = -1;
 	bool stay_connected = false;
 
@@ -311,24 +297,27 @@ static int lxc_cmd(const char *name, struct lxc_cmd_rr *cmd, int *stopped,
 		SYSTRACE("Command \"%s\" failed to connect command socket",
 		         lxc_cmd_str(cmd->req.cmd));
 
-		if (client_fd == -ECONNREFUSED)
+		if (errno == ECONNREFUSED)
 			*stopped = 1;
 
-		if (client_fd == -EPIPE) {
+		if (errno == EPIPE) {
 			*stopped = 1;
 			client_fd = 0;
 		}
 
-		return client_fd;
+		return -1;
 	}
 
 	ret = lxc_cmd_rsp_recv(client_fd, cmd);
-	if (ret == -ECONNRESET)
+	if (ret < 0 && errno == ECONNRESET)
 		*stopped = 1;
 
 	if (!stay_connected || ret <= 0)
-		if (client_fd >= 0)
+		if (client_fd >= 0) {
+			saved_errno = errno;
 			close(client_fd);
+			errno = saved_errno;
+		}
 
 	if (stay_connected && ret > 0)
 		cmd->rsp.ret = client_fd;
@@ -1181,7 +1170,7 @@ static int lxc_cmd_handler(int fd, uint32_t events, void *data,
 			goto out_close;
 		}
 
-		ret = recv(fd, reqdata, req.datalen, 0);
+		ret = lxc_recv_nointr(fd, reqdata, req.datalen, 0);
 		if (ret != req.datalen) {
 			WARN("Failed to receive full command request. Ignoring "
 			     "request for \"%s\"", lxc_cmd_str(req.cmd));

src/lxc/commands_utils.c

@@ -32,6 +32,7 @@
 #include "commands.h"
 #include "commands_utils.h"
 #include "initutils.h"
+#include "file_utils.h"
 #include "log.h"
 #include "lxclock.h"
 #include "monitor.h"
@@ -61,14 +62,8 @@ int lxc_cmd_sock_rcv_state(int state_client_fd, int timeout)
 
 	memset(&msg, 0, sizeof(msg));
 
-again:
-	ret = recv(state_client_fd, &msg, sizeof(msg), 0);
+	ret = lxc_recv_nointr(state_client_fd, &msg, sizeof(msg), 0);
 	if (ret < 0) {
-		if (errno == EINTR) {
-			TRACE("Caught EINTR; retrying");
-			goto again;
-		}
-
 		SYSERROR("Failed to receive message");
 		return -1;
 	}
@@ -178,11 +173,8 @@ int lxc_cmd_connect(const char *name, const char *lxcpath,
 
 	/* Get new client fd. */
 	client_fd = lxc_abstract_unix_connect(path);
-	if (client_fd < 0) {
-		if (errno == ECONNREFUSED)
-			return -ECONNREFUSED;
+	if (client_fd < 0)
 		return -1;
-	}
 
 	return client_fd;
 }

src/lxc/file_utils.c

@@ -106,6 +106,17 @@ again:
 	return ret;
 }
 
+ssize_t lxc_send_nointr(int sockfd, void *buf, size_t len, int flags)
+{
+	ssize_t ret;
+again:
+	ret = send(sockfd, buf, len, flags);
+	if (ret < 0 && errno == EINTR)
+		goto again;
+
+	return ret;
+}
+
 ssize_t lxc_read_nointr(int fd, void *buf, size_t count)
 {
 	ssize_t ret;
@@ -117,6 +128,17 @@ again:
 	return ret;
 }
 
+ssize_t lxc_recv_nointr(int sockfd, void *buf, size_t len, int flags)
+{
+	ssize_t ret;
+again:
+	ret = recv(sockfd, buf, len, flags);
+	if (ret < 0 && errno == EINTR)
+		goto again;
+
+	return ret;
+}
+
 ssize_t lxc_read_nointr_expect(int fd, void *buf, size_t count, const void *expected_buf)
 {
 	ssize_t ret;

src/lxc/file_utils.h

@@ -37,9 +37,12 @@ extern int lxc_read_from_file(const char *filename, void *buf, size_t count);
 
 /* send and receive buffers completely */
 extern ssize_t lxc_write_nointr(int fd, const void *buf, size_t count);
+extern ssize_t lxc_send_nointr(int sockfd, void *buf, size_t len, int flags);
 extern ssize_t lxc_read_nointr(int fd, void *buf, size_t count);
 extern ssize_t lxc_read_nointr_expect(int fd, void *buf, size_t count,
 				      const void *expected_buf);
+extern ssize_t lxc_recv_nointr(int sockfd, void *buf, size_t len, int flags);
+
 extern bool file_exists(const char *f);
 extern int print_to_file(const char *file, const char *content);
 extern int is_dir(const char *path);

src/lxc/log.c

@@ -311,7 +311,7 @@ static int log_append_logfile(const struct lxc_log_appender *appender,
 		return 0;
 
 	if (lxc_unix_epoch_to_utc(date_time, LXC_LOG_TIME_SIZE, &event->timestamp) < 0)
-		return 0;
+		return -1;
 
 	n = snprintf(buffer, sizeof(buffer),
 		     "%s%s%s %s %-8s %s - %s:%s:%d - ",

src/lxc/log.h

@@ -276,6 +276,7 @@ ATTR_UNUSED static inline void LXC_##LEVEL(struct lxc_log_locinfo* locinfo,	\
 {										\
 	if (lxc_log_priority_is_enabled(acategory, LXC_LOG_LEVEL_##LEVEL)) {	\
 		va_list va_ref;							\
+		int saved_errno;						\
 		struct lxc_log_event evt = {					\
 			.category	= (acategory)->name,			\
 			.priority	= LXC_LOG_LEVEL_##LEVEL,		\
@@ -287,12 +288,14 @@ ATTR_UNUSED static inline void LXC_##LEVEL(struct lxc_log_locinfo* locinfo,	\
 		 * without restrictions. So let's use it for our		\
 		 * logging stamps.						\
 		 */								\
+		saved_errno = errno;						\
 		(void)clock_gettime(CLOCK_REALTIME, &evt.timestamp);		\
 										\
 		va_start(va_ref, format);					\
 		evt.vap = &va_ref;						\
 		__lxc_log(acategory, &evt);					\
 		va_end(va_ref);							\
+		errno = saved_errno;						\
 	}									\
 }
 
@@ -341,7 +344,9 @@ ATTR_UNUSED static inline void LXC_##LEVEL(struct lxc_log_locinfo* locinfo,	\
 			char errno_buf[MAXPATHLEN / 2] = {"Failed to get errno string"}; \
 			char *ptr = NULL;                                                \
 			{                                                                \
+				int saved_errno = errno;				 \
 				ptr = strerror_r(errno, errno_buf, sizeof(errno_buf));   \
+				errno = saved_errno;					 \
 				if (!ptr)                                                \
 					ptr = errno_buf;                                 \
 			}
@@ -350,7 +355,9 @@ ATTR_UNUSED static inline void LXC_##LEVEL(struct lxc_log_locinfo* locinfo,	\
 			char errno_buf[MAXPATHLEN / 2] = {"Failed to get errno string"}; \
 			char *ptr = errno_buf;                                           \
 			{                                                                \
+				int saved_errno = errno;				 \
 				(void)strerror_r(errno, errno_buf, sizeof(errno_buf));   \
+				errno = saved_errno;					 \
 			}
 	#endif
 #elif ENFORCE_THREAD_SAFETY

src/lxc/network.c

@@ -48,6 +48,7 @@
 #include "af_unix.h"
 #include "conf.h"
 #include "config.h"
+#include "file_utils.h"
 #include "log.h"
 #include "macro.h"
 #include "network.h"
@@ -3056,7 +3057,7 @@ int lxc_network_send_veth_names_to_child(struct lxc_handler *handler)
 		if (netdev->type != LXC_NET_VETH)
 			continue;
 
-		ret = send(data_sock, netdev->name, IFNAMSIZ, MSG_NOSIGNAL);
+		ret = lxc_send_nointr(data_sock, netdev->name, IFNAMSIZ, MSG_NOSIGNAL);
 		if (ret < 0)
 			return -1;
 		TRACE("Sent network device name \"%s\" to child", netdev->name);
@@ -3081,7 +3082,7 @@ int lxc_network_recv_veth_names_from_parent(struct lxc_handler *handler)
 		if (netdev->type != LXC_NET_VETH)
 			continue;
 
-		ret = recv(data_sock, netdev->name, IFNAMSIZ, 0);
+		ret = lxc_recv_nointr(data_sock, netdev->name, IFNAMSIZ, 0);
 		if (ret < 0)
 			return -1;
 		TRACE("Received network device name \"%s\" from parent", netdev->name);
@@ -3104,14 +3105,14 @@ int lxc_network_send_name_and_ifindex_to_parent(struct lxc_handler *handler)
 		struct lxc_netdev *netdev = iterator->elem;
 
 		/* Send network device name in the child's namespace to parent. */
-		ret = send(data_sock, netdev->name, IFNAMSIZ, MSG_NOSIGNAL);
+		ret = lxc_send_nointr(data_sock, netdev->name, IFNAMSIZ, MSG_NOSIGNAL);
 		if (ret < 0)
 			return -1;
 
 		/* Send network device ifindex in the child's namespace to
 		 * parent.
 		 */
-		ret = send(data_sock, &netdev->ifindex, sizeof(netdev->ifindex), MSG_NOSIGNAL);
+		ret = lxc_send_nointr(data_sock, &netdev->ifindex, sizeof(netdev->ifindex), MSG_NOSIGNAL);
 		if (ret < 0)
 			return -1;
 	}
@@ -3136,14 +3137,14 @@ int lxc_network_recv_name_and_ifindex_from_child(struct lxc_handler *handler)
 		/* Receive network device name in the child's namespace to
 		 * parent.
 		 */
-		ret = recv(data_sock, netdev->name, IFNAMSIZ, 0);
+		ret = lxc_recv_nointr(data_sock, netdev->name, IFNAMSIZ, 0);
 		if (ret < 0)
 			return -1;
 
 		/* Receive network device ifindex in the child's namespace to
 		 * parent.
 		 */
-		ret = recv(data_sock, &netdev->ifindex, sizeof(netdev->ifindex), 0);
+		ret = lxc_recv_nointr(data_sock, &netdev->ifindex, sizeof(netdev->ifindex), 0);
 		if (ret < 0)
 			return -1;
 	}
@@ -3202,6 +3203,7 @@ int lxc_netns_set_nsid(int fd)
 	struct nl_handler nlh;
 	struct nlmsghdr *hdr;
 	struct rtgenmsg *msg;
+	int saved_errno;
 	__s32 ns_id = -1;
 	__u32 netns_fd = fd;
 
@@ -3224,7 +3226,9 @@ int lxc_netns_set_nsid(int fd)
 	addattr(hdr, 1024, __LXC_NETNSA_NSID, &ns_id, sizeof(ns_id));
 
 	ret = __netlink_transaction(&nlh, hdr, hdr);
+	saved_errno = errno;
 	netlink_close(&nlh);
+	errno = saved_errno;
 	if (ret < 0)
 		return -1;
 

src/lxc/nl.c

@@ -20,6 +20,9 @@
  * License along with this library; if not, write to the Free Software
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
  */
+
+#include "config.h"
+
 #include <sys/socket.h>
 #include <string.h>
 #include <stdio.h>
@@ -30,8 +33,11 @@
 #include <linux/netlink.h>
 #include <linux/rtnetlink.h>
 
+#include "log.h"
 #include "nl.h"
 
+lxc_log_define(nl, lxc);
+
 extern size_t nlmsg_len(const struct nlmsg *nlmsg)
 {
 	return nlmsg->nlmsghdr->nlmsg_len - NLMSG_HDRLEN;
@@ -201,8 +207,10 @@ again:
 	if (!ret)
 		return 0;
 
-	if (msg.msg_flags & MSG_TRUNC && (ret == nlmsghdr->nlmsg_len))
-		return -EMSGSIZE;
+	if (msg.msg_flags & MSG_TRUNC && (ret == nlmsghdr->nlmsg_len)) {
+		errno = EMSGSIZE;
+		ret = -1;
+	}
 
 	return ret;
 }
@@ -252,18 +260,21 @@ extern int __netlink_transaction(struct nl_handler *handler,
 
 	ret = __netlink_send(handler, request);
 	if (ret < 0)
-		return ret;
+		return -1;
 
 	ret = __netlink_recv(handler, answer);
 	if (ret < 0)
-		return ret;
+		return -1;
 
+	ret = 0;
 	if (answer->nlmsg_type == NLMSG_ERROR) {
 		struct nlmsgerr *err = (struct nlmsgerr *)NLMSG_DATA(answer);
-		return err->error;
+		errno = -err->error;
+		if (err->error < 0)
+			ret = -1;
 	}
 
-	return 0;
+	return ret;
 }
 
 extern int netlink_transaction(struct nl_handler *handler,

src/lxc/start.c

@@ -61,6 +61,7 @@
 #include "conf.h"
 #include "confile_utils.h"
 #include "error.h"
+#include "file_utils.h"
 #include "list.h"
 #include "lsm/lsm.h"
 #include "log.h"
@@ -440,16 +441,9 @@ int lxc_serve_state_clients(const char *name, struct lxc_handler *handler,
 		TRACE("Sending state %s to state client %d",
 		      lxc_state2str(state), client->clientfd);
 
-	again:
-		ret = send(client->clientfd, &msg, sizeof(msg), MSG_NOSIGNAL);
-		if (ret <= 0) {
-			if (errno == EINTR) {
-				TRACE("Caught EINTR; retrying");
-				goto again;
-			}
-
+		ret = lxc_send_nointr(client->clientfd, &msg, sizeof(msg), MSG_NOSIGNAL);
+		if (ret <= 0)
 			SYSERROR("Failed to send message to client");
-		}
 
 		/* kick client from list */
 		lxc_list_del(cur);
@@ -1795,12 +1789,10 @@ static int lxc_spawn(struct lxc_handler *handler)
 		DEBUG("Preserved net namespace via fd %d", ret);
 
 		ret = lxc_netns_set_nsid(handler->nsfd[LXC_NS_NET]);
-		if (ret < 0) {
-			errno = -ret;
+		if (ret < 0)
 			SYSERROR("Failed to allocate new network namespace id");
-		} else {
+		else
 			TRACE("Allocated new network namespace id");
-		}
 	}
 
 	/* Create the network configuration. */