Merge pull request #3556 from brauner/2020-10-19/fixes

src/lxc/conf.c

@@ -1531,13 +1531,15 @@ static int lxc_setup_devpts(struct lxc_handler *handler)
 
 	devpts_fd = openat(-EBADF, "/dev/pts", O_CLOEXEC | O_DIRECTORY | O_PATH | O_NOFOLLOW);
 	if (devpts_fd < 0) {
+		devpts_fd = -EBADF;
 		TRACE("Failed to create detached devpts mount");
-		ret = lxc_abstract_unix_send_fds(sock, NULL, 0, NULL, 0);
+		ret = lxc_abstract_unix_send_fds(sock, NULL, 0, &devpts_fd, sizeof(int));
 	} else {
 		ret = lxc_abstract_unix_send_fds(sock, &devpts_fd, 1, NULL, 0);
 	}
 	if (ret < 0)
 		return log_error_errno(-1, errno, "Failed to send devpts fd to parent");
+	TRACE("Sent devpts file descriptor %d to parent", devpts_fd);
 
 	/* Remove any pre-existing /dev/ptmx file. */
 	ret = remove("/dev/ptmx");

src/lxc/start.c

@@ -1953,13 +1953,14 @@ static int lxc_spawn(struct lxc_handler *handler)
 		}
 	}
 
-	ret = lxc_abstract_unix_recv_fds(data_sock1, &handler->conf->devpts_fd, 1, NULL, 0);
+	ret = lxc_abstract_unix_recv_fds(data_sock1, &handler->conf->devpts_fd, 1,
+					 &handler->conf->devpts_fd,
+					 sizeof(handler->conf->devpts_fd));
 	if (ret < 0) {
 		SYSERROR("Failed to receive devpts fd from child");
 		goto out_delete_net;
 	}
-	if (ret == 0)
-		handler->conf->devpts_fd = -EBADF;
+	TRACE("Received devpts file descriptor %d from child", handler->conf->devpts_fd);
 
 	/* Now all networks are created, network devices are moved into place,
 	 * and the correct names and ifindices in the respective namespaces have

src/lxc/sync.c

@@ -23,30 +23,21 @@ static int __sync_wait(int fd, int sequence)
 	ssize_t ret;
 
 	ret = lxc_read_nointr(fd, &sync, sizeof(sync));
-	if (ret < 0) {
-		SYSERROR("Sync wait failure");
-		return -1;
-	}
+	if (ret < 0)
+		return log_error_errno(-1, errno, "Sync wait failure");
 
 	if (!ret)
 		return 0;
 
-	if ((size_t)ret != sizeof(sync)) {
-		ERROR("Unexpected sync size: %zu expected %zu", (size_t)ret, sizeof(sync));
-		return -1;
-	}
+	if ((size_t)ret != sizeof(sync))
+		return log_error(-1, "Unexpected sync size: %zu expected %zu", (size_t)ret, sizeof(sync));
 
-	if (sync == LXC_SYNC_ERROR) {
-		ERROR("An error occurred in another process "
-		      "(expected sequence number %d)", sequence);
-		return -1;
-	}
+	if (sync == LXC_SYNC_ERROR)
+		return log_error(-1, "An error occurred in another process (expected sequence number %d)", sequence);
+
+	if (sync != sequence)
+		return log_error(-1, "Invalid sequence number %d. Expected sequence number %d", sync, sequence);
 
-	if (sync != sequence) {
-		ERROR("Invalid sequence number %d. Expected sequence number %d",
-		      sync, sequence);
-		return -1;
-	}
 	return 0;
 }
 
@@ -54,10 +45,9 @@ static int __sync_wake(int fd, int sequence)
 {
 	int sync = sequence;
 
-	if (lxc_write_nointr(fd, &sync, sizeof(sync)) < 0) {
-		SYSERROR("Sync wake failure");
-		return -1;
-	}
+	if (lxc_write_nointr(fd, &sync, sizeof(sync)) < 0)
+		return log_error_errno(-1, errno, "Sync wake failure");
+
 	return 0;
 }
 
@@ -65,36 +55,45 @@ static int __sync_barrier(int fd, int sequence)
 {
 	if (__sync_wake(fd, sequence))
 		return -1;
-	return __sync_wait(fd, sequence+1);
+
+	return __sync_wait(fd, sequence + 1);
 }
 
 int lxc_sync_barrier_parent(struct lxc_handler *handler, int sequence)
 {
+	TRACE("Child waking parent with sequence %s and waiting for sequence %s",
+	      sync_to_string(sequence), sync_to_string(sequence + 1));
 	return __sync_barrier(handler->sync_sock[0], sequence);
 }
 
 int lxc_sync_barrier_child(struct lxc_handler *handler, int sequence)
 {
+	TRACE("Parent waking child with sequence %s and waiting with sequence %s",
+	      sync_to_string(sequence), sync_to_string(sequence + 1));
 	return __sync_barrier(handler->sync_sock[1], sequence);
 }
 
 int lxc_sync_wake_parent(struct lxc_handler *handler, int sequence)
 {
+	TRACE("Child waking parent with sequence %s", sync_to_string(sequence));
 	return __sync_wake(handler->sync_sock[0], sequence);
 }
 
 int lxc_sync_wait_parent(struct lxc_handler *handler, int sequence)
 {
+	TRACE("Parent waiting for child with sequence %s", sync_to_string(sequence));
 	return __sync_wait(handler->sync_sock[0], sequence);
 }
 
 int lxc_sync_wait_child(struct lxc_handler *handler, int sequence)
 {
+	TRACE("Child waiting for parent with sequence %s", sync_to_string(sequence));
 	return __sync_wait(handler->sync_sock[1], sequence);
 }
 
 int lxc_sync_wake_child(struct lxc_handler *handler, int sequence)
 {
+	TRACE("Child waking parent with sequence %s", sync_to_string(sequence));
 	return __sync_wake(handler->sync_sock[1], sequence);
 }
 
@@ -103,31 +102,26 @@ int lxc_sync_init(struct lxc_handler *handler)
 	int ret;
 
 	ret = socketpair(AF_LOCAL, SOCK_STREAM, 0, handler->sync_sock);
-	if (ret) {
-		SYSERROR("failed to create synchronization socketpair");
-		return -1;
-	}
+	if (ret)
+		return log_error_errno(-1, errno, "failed to create synchronization socketpair");
 
 	/* Be sure we don't inherit this after the exec */
