unshare: fix busyloop and reduce racing probability

Replace busy-loop with waiting on pipe from parent.

Note: reduces racing probability, but still there are window where
it is possible (if parent unshare process will be [externally] killed
between successful read(fds[0]) and mount() calls).

[kzak@redhat.com: - use all-io.h to avoid loops around write() and read(),
                  - use less generic 0x06 byte to sync parent and child]

Signed-off-by: Yuriy M. Kaminskiy <yumkam@gmail.com>
Signed-off-by: Karel Zak <kzak@redhat.com>

1 files changed, 30 insertions, 10 deletions

diff --git a/sys-utils/unshare.c b/sys-utils/unshare.c
index 7482a8bd3..988632062 100644
--- a/sys-utils/unshare.c
+++ b/sys-utils/unshare.c
@@ -41,6 +41,9 @@
 #include "pathnames.h"
 #include "all-io.h"
 
+/* synchronize parent and child by pipe */
+#define PIPE_SYNC_BYTE	0x06
+
 /* 'private' is kernel default */
 #define UNSHARE_PROPAGATION_DEFAULT	(MS_REC | MS_PRIVATE)
 
@@ -199,27 +202,37 @@ static ino_t get_mnt_ino(pid_t pid)
 	return st.st_ino;
 }
 
-static void bind_ns_files_from_child(pid_t *child)
+static void bind_ns_files_from_child(pid_t *child, int fds[2])
 {
+	char ch;
 	pid_t ppid = getpid();
 	ino_t ino = get_mnt_ino(ppid);
 
+	if (pipe(fds) < 0)
+		err(EXIT_FAILURE, _("pipe failed"));
+
 	*child = fork();
 
-	switch(*child) {
+	switch (*child) {
 	case -1:
 		err(EXIT_FAILURE, _("fork failed"));
+
 	case 0:	/* child */
-		do {
-			/* wait until parent unshare() */
-			ino_t new_ino = get_mnt_ino(ppid);
-			if (ino != new_ino)
-				break;
-		} while (1);
+		close(fds[1]);
+		fds[1] = -1;
+
+		/* wait for parent */
+		if (read_all(fds[0], &ch, 1) != 1 && ch != PIPE_SYNC_BYTE)
+			err(EXIT_FAILURE, _("failed to read pipe"));
+		if (get_mnt_ino(ppid) == ino)
+			exit(EXIT_FAILURE);
 		bind_ns_files(ppid);
 		exit(EXIT_SUCCESS);
 		break;
+
 	default: /* parent */
+		close(fds[0]);
+		fds[0] = -1;
 		break;
 	}
 }
@@ -288,6 +301,7 @@ int main(int argc, char *argv[])
 	int c, forkit = 0, maproot = 0;
 	const char *procmnt = NULL;
 	pid_t pid = 0;
+	int fds[2];
 	int status;
 	unsigned long propagation = UNSHARE_PROPAGATION_DEFAULT;
 	uid_t real_euid = geteuid();
@@ -358,16 +372,22 @@ int main(int argc, char *argv[])
 	}
 
 	if (npersists && (unshare_flags & CLONE_NEWNS))
-		bind_ns_files_from_child(&pid);
+		bind_ns_files_from_child(&pid, fds);
 
 	if (-1 == unshare(unshare_flags))
 		err(EXIT_FAILURE, _("unshare failed"));
 
 	if (npersists) {
 		if (pid && (unshare_flags & CLONE_NEWNS)) {
-			/* wait for bind_ns_files_from_child() */
 			int rc;
+			char ch = PIPE_SYNC_BYTE;
+
+			/* signal child we are ready */
+			write_all(fds[1], &ch, 1);
+			close(fds[1]);
+			fds[1] = -1;
 
+			/* wait for bind_ns_files_from_child() */
 			do {
 				rc = waitpid(pid, &status, 0);
 				if (rc < 0) {