mdadm: Unify forks behaviour

If mdadm is run by udev or systemd, it gets a pipe as each stream.
Forks in the background may run after an event or service has been
processed when udev is detached from pipe. As a result process
fails quietly if any message is written.
To prevent from it, each fork has to close all parent streams. Leave
stderr and stdout opened only for debug purposes.
Unify it across all forks. Introduce other descriptors detection by
scanning /proc/self/fd directory. Add generic method for
managing systemd services.

Signed-off-by: Mariusz Tkaczyk <mariusz.tkaczyk@intel.com>

diff --git a/Grow.c b/Grow.c
index 57db7d454800c6f850608694e27e353aff62017f..6b8321c5172f11e352013f55825f5de2de620dc7 100644 (file)
--- a/Grow.c
+++ b/Grow.c
@@ -2982,47 +2982,6 @@ static void catch_term(int sig)
        sigterm = 1;
 }
 
-static int continue_via_systemd(char *devnm)
-{
-       int skipped, i, pid, status;
-       char pathbuf[1024];
-       /* In a systemd/udev world, it is best to get systemd to
-        * run "mdadm --grow --continue" rather than running in the
-        * background.
-        */
-       switch(fork()) {
-       case  0:
-               /* FIXME yuk. CLOSE_EXEC?? */
-               skipped = 0;
-               for (i = 3; skipped < 20; i++)
-                       if (close(i) < 0)
-                               skipped++;
-                       else
-                               skipped = 0;
-
-               /* Don't want to see error messages from
-                * systemctl.  If the service doesn't exist,
-                * we fork ourselves.
-                */
-               close(2);
-               open("/dev/null", O_WRONLY);
-               snprintf(pathbuf, sizeof(pathbuf),
-                        "mdadm-grow-continue@%s.service", devnm);
-               status = execl("/usr/bin/systemctl", "systemctl", "restart",
-                              pathbuf, NULL);
-               status = execl("/bin/systemctl", "systemctl", "restart",
-                              pathbuf, NULL);
-               exit(1);
-       case -1: /* Just do it ourselves. */
-               break;
-       default: /* parent - good */
-               pid = wait(&status);
-               if (pid >= 0 && status == 0)
-                       return 1;
-       }
-       return 0;
-}
-
 static int reshape_array(char *container, int fd, char *devname,
                         struct supertype *st, struct mdinfo *info,
                         int force, struct mddev_dev *devlist,
@@ -3401,6 +3360,7 @@ static int reshape_array(char *container, int fd, char *devname,
                default: /* parent */
                        return 0;
                case 0:
+                       manage_fork_fds(0);
                        map_fork();
                        break;
                }
@@ -3509,8 +3469,9 @@ started:
                return 1;
        }
 
