Use posix_spawn instead of fork+exec

Client code might allocate a lot of memory before starting the mount.
Fork is slow for processes that are using a lot of memory.  But
posix_spawn fixes that.

Another issue with fork is if the process is also doing RDMA - this
might lead to data corruption, as least if memory used for RDMA
is not marked with MADV_DONTFORK.  At least with linux kernels
before 5.12.
Also see https://blog.nelhage.com/post/a-cursed-bug/ for more details

Change by Bernd:
This also prepares the new fusermount option "--comm-fd", but keeps
the previous way to pass the parameter as env variable. In a future
release (exact data to be determined) we are going to remove usage
of the env variable and will switch to the new parameter.

diff --git a/lib/mount.c b/lib/mount.c
index f98a8bb368c17d5d5b2a7403cbb14bd11cd458bf..2ffb05c8410af09332a46ce1cb28db0455d0c232 100644 (file)
--- a/lib/mount.c
+++ b/lib/mount.c
@@ -8,6 +8,9 @@
   See the file COPYING.LIB.
 */
 
+/* For environ */
+#define _GNU_SOURCE
+
 #include "fuse_config.h"
 #include "fuse_i.h"
 #include "fuse_misc.h"
@@ -22,6 +25,7 @@
 #include <fcntl.h>
 #include <errno.h>
 #include <poll.h>
+#include <spawn.h>
 #include <sys/socket.h>
 #include <sys/un.h>
 #include <sys/wait.h>
@@ -44,10 +48,6 @@
 #define FUSERMOUNT_PROG                "fusermount3"
 #define FUSE_COMMFD_ENV                "_FUSE_COMMFD"
 
-#ifndef HAVE_FORK
-#define fork() vfork()
-#endif
-
 #ifndef MS_DIRSYNC
 #define MS_DIRSYNC 128
 #endif
@@ -117,20 +117,17 @@ static const struct fuse_opt fuse_mount_opts[] = {
        FUSE_OPT_END
 };
 
-static void exec_fusermount(const char *argv[])
-{
-       execv(FUSERMOUNT_DIR "/" FUSERMOUNT_PROG, (char **) argv);
-       execvp(FUSERMOUNT_PROG, (char **) argv);
-}
-
 void fuse_mount_version(void)
 {
-       int pid = fork();
-       if (!pid) {
-               const char *argv[] = { FUSERMOUNT_PROG, "--version", NULL };
-               exec_fusermount(argv);
-               _exit(1);
-       } else if (pid != -1)
+       char const *const argv[] = {FUSERMOUNT_PROG, "--version", NULL};
+       pid_t pid;
+       int status = posix_spawn(&pid, FUSERMOUNT_DIR "/" FUSERMOUNT_PROG, NULL,
+                       NULL, (char * const *) argv, environ)
+               && posix_spawnp(&pid, FUSERMOUNT_PROG, NULL,
+                       NULL, (char * const *) argv, environ);
+       if(status != 0)
+               perror("posix_spawn");
+       else
                waitpid(pid, NULL, 0);
 }
 
