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>

diff --git a/sys-utils/unshare.c b/sys-utils/unshare.c
index 7482a8bd38c71ddbf78ff317b0c760f033552e6b..988632062c00952f271bebd7a2760d7fb11ecd3e 100644 (file)
--- 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) {