-       if (!forked && !check_env("MDADM_NO_SYSTEMCTL"))
-               if (continue_via_systemd(container ?: sra->sys_name)) {
+       if (!forked)
+               if (continue_via_systemd(container ?: sra->sys_name,
+                                        GROW_SERVICE)) {
                        free(fdlist);
                        free(offsets);
                        sysfs_free(sra);
@@ -3704,8 +3665,8 @@ int reshape_container(char *container, char *devname,
         */
        ping_monitor(container);
 
-       if (!forked && !freeze_reshape && !check_env("MDADM_NO_SYSTEMCTL"))
-               if (continue_via_systemd(container))
+       if (!forked && !freeze_reshape)
+               if (continue_via_systemd(container, GROW_SERVICE))
                        return 0;
 
        switch (forked ? 0 : fork()) {
@@ -3718,6 +3679,7 @@ int reshape_container(char *container, char *devname,
                        printf("%s: multi-array reshape continues in background\n", Name);
                return 0;
        case 0: /* child */
+               manage_fork_fds(0);
                map_fork();
                break;
        }

diff --git a/Incremental.c b/Incremental.c
index 98dbcd920f535a44f8764ee91f72fb4fcb6efea1..ad9ec1ccde8da6415a1b425a26306a89e45fbb86 100644 (file)
--- a/Incremental.c
+++ b/Incremental.c
@@ -1679,6 +1679,7 @@ static void run_udisks(char *arg1, char *arg2)
        int pid = fork();
        int status;
        if (pid == 0) {
+               manage_fork_fds(1);
                execl("/usr/bin/udisks", "udisks", arg1, arg2, NULL);
                execl("/bin/udisks", "udisks", arg1, arg2, NULL);
                exit(1);

diff --git a/Monitor.c b/Monitor.c
index a82e99d693abc287553fae52536571a2825cb6d2..3f3005b845c9fc42d353379384819e0abb82f393 100644 (file)
--- a/Monitor.c
+++ b/Monitor.c
@@ -323,10 +323,7 @@ static int make_daemon(char *pidfile)
                perror("daemonise");
                return 1;
        }
-       close(0);
-       open("/dev/null", O_RDWR);
-       dup2(0, 1);
-       dup2(0, 2);
+       manage_fork_fds(0);
        setsid();
        return -1;
 }

diff --git a/mdadm.h b/mdadm.h
index 4961c0f9d8691340129112e947153ae633a506a0..56b1b197ff81721c1f4fd0564e6681694047a002 100644 (file)
--- a/mdadm.h
+++ b/mdadm.h
@@ -129,6 +129,14 @@ struct dlm_lksb {
 #define FAILED_SLOTS_DIR "/run/mdadm/failed-slots"
 #endif /* FAILED_SLOTS */
 
+#ifndef MDMON_SERVICE
+#define MDMON_SERVICE "mdmon"
+#endif /* MDMON_SERVICE */
+
+#ifndef GROW_SERVICE
+#define GROW_SERVICE "mdadm-grow-continue"
+#endif /* GROW_SERVICE */
+
 #include       "md_u.h"
 #include       "md_p.h"
 #include       "bitmap.h"
@@ -1497,6 +1505,8 @@ extern int is_standard(char *dev, int *nump);
 extern int same_dev(char *one, char *two);
 extern int compare_paths (char* path1,char* path2);
 extern void enable_fds(int devices);
+extern void manage_fork_fds(int close_all);
+extern int continue_via_systemd(char *devnm, char *service_name);
 
 extern int parse_auto(char *str, char *msg, int config);
 extern struct mddev_ident *conf_get_ident(char *dev);

diff --git a/mdmon.c b/mdmon.c
index ff985d291ee9045ede3daf68c1184af3ee0384eb..c71e62c6b35a7e4d3ed7751794a2b0674b41113f 100644 (file)
--- a/mdmon.c
+++ b/mdmon.c
@@ -546,14 +546,7 @@ static int mdmon(char *devnm, int must_fork, int takeover)
        }
 
        setsid();
-       close(0);
-       open("/dev/null", O_RDWR);
-       close(1);
-       ignore = dup(0);
-#ifndef DEBUG
-       close(2);
-       ignore = dup(0);
-#endif
+       manage_fork_fds(0);
 
        /* This silliness is to stop the compiler complaining
         * that we ignore 'ignore'

diff --git a/util.c b/util.c
index 579dd4238a759a092fa58c2928e4bcaf25c6f115..5879694758ad836afd722f298ce1fc73076fa4aa 100644 (file)
--- a/util.c
+++ b/util.c
@@ -1915,7 +1915,7 @@ int mdmon_running(char *devnm)
 
 int start_mdmon(char *devnm)
 {
-       int i, skipped;
+       int i;
        int len;
        pid_t pid;
        int status;
@@ -1929,7 +1929,10 @@ int start_mdmon(char *devnm)
 
        if (check_env("MDADM_NO_MDMON"))
                return 0;
+       if (continue_via_systemd(devnm, MDMON_SERVICE))
+               return 0;
 
+       /* That failed, try running mdmon directly */
        len = readlink("/proc/self/exe", pathbuf, sizeof(pathbuf)-1);
        if (len > 0) {
                char *sl;
@@ -1943,51 +1946,9 @@ int start_mdmon(char *devnm)
        } else
                pathbuf[0] = '\0';
 
-       /* First try to run systemctl */
-       if (!check_env("MDADM_NO_SYSTEMCTL"))
-               switch(fork()) {
-               case 0:
-                       /* FIXME yuk. CLOSE_EXEC?? */
-                       skipped = 0;
-                       for (i = 3; skipped < 20; i++)
-                               if (close(i) < 0)
-                                       skipped++;
-                               else
-                                       skipped = 0;
-
-                       /* Don't want to see error messages from
-                        * systemctl.  If the service doesn't exist,
-                        * we start mdmon ourselves.
-                        */
-                       close(2);
-                       open("/dev/null", O_WRONLY);
-                       snprintf(pathbuf, sizeof(pathbuf), "mdmon@%s.service",
-                                devnm);
-                       status = execl("/usr/bin/systemctl", "systemctl",
-                                      "start",
-                                      pathbuf, NULL);
-                       status = execl("/bin/systemctl", "systemctl", "start",
-                                      pathbuf, NULL);
-                       exit(1);
-               case -1: pr_err("cannot run mdmon. Array remains readonly\n");
-                       return -1;
-               default: /* parent - good */
-                       pid = wait(&status);
-                       if (pid >= 0 && status == 0)
-                               return 0;
-               }
-
-       /* That failed, try running mdmon directly */
        switch(fork()) {
        case 0:
-               /* FIXME yuk. CLOSE_EXEC?? */
-               skipped = 0;
-               for (i = 3; skipped < 20; i++)
-                       if (close(i) < 0)
-                               skipped++;
-                       else
-                               skipped = 0;
-
+               manage_fork_fds(1);
                for (i = 0; paths[i]; i++)
                        if (paths[i][0]) {
                                execl(paths[i], paths[i],
@@ -2192,6 +2153,81 @@ void enable_fds(int devices)
        setrlimit(RLIMIT_NOFILE, &lim);
 }
 
+/* Close all opened descriptors if needed and redirect
+ * streams to /dev/null.
+ * For debug purposed, leave STDOUT and STDERR untouched
+ * Returns:
+ *     1- if any error occurred
+ *     0- otherwise
+ */
+void manage_fork_fds(int close_all)
+{
+       DIR *dir;
+       struct dirent *dirent;
+
+       close(0);
+       open("/dev/null", O_RDWR);
+
+#ifndef DEBUG
+       dup2(0, 1);
+       dup2(0, 2);
+#endif
+
+       if (close_all == 0)
+               return;
+
+       dir = opendir("/proc/self/fd");
+       if (!dir) {
+               pr_err("Cannot open /proc/self/fd directory.\n");
+               return;
+       }
+       for (dirent = readdir(dir); dirent; dirent = readdir(dir)) {
+               int fd = -1;
+
+               if ((strcmp(dirent->d_name, ".") == 0) ||
+                   (strcmp(dirent->d_name, "..")) == 0)
+                       continue;
+
+               fd = strtol(dirent->d_name, NULL, 10);
+               if (fd > 2)
+                       close(fd);
+       }
+}
+
+/* In a systemd/udev world, it is best to get systemd to
+ * run daemon rather than running in the background.
+ * Returns:
+ *     1- if systemd service has been started
+ *     0- otherwise
+ */
+int continue_via_systemd(char *devnm, char *service_name)
+{
+       int pid, status;
+       char pathbuf[1024];
+
+       /* Simply return that service cannot be started */
+       if (check_env("MDADM_NO_SYSTEMCTL"))
+               return 0;
+       switch (fork()) {
+       case  0:
+               manage_fork_fds(1);
+               snprintf(pathbuf, sizeof(pathbuf),
+                        "%s@%s.service", service_name, devnm);
+               status = execl("/usr/bin/systemctl", "systemctl", "restart",
+                              pathbuf, NULL);
+               status = execl("/bin/systemctl", "systemctl", "restart",
+                              pathbuf, NULL);
+               exit(1);
+       case -1: /* Just do it ourselves. */
+               break;
+       default: /* parent - good */
+               pid = wait(&status);
+               if (pid >= 0 && status == 0)
+                       return 1;
+       }
+       return 0;
+}
+
 int in_initrd(void)
 {
        /* This is based on similar function in systemd. */