libct/nsenter: better read/write errors

Introduce and use CHECK_IO and CHECK_IO_CLEANUP macros so that we can
append %m to the error message depending on read/write return value.

This prevents the "Success" prefix in errors like:

	failed to sync with stage-1: next state: Success

Signed-off-by: Kir Kolyshkin <[email protected]>

Author: kolyshkin

libcontainer/nsenter/nsexec.c

@@ -132,6 +132,42 @@ int setns(int fd, int nstype)
 }
 #endif
 
+/*
+ * Check the result of read(2) or write(2) system calls.
+ * If the syscall returned -1 (error), call bail() to report errno.
+ * Otherwise, if the actual bytes transferred differs from expected,
+ * call bailx() since errno is not meaningful for short reads/writes.
+ *
+ * Usage: CHECK_IO(read, syncfd, &s, sizeof(s), "failed to sync with parent");
+ */
+#define CHECK_IO(op, fd, buf, count, ...) \
+	do { \
+		ssize_t __ret = op(fd, buf, count); \
+		if (__ret < 0) \
+			bail(__VA_ARGS__); \
+		if (__ret != (ssize_t)(count)) \
+			bailx(__VA_ARGS__ ": short " #op); \
+	} while (0)
+
+/*
+ * CHECK_IO variant that kills PIDs before bailing.
+ * Use this when you need to clean up child processes on I/O failure.
+ *
+ * Usage: CHECK_IO_CLEANUP(write, syncfd, &s, sizeof(s), stage1_pid, stage2_pid,
+ *                         "failed to sync");
+ */
+#define CHECK_IO_CLEANUP(op, fd, buf, count, pid1, pid2, ...) \
+	do { \
+		ssize_t __ret = op(fd, buf, count); \
+		if (__ret != (ssize_t)(count)) { \
+			sane_kill(pid1, SIGKILL); \
+			sane_kill(pid2, SIGKILL); \
+			if (__ret < 0) \
+				bail(__VA_ARGS__); \
+			bailx(__VA_ARGS__ ": short " #op); \
+		} \
+	} while (0)
+
 /* XXX: This is ugly. */
 static int syncfd = -1;
 
@@ -334,16 +370,12 @@ static uint8_t readint8(char *buf)
 
 static void nl_parse(int fd, struct nlconfig_t *config)
 {
-	size_t len, size;
+	size_t size;
 	struct nlmsghdr hdr;
 	char *data, *current;
 
 	/* Retrieve the netlink header. */
-	len = read(fd, &hdr, NLMSG_HDRLEN);
-	if (len < 0)
-		bail("failed to read netlink header");
-	if (len != NLMSG_HDRLEN)
-		bailx("invalid netlink header length %zu", len);
+	CHECK_IO(read, fd, &hdr, NLMSG_HDRLEN, "failed to read netlink header");
 
 	if (hdr.nlmsg_type == NLMSG_ERROR)
 		bailx("failed to read netlink message");
@@ -357,11 +389,7 @@ static void nl_parse(int fd, struct nlconfig_t *config)
 	if (!data)
 		bail("failed to allocate %zu bytes of memory for nl_payload", size);
 
-	len = read(fd, data, size);
-	if (len < 0)
-		bail("failed to read netlink payload");
-	if (len != size)
-		bailx("failed to read netlink payload, %zu != %zu", len, size);
+	CHECK_IO(read, fd, data, size, "failed to read netlink payload");
 
 	/* Parse the netlink payload. */
 	config->data = data;
@@ -863,8 +891,7 @@ void nsexec(void)
 			while (!stage1_complete) {
 				enum sync_t s;
 
-				if (read(syncfd, &s, sizeof(s)) != sizeof(s))
-					bail("failed to sync with stage-1: next state");
+				CHECK_IO(read, syncfd, &s, sizeof(s), "failed to sync with stage-1: next state");
 
 				switch (s) {
 				case SYNC_USERMAP_PLS:
@@ -888,28 +915,20 @@ void nsexec(void)
 					update_gidmap(config.gidmappath, stage1_pid, config.gidmap, config.gidmap_len);
 
 					s = SYNC_USERMAP_ACK;
-					if (write(syncfd, &s, sizeof(s)) != sizeof(s)) {
-						sane_kill(stage1_pid, SIGKILL);
-						sane_kill(stage2_pid, SIGKILL);
-						bail("failed to sync with stage-1: write(SYNC_USERMAP_ACK)");
-					}
+					CHECK_IO_CLEANUP(write, syncfd, &s, sizeof(s), stage1_pid, stage2_pid,
+							 "failed to sync with stage-1: write(SYNC_USERMAP_ACK)");
 					break;
 				case SYNC_RECVPID_PLS:
 					write_log(DEBUG, "stage-1 requested pid to be forwarded");
 
 					/* Get the stage-2 pid. */
-					if (read(syncfd, &stage2_pid, sizeof(stage2_pid)) != sizeof(stage2_pid)) {
-						sane_kill(stage1_pid, SIGKILL);
-						bail("failed to sync with stage-1: read(stage2_pid)");
-					}
+					CHECK_IO_CLEANUP(read, syncfd, &stage2_pid, sizeof(stage2_pid), stage1_pid, -1,
+							 "failed to sync with stage-1: read(stage2_pid)");
 
 					/* Send ACK. */
 					s = SYNC_RECVPID_ACK;
-					if (write(syncfd, &s, sizeof(s)) != sizeof(s)) {
-						sane_kill(stage1_pid, SIGKILL);
-						sane_kill(stage2_pid, SIGKILL);
-						bail("failed to sync with stage-1: write(SYNC_RECVPID_ACK)");
-					}
+					CHECK_IO_CLEANUP(write, syncfd, &s, sizeof(s), stage1_pid, stage2_pid,
+							 "failed to sync with stage-1: write(SYNC_RECVPID_ACK)");
 
 					/*
 					 * Send both the stage-1 and stage-2 pids back to runc.
@@ -933,10 +952,8 @@ void nsexec(void)
 					write_log(DEBUG, "stage-1 requested timens offsets to be configured");
 					update_timens_offsets(stage1_pid, config.timensoffset, config.timensoffset_len);
 					s = SYNC_TIMEOFFSETS_ACK;
-					if (write(syncfd, &s, sizeof(s)) != sizeof(s)) {
-						sane_kill(stage1_pid, SIGKILL);
-						bail("failed to sync with child: write(SYNC_TIMEOFFSETS_ACK)");
-					}
+					CHECK_IO_CLEANUP(write, syncfd, &s, sizeof(s), stage1_pid, -1,
+							 "failed to sync with child: write(SYNC_TIMEOFFSETS_ACK)");
 					break;
 				case SYNC_CHILD_FINISH:
 					write_log(DEBUG, "stage-1 complete");
@@ -960,13 +977,10 @@ void nsexec(void)
 
 				write_log(DEBUG, "signalling stage-2 to run");
 				s = SYNC_GRANDCHILD;
-				if (write(syncfd, &s, sizeof(s)) != sizeof(s)) {
-					sane_kill(stage2_pid, SIGKILL);
-					bail("failed to sync with child: write(SYNC_GRANDCHILD)");
-				}
+				CHECK_IO_CLEANUP(write, syncfd, &s, sizeof(s), stage2_pid, -1,
+						 "failed to sync with child: write(SYNC_GRANDCHILD)");
 
-				if (read(syncfd, &s, sizeof(s)) != sizeof(s))
-					bail("failed to sync with child: next state");
+				CHECK_IO(read, syncfd, &s, sizeof(s), "failed to sync with child: next state");
 
 				switch (s) {
 				case SYNC_CHILD_FINISH:
@@ -1057,13 +1071,13 @@ void nsexec(void)
 				 */
 				write_log(DEBUG, "request stage-0 to map user namespace");
 				s = SYNC_USERMAP_PLS;
-				if (write(syncfd, &s, sizeof(s)) != sizeof(s))
-					bail("failed to sync with parent: write(SYNC_USERMAP_PLS)");
+				CHECK_IO(write, syncfd, &s, sizeof(s),
+					 "failed to sync with parent: write(SYNC_USERMAP_PLS)");
 
 				/* ... wait for mapping ... */
 				write_log(DEBUG, "waiting stage-0 to complete the mapping of user namespace");
-				if (read(syncfd, &s, sizeof(s)) != sizeof(s))
-					bail("failed to sync with parent: read(SYNC_USERMAP_ACK)");
+				CHECK_IO(read, syncfd, &s, sizeof(s),
+					 "failed to sync with parent: read(SYNC_USERMAP_ACK)");
 				if (s != SYNC_USERMAP_ACK)
 					bailx("failed to sync with parent: SYNC_USERMAP_ACK: got %u", s);
 
@@ -1095,11 +1109,11 @@ void nsexec(void)
 				write_log(DEBUG, "request stage-0 to write timens offsets");
 
 				s = SYNC_TIMEOFFSETS_PLS;
-				if (write(syncfd, &s, sizeof(s)) != sizeof(s))
-					bail("failed to sync with parent: write(SYNC_TIMEOFFSETS_PLS)");
+				CHECK_IO(write, syncfd, &s, sizeof(s),
+					 "failed to sync with parent: write(SYNC_TIMEOFFSETS_PLS)");
 
-				if (read(syncfd, &s, sizeof(s)) != sizeof(s))
-					bail("failed to sync with parent: read(SYNC_TIMEOFFSETS_ACK)");
+				CHECK_IO(read, syncfd, &s, sizeof(s),
+					 "failed to sync with parent: read(SYNC_TIMEOFFSETS_ACK)");
 				if (s != SYNC_TIMEOFFSETS_ACK)
 					bailx("failed to sync with parent: SYNC_TIMEOFFSETS_ACK: got %u", s);
 			}
@@ -1121,31 +1135,23 @@ void nsexec(void)
 			/* Send the child to our parent, which knows what it's doing. */
 			write_log(DEBUG, "request stage-0 to forward stage-2 pid (%d)", stage2_pid);
 			s = SYNC_RECVPID_PLS;
-			if (write(syncfd, &s, sizeof(s)) != sizeof(s)) {
-				sane_kill(stage2_pid, SIGKILL);
-				bail("failed to sync with parent: write(SYNC_RECVPID_PLS)");
-			}
-			if (write(syncfd, &stage2_pid, sizeof(stage2_pid)) != sizeof(stage2_pid)) {
-				sane_kill(stage2_pid, SIGKILL);
-				bail("failed to sync with parent: write(stage2_pid)");
-			}
+			CHECK_IO_CLEANUP(write, syncfd, &s, sizeof(s), stage2_pid, -1,
+					 "failed to sync with parent: write(SYNC_RECVPID_PLS)");
+			CHECK_IO_CLEANUP(write, syncfd, &stage2_pid, sizeof(stage2_pid), stage2_pid, -1,
+					 "failed to sync with parent: write(stage2_pid)");
 
 			/* ... wait for parent to get the pid ... */
-			if (read(syncfd, &s, sizeof(s)) != sizeof(s)) {
-				sane_kill(stage2_pid, SIGKILL);
-				bail("failed to sync with parent: read(SYNC_RECVPID_ACK)");
-			}
+			CHECK_IO_CLEANUP(read, syncfd, &s, sizeof(s), stage2_pid, -1,
+					 "failed to sync with parent: read(SYNC_RECVPID_ACK)");
 			if (s != SYNC_RECVPID_ACK) {
 				sane_kill(stage2_pid, SIGKILL);
 				bailx("failed to sync with parent: SYNC_RECVPID_ACK: got %u", s);
 			}
 
 			write_log(DEBUG, "signal completion to stage-0");
 			s = SYNC_CHILD_FINISH;
-			if (write(syncfd, &s, sizeof(s)) != sizeof(s)) {
-				sane_kill(stage2_pid, SIGKILL);
-				bail("failed to sync with parent: write(SYNC_CHILD_FINISH)");
-			}
+			CHECK_IO_CLEANUP(write, syncfd, &s, sizeof(s), stage2_pid, -1,
+					 "failed to sync with parent: write(SYNC_CHILD_FINISH)");
 
 			/* Our work is done. [Stage 2: STAGE_INIT] is doing the rest of the work. */
 			write_log(DEBUG, "<~ nsexec stage-1");
@@ -1181,8 +1187,7 @@ void nsexec(void)
 			prctl(PR_SET_NAME, (unsigned long)"runc:[2:INIT]", 0, 0, 0);
 			write_log(DEBUG, "~> nsexec stage-2");
 
-			if (read(syncfd, &s, sizeof(s)) != sizeof(s))
-				bail("failed to sync with parent: read(SYNC_GRANDCHILD)");
+			CHECK_IO(read, syncfd, &s, sizeof(s), "failed to sync with parent: read(SYNC_GRANDCHILD)");
 			if (s != SYNC_GRANDCHILD)
 				bailx("failed to sync with parent: SYNC_GRANDCHILD: got %u", s);
 
@@ -1202,8 +1207,7 @@ void nsexec(void)
 
 			write_log(DEBUG, "signal completion to stage-0");
 			s = SYNC_CHILD_FINISH;
-			if (write(syncfd, &s, sizeof(s)) != sizeof(s))
-				bail("failed to sync with parent: write(SYNC_CHILD_FINISH)");
+			CHECK_IO(write, syncfd, &s, sizeof(s), "failed to sync with parent: write(SYNC_CHILD_FINISH)");
 
 			/* Close sync pipes. */
 			if (close(sync_grandchild_pipe[0]) < 0)