diff --git a/sys/fs/fuse/fuse_ipc.c b/sys/fs/fuse/fuse_ipc.c index c24e11a7ff2f..6b0bbc22d3c4 100644 --- a/sys/fs/fuse/fuse_ipc.c +++ b/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 */ diff --git a/sys/fs/fuse/fuse_ipc.h b/sys/fs/fuse/fuse_ipc.h index 7091296bb453..788f48c3ffe0 100644 --- a/sys/fs/fuse/fuse_ipc.h +++ b/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); diff --git a/sys/fs/fuse/fuse_vfsops.c b/sys/fs/fuse/fuse_vfsops.c index 7b6ad86e4b2b..aba059fdffc0 100644 --- a/sys/fs/fuse/fuse_vfsops.c +++ b/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; diff --git a/sys/fs/fuse/fuse_vnops.c b/sys/fs/fuse/fuse_vnops.c index 1a4fc60760d8..a5ba292d03c4 100644 --- a/sys/fs/fuse/fuse_vnops.c +++ b/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; } diff --git a/tests/sys/fs/fusefs/mockfs.cc b/tests/sys/fs/fusefs/mockfs.cc index fe4979762d92..d2aec2e7828c 100644 --- a/tests/sys/fs/fusefs/mockfs.cc +++ b/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); diff --git a/tests/sys/fs/fusefs/mockfs.hh b/tests/sys/fs/fusefs/mockfs.hh index 19693e50d212..5955a4bca582 100644 --- a/tests/sys/fs/fusefs/mockfs.hh +++ b/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(); diff --git a/tests/sys/fs/fusefs/pre-init.cc b/tests/sys/fs/fusefs/pre-init.cc index 70d7ddcdb0f4..988040eb6c48 100644 --- a/tests/sys/fs/fusefs/pre-init.cc +++ b/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); } diff --git a/tests/sys/fs/fusefs/utils.cc b/tests/sys/fs/fusefs/utils.cc index eece4ae100cf..25324f0289bc 100644 --- a/tests/sys/fs/fusefs/utils.cc +++ b/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 diff --git a/tests/sys/fs/fusefs/utils.hh b/tests/sys/fs/fusefs/utils.hh index ebd8d41d6961..8bf3765ac5a6 100644 --- a/tests/sys/fs/fusefs/utils.hh +++ b/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),