@@ -269,7 +266,7 @@ static int receive_fd(int fd)
 void fuse_kern_unmount(const char *mountpoint, int fd)
 {
        int res;
-       int pid;
+       pid_t pid;
 
        if (fd != -1) {
                struct pollfd pfd;
@@ -301,23 +298,24 @@ void fuse_kern_unmount(const char *mountpoint, int fd)
        if (res == 0)
                return;
 
-       pid = fork();
-       if(pid == -1)
+       char const * const argv[] =
+               { FUSERMOUNT_PROG, "--unmount", "--quiet", "--lazy",
+                               "--", mountpoint, NULL };
+       int status = posix_spawn(
+               &pid, FUSERMOUNT_DIR "/" FUSERMOUNT_PROG, NULL, NULL, (char * const *) argv, environ)
+               && posix_spawnp(
+                       &pid, FUSERMOUNT_PROG, NULL, NULL, (char * const *) argv, environ);
+       if(status != 0) {
+               perror("posix_spawn");
                return;
-
-       if(pid == 0) {
-               const char *argv[] = { FUSERMOUNT_PROG, "-u", "-q", "-z",
-                                      "--", mountpoint, NULL };
-
-               exec_fusermount(argv);
-               _exit(1);
        }
        waitpid(pid, NULL, 0);
 }
 
 static int setup_auto_unmount(const char *mountpoint, int quiet)
 {
-       int fds[2], pid;
+       int fds[2];
+       pid_t pid;
        int res;
 
        if (!mountpoint) {
@@ -331,55 +329,55 @@ static int setup_auto_unmount(const char *mountpoint, int quiet)
                return -1;
        }
 
-       pid = fork();
-       if(pid == -1) {
-               perror("fuse: fork() failed");
-               close(fds[0]);
-               close(fds[1]);
-               return -1;
+       char arg_fd_entry[30];
+       snprintf(arg_fd_entry, sizeof(arg_fd_entry), "%i", fds[0]);
+       setenv(FUSE_COMMFD_ENV, arg_fd_entry, 1);
+
+       char const *const argv[] = {
+               FUSERMOUNT_PROG,
+               "--auto-unmount",
+               "--",
+               mountpoint,
+               NULL,
+       };
+
+       // TODO: add error handling for all manipulations of action.
+       posix_spawn_file_actions_t action;
+       posix_spawn_file_actions_init(&action);
+
+       if (quiet) {
+               posix_spawn_file_actions_addclose(&action, 1);
+               posix_spawn_file_actions_addclose(&action, 2);
        }
+       posix_spawn_file_actions_addclose(&action, fds[1]);
 
-       if(pid == 0) {
-               char env[10];
-               const char *argv[32];
-               int a = 0;
-
-               if (quiet) {
-                       int fd = open("/dev/null", O_RDONLY);
-                       if (fd != -1) {
-                               dup2(fd, 1);
-                               dup2(fd, 2);
-                       }
-               }
-
-               argv[a++] = FUSERMOUNT_PROG;
-               argv[a++] = "--auto-unmount";
-               argv[a++] = "--";
-               argv[a++] = mountpoint;
-               argv[a++] = NULL;
+       int status = posix_spawn(&pid, FUSERMOUNT_DIR "/" FUSERMOUNT_PROG, &action,
+                       NULL, (char *const *) argv, environ)
+               && posix_spawnp(&pid, FUSERMOUNT_PROG, &action,
+                       NULL, (char *const *) argv, environ);
+       posix_spawn_file_actions_destroy(&action);
 
+       if(status != 0) {
+               close(fds[0]);
                close(fds[1]);
-               fcntl(fds[0], F_SETFD, 0);
-               snprintf(env, sizeof(env), "%i", fds[0]);
-               setenv(FUSE_COMMFD_ENV, env, 1);
-               exec_fusermount(argv);
-               perror("fuse: failed to exec fusermount3");
-               _exit(1);
+               perror("fuse: posix_spawnp() of fusermount3 failed");
+               return -1;
        }
-
+       // passed to child now, so can close here.
        close(fds[0]);
 
        // Now fusermount3 will only exit when fds[1] closes automatically when our
        // process exits.
        return 0;
+       // Note: fds[1] is leakend and doesn't get FD_CLOEXEC
 }
 
 static int fuse_mount_fusermount(const char *mountpoint, struct mount_opts *mo,
                const char *opts, int quiet)
 {
-       int fds[2], pid;
+       int fds[2];
+       pid_t pid;
        int res;
-       int rv;
 
        if (!mountpoint) {
                fuse_log(FUSE_LOG_ERR, "fuse: missing mountpoint parameter\n");
@@ -392,47 +390,45 @@ static int fuse_mount_fusermount(const char *mountpoint, struct mount_opts *mo,
                return -1;
        }
 
-       pid = fork();
-       if(pid == -1) {
-               perror("fuse: fork() failed");
-               close(fds[0]);
-               close(fds[1]);
-               return -1;
-       }
+       char arg_fd_entry[30];
+       snprintf(arg_fd_entry, sizeof(arg_fd_entry), "%i", fds[0]);
+       setenv(FUSE_COMMFD_ENV, arg_fd_entry, 1);
 
-       if(pid == 0) {
-               char env[10];
-               const char *argv[32];
-               int a = 0;
+       char const *const argv[] = {
+               FUSERMOUNT_PROG,
+               "-o", opts ? opts : "",
+               "--",
+               mountpoint,
+               NULL,
+       };
 
-               if (quiet) {
-                       int fd = open("/dev/null", O_RDONLY);
-                       if (fd != -1) {
-                               dup2(fd, 1);
-                               dup2(fd, 2);
-                       }
-               }
 
-               argv[a++] = FUSERMOUNT_PROG;
-               if (opts) {
-                       argv[a++] = "-o";
-                       argv[a++] = opts;
-               }
-               argv[a++] = "--";
-               argv[a++] = mountpoint;
-               argv[a++] = NULL;
+       posix_spawn_file_actions_t action;
+       posix_spawn_file_actions_init(&action);
+
+       if (quiet) {
+               posix_spawn_file_actions_addclose(&action, 1);
+               posix_spawn_file_actions_addclose(&action, 2);
+       }
+       posix_spawn_file_actions_addclose(&action, fds[1]);
 
+       int status = posix_spawn(&pid, FUSERMOUNT_DIR "/" FUSERMOUNT_PROG, &action,
+                       NULL, (char *const *) argv, environ)
+               && posix_spawnp(&pid, FUSERMOUNT_PROG, &action,
+                       NULL, (char *const *) argv, environ);
+       posix_spawn_file_actions_destroy(&action);
+
+       if(status != 0) {
+               close(fds[0]);
                close(fds[1]);
-               fcntl(fds[0], F_SETFD, 0);
-               snprintf(env, sizeof(env), "%i", fds[0]);
-               setenv(FUSE_COMMFD_ENV, env, 1);
-               exec_fusermount(argv);
-               perror("fuse: failed to exec fusermount3");
-               _exit(1);
+               perror("fuse: posix_spawnp() of fusermount3 failed");
+               return -1;
        }
 
+       // passed to child now, so can close here.
        close(fds[0]);
-       rv = receive_fd(fds[1]);
+
+       int rv = receive_fd(fds[1]);
 
        if (!mo->auto_unmount) {
                /* with auto_unmount option fusermount3 will not exit until

diff --git a/util/fusermount.c b/util/fusermount.c
index 06f5f56fb3c43d23bcbecfb9a33553f329117a39..5716c76990a086c0d87a1f970b215f011774e404 100644 (file)
--- a/util/fusermount.c
+++ b/util/fusermount.c
@@ -1454,7 +1454,7 @@ int main(int argc, char *argv[])
        static int unmount = 0;
        static int lazy = 0;
        static int quiet = 0;
-       char *commfd;
+       char *commfd = NULL;
        int cfd;
        const char *opts = "";
        const char *type = NULL;
@@ -1462,14 +1462,15 @@ int main(int argc, char *argv[])
 
        static const struct option long_opts[] = {
                {"unmount", no_argument, NULL, 'u'},
-               // Note: auto-unmount deliberately does not have a short version.
-               // It's meant for internal use by mount.c's setup_auto_unmount.
-               {"auto-unmount", no_argument, NULL, 'U'},
                {"lazy",    no_argument, NULL, 'z'},
                {"quiet",   no_argument, NULL, 'q'},
                {"help",    no_argument, NULL, 'h'},
                {"version", no_argument, NULL, 'V'},
                {"options", required_argument, NULL, 'o'},
+               // Note: auto-unmount and comm-fd don't have short versions.
+               // They'ne meant for internal use by mount.c
+               {"auto-unmount", no_argument, NULL, 'U'},
+               {"comm-fd", required_argument, NULL, 'c'},
                {0, 0, 0, 0}};
 
        progname = strdup(argc > 0 ? argv[0] : "fusermount");
@@ -1501,6 +1502,9 @@ int main(int argc, char *argv[])
                        auto_unmount = 1;
                        setup_auto_unmount_only = 1;
                        break;
+               case 'c':
+                       commfd = optarg;
+                       break;
                case 'z':
                        lazy = 1;
                        break;
@@ -1547,7 +1551,8 @@ int main(int argc, char *argv[])
        if (!setup_auto_unmount_only && unmount)
                goto do_unmount;
 
-       commfd = getenv(FUSE_COMMFD_ENV);
+       if(commfd == NULL)
+               commfd = getenv(FUSE_COMMFD_ENV);
        if (commfd == NULL) {
                fprintf(stderr, "%s: old style mounting not supported\n",
                        progname);