Merge pull request #901 from bsbernd/posix_spawn

Switch from fork to posix_spawn

3 files changed, 168 insertions, 127 deletions

diff --git a/lib/mount.c b/lib/mount.c
index f98a8bb..3fbbe72 100644
--- 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,21 +117,51 @@ static const struct fuse_opt fuse_mount_opts[] = {
 	FUSE_OPT_END
 };
 
-static void exec_fusermount(const char *argv[])
+/*
+ * Running fusermount by calling 'posix_spawn'
+ *
+ * @param out_pid might be NULL
+ */
+static int fusermount_posix_spawn(posix_spawn_file_actions_t *action,
+				  char const * const argv[], pid_t *out_pid)
 {
-	execv(FUSERMOUNT_DIR "/" FUSERMOUNT_PROG, (char **) argv);
-	execvp(FUSERMOUNT_PROG, (char **) argv);
+	const char *full_path = FUSERMOUNT_DIR "/" FUSERMOUNT_PROG;
+	pid_t pid;
+
+	/* See man 7 environ for the global environ pointer */
+
+	/* first try the install path */
+	int status = posix_spawn(&pid, full_path,  action, NULL,
+				 (char * const *) argv, environ);
+	if (status != 0) {
+		/* if that fails, try a system install */
+		status = posix_spawnp(&pid, FUSERMOUNT_PROG, action, NULL,
+				      (char * const *) argv, environ);
+	}
+
+	if (status != 0) {
+		fuse_log(FUSE_LOG_ERR,
+			 "On calling fusermount posix_spawn failed: %s\n",
+			 strerror(status));
+		return -status;
+	}
+
+	if (out_pid)
+		*out_pid = pid;
+	else
+		waitpid(pid, NULL, 0);
+
+	return 0;
 }
 
 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)
