fusefs: fix a deadlock caused by daemons that never initialize

If a daemon never responds to FUSE_INIT, but some process attempts to
access the mountpoint, and a different process (possibly the daemon
itself) attempts to unmount, a deadlock would result.

Fix this bug by blocking any thread that enters fuse_vfsop_root until
the daemon responds to FUSE_INIT or it times out.  That will block any
thread attempting to lookup a path in the fuse mountpoint, before it
even gets to fuse VOPs.  Remove less thorough initialization checks in
fuse_ticket_fetch and fuse_vnop_access that are no longer necessary.
And add a test case.

PR:		287431
MFC after:	2 weeks
Sponsored by:	ConnectWise
Reviewed by:	js
Differential Revision: https://reviews.freebsd.org/D59737

sys/fs/fuse/fuse_ipc.c

@@ -590,28 +590,6 @@ fdata_set_dead(struct fuse_data *data)
 	FUSE_UNLOCK();
 }
 
-struct fuse_ticket *
-fuse_ticket_fetch(struct fuse_data *data)
-{
-	int err = 0;
-	struct fuse_ticket *ftick;
-
-	ftick = fticket_alloc(data);
-
-	if (!(data->dataflags & FSESS_INITED)) {
-		/* Sleep until get answer for INIT message */
-		FUSE_LOCK();
-		if (!(data->dataflags & FSESS_INITED) && data->ticketer > 2) {
-			err = msleep(&data->ticketer, &fuse_mtx, PCATCH | PDROP,
-			    "fu_ini", 0);
-			if (err)
-				fdata_set_dead(data);
-		} else
-			FUSE_UNLOCK();
-	}
-	return ftick;
-}
-
 int
 fuse_ticket_drop(struct fuse_ticket *ftick)
 {
@@ -924,7 +902,7 @@ fdisp_make_pid(struct fuse_dispatcher *fdip, enum fuse_opcode op,
 	if (fdip->tick) {
 		fticket_refresh(fdip->tick);
 	} else {
-		fdip->tick = fuse_ticket_fetch(data);
+		fdip->tick = fticket_alloc(data);
 	}
 
 	/* FUSE_DIMALLOC will bzero the fiovs when it enlarges them */

sys/fs/fuse/fuse_ipc.h

@@ -389,7 +389,6 @@ fuse_aw_pop(struct fuse_data *data)
 	return (ftick);
 }
 
-struct fuse_ticket *fuse_ticket_fetch(struct fuse_data *data);
 int fuse_ticket_drop(struct fuse_ticket *ftick);
 void fuse_insert_callback(struct fuse_ticket *ftick, fuse_handler_t *handler);
 void fuse_insert_message(struct fuse_ticket *ftick, bool irq);

sys/fs/fuse/fuse_vfsops.c

@@ -627,10 +627,37 @@ fuse_vfsop_root(struct mount *mp, int lkflags, struct vnode **vpp)
 	int err = 0;
 
 	if (data->vroot != NULL) {
+		if (!(data->dataflags & FSESS_INITED)) {
+			/* Block all operations until init completes */
+			FUSE_LOCK();
+			if (data->dataflags & FSESS_INITED) {
+				/* We must've just gotten initialized */
+				FUSE_UNLOCK();
+			} else {
+				err = msleep(&data->ticketer, &fuse_mtx,
+				    PCATCH | PDROP, "fu_ini",
+				    data->daemon_timeout * hz);
+				if (err) {
+					/*
+					 * The daemon didn't initialize on
+					 * time.  Mark it as dead.  Most vnops
+					 * will fail as a result, but VFS_ROOT
+					 * must still succeed for the sake of
+					 * unmount.
+					 */
+					fdata_set_dead(data);
+				}
+			}
+		}
 		err = vget(data->vroot, lkflags);
 		if (err == 0)
 			*vpp = data->vroot;
 	} else {
+		/*
+		 * Get a vnode for the root.  Note that this step can happen
+		 * straight from sys_nmount, before INIT is complete.
+		 */
+
 		err = fuse_vnode_get(mp, NULL, FUSE_ROOT_ID, NULL, vpp, NULL,
 		    VDIR);
 		if (err == 0) {
@@ -667,6 +694,10 @@ fuse_vfsop_statfs(struct mount *mp, struct statfs *sbp)
 
 	data = fuse_get_mpdata(mp);
 
+	/*
+	 * We must fake the statfs data before initialization completes,
+	 * because nmount itself calls VFS_STATFS.
+	 */
 	if (!(data->dataflags & FSESS_INITED))
 		goto fake;
 

sys/fs/fuse/fuse_vnops.c

@@ -507,10 +507,6 @@ fuse_vnop_access(struct vop_access_args *ap)
 {
 	struct vnode *vp = ap->a_vp;
 	int accmode = ap->a_accmode;
-	struct ucred *cred = ap->a_cred;
-
-	struct fuse_data *data = fuse_get_mpdata(vnode_mount(vp));
-
 	int err;
 
 	if (fuse_isdeadfs(vp)) {
@@ -520,16 +516,6 @@ fuse_vnop_access(struct vop_access_args *ap)
 		return (EXTERROR(ENXIO, "This FUSE session is about "
 		    "to be closed"));
 	}
-	if (!(data->dataflags & FSESS_INITED)) {
-		if (vnode_isvroot(vp)) {
-			if (priv_check_cred(cred, PRIV_VFS_ADMIN) ||
-			    (fuse_match_cred(data->daemoncred, cred) == 0)) {
-				return 0;
-			}
-		}
-		return (EXTERROR(EBADF, "Access denied until FUSE session "
-		    "is initialized"));
-	}
 	if (vnode_islnk(vp)) {
 		return 0;
 	}

tests/sys/fs/fusefs/mockfs.cc

@@ -427,7 +427,7 @@ MockFS::MockFS(int max_read, int max_readahead, bool allow_other,
 	uint32_t kernel_minor_version, uint32_t max_write, bool async,
 	bool noclusterr, unsigned time_gran, bool nointr, bool noatime,
 	const char *fsname, const char *subtype, bool no_auto_init,
-	bool auto_unmount)
+	bool auto_unmount, unsigned daemon_timeout)
 	: m_daemon_id(NULL),
 	  m_kernel_minor_version(kernel_minor_version),
 	  m_kq(pm == KQ ? kqueue() : -1),
@@ -524,6 +524,12 @@ MockFS::MockFS(int max_read, int max_readahead, bool allow_other,
 		build_iovec(&iov, &iovlen, "auto_unmount",
 			__DECONST(void*, &trueval), sizeof(bool));
 	}
+	if (daemon_timeout > 0) {
+		char val[12];
+
+		snprintf(val, sizeof(val), "%u", daemon_timeout);
+		build_iovec(&iov, &iovlen, "timeout=", &val, -1);
+	}
 	if (*fsname) {
 		build_iovec(&iov, &iovlen, "fsname=",
 			__DECONST(void*, fsname), -1);

tests/sys/fs/fusefs/mockfs.hh

@@ -372,7 +372,7 @@ class MockFS {
 		uint32_t kernel_minor_version, uint32_t max_write, bool async,
 		bool no_clusterr, unsigned time_gran, bool nointr,
 		bool noatime, const char *fsname, const char *subtype,
-		bool no_auto_init, bool auto_unmount);
+		bool no_auto_init, bool auto_unmount, unsigned daemon_timeout);
 
 	virtual ~MockFS();
 

tests/sys/fs/fusefs/pre-init.cc

@@ -47,6 +47,7 @@ class PreInit: public FuseTest {
 public:
 void SetUp() {
 	m_no_auto_init = true;
+	m_daemon_timeout = 1;
 	FuseTest::SetUp();
 }
 };
@@ -64,6 +65,28 @@ void SetUp() {
 }
 };
 
+const char FULLPATH0[] = "mountpoint/some_file.txt";
+static void* access0(void* arg __unused) {
+	ssize_t r;
+
+	r = access(FULLPATH0, F_OK);
+	if (r >= 0)
+		return 0;
+	else
+		return (void*)(intptr_t)errno;
+}
+
+static void* stat1(void* arg) {
+	struct stat *sb = (struct stat*) arg;
+	int r;
+
+	r = stat("mountpoint", sb);
+	if (r != 0)
+               return 0;
+	else
+               return (void*)(intptr_t)errno;
+}
+
 static void* unmount1(void* arg __unused) {
 	ssize_t r;
 
@@ -172,25 +195,65 @@ TEST_F(PreInit, signal_during_unmount_before_init)
 }
 
 /*
- * If some process attempts VOP_GETATTR for the mountpoint before init is
- * complete, fusefs should wait, just like it does for other VOPs.
+ * If the daemon tries to unmount without ever completing INIT but after an
+ * operation like FUSE_ACCESS is blocking, waiting for the daemon, don't
+ * deadlock.  This will probably also happen if the unmount comes from a
+ * different process.
  *
- * To verify that fuse_vnop_getattr does indeed wait for FUSE_INIT to complete,
+ * When m_default_permissions is true, this is a regression test for bug 287431
+ * https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=287431
+ */
+TEST_F(PreInit, access_but_never_init)
+{
+	pthread_t th0;
+
+	/*
+	 * Don't call m_mock->start_service(), so nothing will ever respond to
+	 * FUSE_INIT.
+	 */
+
+	/*
+	* Create a thread to issue a fuse operation.  It will block since
+	* FUSE_INIT is not complete.
+	*/
+	ASSERT_EQ(0, pthread_create(&th0, NULL, access0, NULL));
+	nap();
+
+	/*
+	* Unmounting now, without ever responding to FUSE_INIT, should
+	* hopefully not trigger a deadlock.
+	*/
+	m_mock->unmount();
+
+	pthread_join(th0, NULL);
+}
+
+/*
+ * If some process attempts to access a vnode on the mountpoint before init is
+ * complete, fusefs should wait.
+ *
+ * To verify that fuse_vfsop_root does indeed wait for FUSE_INIT to complete,
  * invoke the test like this:
  *
-> sudo cpuset -c -l 0 dtrace -i 'fbt:fusefs:fuse_internal_init_callback:' -i 'fbt:fusefs:fuse_vnop_getattr:' -c "./pre-init --gtest_filter=PI/PreInitP.getattr_before_init/0"
+> sudo cpuset -c -l 0 dtrace -i 'fbt:fusefs:fuse_vfsop_root: {printf("tid=%d", tid);}' -i 'fbt:fusefs:fuse_internal_init_callback: {printf("tid=%d", tid);}' -i 'fbt:fusefs:fuse_vnop_getattr: {printf("tid=%d", tid);}' -c "./pre-init --gtest_filter=PI/PreInitP.getattr_before_init/0"
 ...
-dtrace: pid 4224 has exited
+dtrace: pid 7399 has exited
 CPU     ID                    FUNCTION:NAME
-  0  68670          fuse_vnop_getattr:entry
-  0  68893 fuse_internal_init_callback:entry
-  0  68894 fuse_internal_init_callback:return
-  0  68671         fuse_vnop_getattr:return
+...
+  0  72004            fuse_vfsop_root:entry tid=102466
+  0  72245 fuse_internal_init_callback:entry tid=102465
+  0  72246 fuse_internal_init_callback:return tid=102465
+  0  72005           fuse_vfsop_root:return tid=102466
+  0  72017          fuse_vnop_getattr:entry tid=102466
+  0  72018         fuse_vnop_getattr:return tid=102466
+...
  *
- * Note that fuse_vnop_getattr was entered first, but exitted last.
+ * Note that fuse_vnop_getattr's thread entered fuse_vfsop_root first, but
+ * exitted only after fuse_internal_init_callback completed.
  */
 TEST_P(PreInitP, getattr_before_init)
 {
+	pthread_t th0;
 	struct stat sb;
 	nlink_t nlink = 12345;
 
@@ -225,7 +288,8 @@ TEST_P(PreInitP, getattr_before_init)
 
 	m_mock->start_service();
 
-	EXPECT_EQ(0, stat("mountpoint", &sb));
+	ASSERT_EQ(0, pthread_create(&th0, NULL, stat1, &sb));
+	pthread_join(th0, NULL);
 	EXPECT_EQ(nlink, sb.st_nlink);
 }
 

tests/sys/fs/fusefs/utils.cc

@@ -152,7 +152,7 @@ void FuseTest::SetUp() {
 			m_pm, m_init_flags, m_kernel_minor_version,
 			m_maxwrite, m_async, m_noclusterr, m_time_gran,
 			m_nointr, m_noatime, m_fsname, m_subtype,
-			m_no_auto_init, m_auto_unmount);
+			m_no_auto_init, m_auto_unmount, m_daemon_timeout);
 		/* 
 		 * FUSE_ACCESS is called almost universally.  Expecting it in
 		 * each test case would be super-annoying.  Instead, set a

tests/sys/fs/fusefs/utils.hh

@@ -72,6 +72,7 @@ class FuseTest : public ::testing::Test {
 	bool m_no_auto_init;
 	bool m_auto_unmount;
 	unsigned m_time_gran;
+	unsigned m_daemon_timeout;
 	MockFS *m_mock = NULL;
 	const static uint64_t FH = 0xdeadbeef1a7ebabe;
 	const char *reclaim_mib = "debug.try_reclaim_vnode";
@@ -100,6 +101,7 @@ class FuseTest : public ::testing::Test {
 		m_no_auto_init(false),
 		m_auto_unmount(false),
 		m_time_gran(1),
+		m_daemon_timeout(0),
 		m_fsname(""),
 		m_subtype(""),
 		m_maxbcachebuf(0),