Avoid double unmount on FUSE_DESTROY

This is a long standing issue, a system could have unmounted
/path/to/mnt and then fuse-client/kernel would send FUSE_DESTROY,
which would then again try a umount.

Given that FUSE_DESTROY is async, that umount might arrive
any time later and might possibly unmount a wrong mount point.
A warning as in issue #1286 is just minor to that.

Code wise this uses atomics to free the char *, as FUSE_DESTROY
might race with a signal and a double free might be possible
without proctection. A lock might run into the same issue,
if the signal would arrive at the wrong time a double lock
would be possible.

Additionally, fuse_session_mount() is updated, to first
duplicatate the pointer and to then do the kernel mount -
reverting the kernel mount in case of strdup() failure
is much harder.

Closes: https://github.com/libfuse/libfuse/issues/1286
Signed-off-by: Bernd Schubert <bernd@bsbernd.com>
(cherry picked from commit d8253770ac2cf4b8769e8cf41eb3c629f30ee80f)

2 files changed, 24 insertions, 12 deletions

diff --git a/lib/fuse_i.h b/lib/fuse_i.h
index 718fa14..acf9d5a 100644
--- a/lib/fuse_i.h
+++ b/lib/fuse_i.h
@@ -13,6 +13,7 @@
 #include <stdint.h>
 #include <stdbool.h>
 #include <errno.h>
+#include <stdatomic.h>
 
 #define MIN(a, b) \
 ({									\
@@ -54,7 +55,7 @@ struct fuse_notify_req {
 };
 
 struct fuse_session {
-	char *mountpoint;
+	_Atomic(char *)mountpoint;
 	volatile int exited;
 	int fd;
 	struct fuse_custom_io *io;
diff --git a/lib/fuse_lowlevel.c b/lib/fuse_lowlevel.c
index 1276a0f..3c9994f 100644
--- a/lib/fuse_lowlevel.c
+++ b/lib/fuse_lowlevel.c
@@ -2378,10 +2378,14 @@ void do_init(fuse_req_t req, fuse_ino_t nodeid, const void *inarg)
 static void do_destroy(fuse_req_t req, fuse_ino_t nodeid, const void *inarg)
 {
 	struct fuse_session *se = req->se;
+	char *mountpoint;
 
 	(void) nodeid;
 	(void) inarg;
 
+	mountpoint = atomic_exchange(&se->mountpoint, NULL);
+	free(mountpoint);
+
 	se->got_destroy = 1;
 	se->got_init = 0;
 	if (se->op.destroy)
@@ -3466,15 +3470,23 @@ int fuse_session_custom_io_30(struct fuse_session *se,
 			offsetof(struct fuse_custom_io, clone_fd), fd);
 }
 
-int fuse_session_mount(struct fuse_session *se, const char *mountpoint)
+int fuse_session_mount(struct fuse_session *se, const char *_mountpoint)
 {
 	int fd;
+	char *mountpoint;
 
-	if (mountpoint == NULL) {
+	if (_mountpoint == NULL) {
 		fuse_log(FUSE_LOG_ERR, "Invalid null-ptr mountpoint!\n");
 		return -1;
 	}
 
+	mountpoint = strdup(_mountpoint);
+	if (mountpoint == NULL) {
+		fuse_log(FUSE_LOG_ERR, "Failed to allocate memory for mountpoint. Error: %s\n",
+			strerror(errno));
+		return -1;
+	}
+
 	/*
 	 * Make sure file descriptors 0, 1 and 2 are open, otherwise chaos
 	 * would ensue.
@@ -3497,7 +3509,7 @@ int fuse_session_mount(struct fuse_session *se, const char *mountpoint)
 			fuse_log(FUSE_LOG_ERR,
 				"fuse: Invalid file descriptor /dev/fd/%u\n",
 				fd);
-			return -1;
+			goto error_out;
 		}
 		se->fd = fd;
 		return 0;
@@ -3506,18 +3518,16 @@ int fuse_session_mount(struct fuse_session *se, const char *mountpoint)
 	/* Open channel */
 	fd = fuse_kern_mount(mountpoint, se->mo);
 	if (fd == -1)
-		return -1;
+		goto error_out;
 	se->fd = fd;
 
 	/* Save mountpoint */
-	se->mountpoint = strdup(mountpoint);
-	if (se->mountpoint == NULL)
-		goto error_out;
+	se->mountpoint = mountpoint;
 
 	return 0;
 
 error_out:
-	fuse_kern_unmount(mountpoint, fd);
+	free(mountpoint);
 	return -1;
 }
 
@@ -3529,10 +3539,11 @@ int fuse_session_fd(struct fuse_session *se)
 void fuse_session_unmount(struct fuse_session *se)
 {
 	if (se->mountpoint != NULL) {
-		fuse_kern_unmount(se->mountpoint, se->fd);
+		char *mountpoint = atomic_exchange(&se->mountpoint, NULL);
+
+		fuse_kern_unmount(mountpoint, se->fd);
 		se->fd = -1;
-		free(se->mountpoint);
-		se->mountpoint = NULL;
+		free(mountpoint);
 	}
 }