-		waitpid(pid, NULL, 0);
+	char const *const argv[] = {FUSERMOUNT_PROG, "--version", NULL};
+	int status = fusermount_posix_spawn(NULL, argv, NULL);
+
+	if(status != 0)
+		fuse_log(FUSE_LOG_ERR, "Running '%s --version' failed",
+			 FUSERMOUNT_PROG);
 }
 
 struct mount_flags {
@@ -249,7 +279,7 @@ static int receive_fd(int fd)
 
 	while(((rv = recvmsg(fd, &msg, 0)) == -1) && errno == EINTR);
 	if (rv == -1) {
-		perror("recvmsg");
+		fuse_log(FUSE_LOG_ERR, "recvmsg failed: %s", strerror(errno));
 		return -1;
 	}
 	if(!rv) {
@@ -269,7 +299,6 @@ static int receive_fd(int fd)
 void fuse_kern_unmount(const char *mountpoint, int fd)
 {
 	int res;
-	int pid;
 
 	if (fd != -1) {
 		struct pollfd pfd;
@@ -301,23 +330,21 @@ 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 = fusermount_posix_spawn(NULL, argv, NULL);
+	if(status != 0) {
+		fuse_log(FUSE_LOG_ERR, "Spawaning %s to unumount failed",
+			 FUSERMOUNT_PROG);
 		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) {
@@ -327,59 +354,62 @@ static int setup_auto_unmount(const char *mountpoint, int quiet)
 
 	res = socketpair(PF_UNIX, SOCK_STREAM, 0, fds);
 	if(res == -1) {
-		perror("fuse: socketpair() failed");
+		fuse_log(FUSE_LOG_ERR, "Setting up auto-unmountsocketpair() failed",
+			 strerror(errno));
 		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);
-			}
-		}
+	/*
+	 * auto-umount runs in the background - it is not waiting for the
+	 * process
+	 */
+	int status = fusermount_posix_spawn(&action, argv, &pid);
 
-		argv[a++] = FUSERMOUNT_PROG;
-		argv[a++] = "--auto-unmount";
-		argv[a++] = "--";
-		argv[a++] = mountpoint;
-		argv[a++] = NULL;
+	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);
+		fuse_log(FUSE_LOG_ERR, "fuse: Setting up auto-unmount 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");
@@ -388,51 +418,49 @@ static int fuse_mount_fusermount(const char *mountpoint, struct mount_opts *mo,
 
 	res = socketpair(PF_UNIX, SOCK_STREAM, 0, fds);
 	if(res == -1) {
-		perror("fuse: socketpair() failed");
+		fuse_log(FUSE_LOG_ERR, "Running %s: socketpair() failed: %s\n",
+			 FUSERMOUNT_PROG, strerror(errno));
 		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 = fusermount_posix_spawn(&action, argv, &pid);
+
+	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);
+		fuse_log(FUSE_LOG_ERR, "posix_spawnp() for %s failed",
+			 FUSERMOUNT_PROG, strerror(errno));
+		return -1;
 	}
 
+	// passed to child now, so can close here.
 	close(fds[0]);
-	rv = receive_fd(fds[1]);
+
+	int fd = receive_fd(fds[1]);
 
 	if (!mo->auto_unmount) {
 		/* with auto_unmount option fusermount3 will not exit until
@@ -441,10 +469,10 @@ static int fuse_mount_fusermount(const char *mountpoint, struct mount_opts *mo,
 		waitpid(pid, NULL, 0); /* bury zombie */
 	}
 
-	if (rv >= 0)
-		fcntl(rv, F_SETFD, FD_CLOEXEC);
+	if (fd >= 0)
+		fcntl(fd, F_SETFD, FD_CLOEXEC);
 
-	return rv;
+	return fd;
 }
 
 #ifndef O_CLOEXEC
diff --git a/test/ci-build.sh b/test/ci-build.sh
index 28ff0d8..a023c15 100755
--- a/test/ci-build.sh
+++ b/test/ci-build.sh
@@ -3,6 +3,10 @@
 set -e
 
 TEST_CMD="python3 -m pytest --maxfail=99 test/"
+SAN="-Db_sanitize=address,undefined"
+
+# not default
+export UBSAN_OPTIONS=halt_on_error=1
 
 # Make sure binaries can be accessed when invoked by root.
 umask 0022
@@ -14,6 +18,9 @@ umask 0022
 # readable/executable.
 SOURCE_DIR="$(readlink -f .)"
 TEST_DIR="$(mktemp -dt libfuse-build-XXXXXX)"
+
+PREFIX_DIR="$(mktemp -dt libfuse-install-XXXXXXX)"
+
 chmod 0755 "${TEST_DIR}"
 cd "${TEST_DIR}"
 echo "Running in ${TEST_DIR}"
@@ -26,7 +33,7 @@ export CC
 # Standard build
 for CC in gcc gcc-9 gcc-10 clang; do
     echo "=== Building with ${CC} ==="
-    mkdir build-${CC}; cd build-${CC}
+    mkdir build-${CC}; pushd build-${CC}
     if [ "${CC}" == "clang" ]; then
         export CXX="clang++"
         export TEST_WITH_VALGRIND=false
@@ -43,60 +50,61 @@ for CC in gcc gcc-9 gcc-10 clang; do
     else
         build_opts=''
     fi
-    meson setup -D werror=true ${build_opts} "${SOURCE_DIR}" || (cat meson-logs/meson-log.txt; false)
+    meson setup -Dprefix=${PREFIX_DIR} -D werror=true ${build_opts} "${SOURCE_DIR}" || (cat meson-logs/meson-log.txt; false)
     ninja
+    sudo ninja install
+
+    # libfuse will first try the install path and then system defaults
+    sudo chmod 4755 ${PREFIX_DIR}/bin/fusermount3
 
-    sudo chown root:root util/fusermount3
-    sudo chmod 4755 util/fusermount3
     ${TEST_CMD}
-    cd ..
+    popd
+    rm -fr build-${CC}
+    sudo rm -fr ${PREFIX_DIR}
 done
-(cd build-$CC; sudo ninja install)
 
 sanitized_build()
-{
-    san=$1
-    additonal_option=$2
+(
+    echo "=== Building with clang and sanitizers"
 
-    echo "=== Building with clang and ${san} sanitizer ==="
-    [ -n ${additonal_option} ] || echo "Additional option: ${additonal_option}"
+    mkdir build-san; pushd build-san
 
-    mkdir build-${san}; pushd build-${san}
+    meson setup -Dprefix=${PREFIX_DIR} -D werror=true\
+           "${SOURCE_DIR}" \
+           || (ct meson-logs/meson-log.txt; false)
+    meson configure $SAN
 
     # b_lundef=false is required to work around clang
     # bug, cf. https://groups.google.com/forum/#!topic/mesonbuild/tgEdAXIIdC4
-    meson setup -D b_sanitize=${san} -D b_lundef=false -D werror=true\
-           ${additonal_option} "${SOURCE_DIR}" \
-           || (cat meson-logs/meson-log.txt; false)
+    meson configure -D b_lundef=false
+
+    meson configure
     ninja
+    sudo ninja install
+    sudo chmod 4755 ${PREFIX_DIR}/bin/fusermount3
 
     # Test as root and regular user
     sudo ${TEST_CMD}
-    sudo chown root:root util/fusermount3
-    sudo chmod 4755 util/fusermount3
     # Cleanup temporary files (since they are now owned by root)
     sudo rm -rf test/.pytest_cache/ test/__pycache__
 
     ${TEST_CMD}
     
     popd
-    rm -fr build-${san}
-}
+    rm -fr build-san
+    sudo rm -fr ${PREFIX_DIR}
+)
 
 # Sanitized build
 CC=clang
 CXX=clang++
 TEST_WITH_VALGRIND=false
-for san in undefined address; do
-    sanitized_build ${san}
-done
+sanitized_build $SAN
 
 # Sanitized build without libc versioned symbols
 CC=clang
 CXX=clang++
-for san in undefined address; do
-    sanitized_build ${san} "-Ddisable-libc-symbol-version=true"
-done
+sanitized_build
 
 # Documentation.
 (cd "${SOURCE_DIR}"; doxygen doc/Doxyfile)
diff --git a/util/fusermount.c b/util/fusermount.c
index 06f5f56..5716c76 100644
--- 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);