-	fcntl(handler->sync_sock[0], F_SETFD, FD_CLOEXEC);
+	ret = fcntl(handler->sync_sock[0], F_SETFD, FD_CLOEXEC);
+	if (ret < 0)
+		return log_error_errno(-1, errno, "Failed to make socket close-on-exec");
 
+	TRACE("Initialized synchronization infrastructure");
 	return 0;
 }
 
 void lxc_sync_fini_child(struct lxc_handler *handler)
 {
-	if (handler->sync_sock[0] != -1) {
-		close(handler->sync_sock[0]);
-		handler->sync_sock[0] = -1;
-	}
+	close_prot_errno_disarm(handler->sync_sock[0]);
 }
 
 void lxc_sync_fini_parent(struct lxc_handler *handler)
 {
-	if (handler->sync_sock[1] != -1) {
-		close(handler->sync_sock[1]);
-		handler->sync_sock[1] = -1;
-	}
+	close_prot_errno_disarm(handler->sync_sock[1]);
 }
 
 void lxc_sync_fini(struct lxc_handler *handler)

src/lxc/sync.h

@@ -20,6 +20,34 @@ enum {
 	LXC_SYNC_ERROR		= -1 /* Used to report errors from another process */
 };
 
+static inline const char *sync_to_string(int state)
+{
+	switch (state) {
+	case LXC_SYNC_STARTUP:
+		return "startup";
+	case LXC_SYNC_CONFIGURE:
+		return "configure";
+	case LXC_SYNC_POST_CONFIGURE:
+		return "post-configure";
+	case LXC_SYNC_CGROUP:
+		return "cgroup";
+	case LXC_SYNC_CGROUP_UNSHARE:
+		return "cgroup-unshare";
+	case LXC_SYNC_CGROUP_LIMITS:
+		return "cgroup-limits";
+	case LXC_SYNC_READY_START:
+		return "ready-start";
+	case LXC_SYNC_RESTART:
+		return "restart";
+	case LXC_SYNC_POST_RESTART:
+		return "post-restart";
+	case LXC_SYNC_ERROR:
+		return "error";
+	default:
+		return "invalid sync state";
+	}
+}
+
 __hidden extern int lxc_sync_init(struct lxc_handler *handler);
 __hidden extern void lxc_sync_fini(struct lxc_handler *);
 __hidden extern void lxc_sync_fini_parent(struct lxc_handler *);