fusefs: add a intr/nointr mount option

FUSE file systems can optionally support interrupting outstanding
operations.  However, the file system does not identify to the kernel at
mount time whether it's capable of doing that.  Instead it signals its
noncapability by returning ENOSYS to the first FUSE_INTERRUPT operation it
receives.  That's a problem for reliable signal delivery, because the kernel
must choose which thread should get a signal before it knows whether the
FUSE server can handle interrupts.  The problem is even worse because the
FUSE protocol allows a file system to simply ignore all FUSE_INTERRUPT
operations.

Fix the signal delivery logic by making interruptibility an opt-in mount
option.  This will require a corresponding change to libfuse, but not to
most file systems that link to libfuse.

Bump __FreeBSD_version due to the new mount option.

Sponsored by:	The FreeBSD Foundation

sbin/mount_fusefs/mount_fusefs.8

@@ -34,7 +34,7 @@
 .\"
 .\" $FreeBSD$
 .\"
-.Dd June 14, 2019
+.Dd July 18, 2019
 .Dt MOUNT_FUSEFS 8
 .Os
 .Sh NAME
@@ -150,6 +150,11 @@ I/O to the file system may be done asynchronously.
 Writes may delayed and/or reordered.
 .It Cm default_permissions
 Enable traditional (file mode based) permission checking in kernel
+.It Cm intr
+Allow signals to interrupt operations that are blocked waiting for a reply from the server.
+When this option is in use, system calls may fail with
+.Er EINTR
+whenever a signal is received.
 .It Cm max_read Ns = Ns Ar n
 Limit size of read requests to
 .Ar n

sbin/mount_fusefs/mount_fusefs.c

@@ -84,6 +84,8 @@ static struct mntopt mopts[] = {
 	 */
 	#define ALTF_AUTOMOUNTED 0x100
 	{ "automounted",	0, ALTF_AUTOMOUNTED, 1 },
+	#define ALTF_INTR 0x200
+	{ "intr",		0, ALTF_INTR, 1 },
 	/* Linux specific options, we silently ignore them */
 	{ "fsname=",             0, 0x00, 1 },
 	{ "fd=",                 0, 0x00, 1 },
@@ -469,6 +471,7 @@ helpmsg(void)
 	        "    -o allow_other         allow access to other users\n"
 	        /* "    -o nonempty            allow mounts over non-empty file/dir\n" */
 	        "    -o default_permissions enable permission checking by kernel\n"
+		"    -o intr                interruptible mount\n"
 		/*
 	        "    -o fsname=NAME         set filesystem name\n"
 	        "    -o large_read          issue large read requests (2.4 only)\n"

sys/fs/fuse/fuse_ipc.c

@@ -438,10 +438,11 @@ fticket_wait_answer(struct fuse_ticket *ftick)
 	struct thread *td = curthread;
 	sigset_t blockedset, oldset;
 	int err = 0, stops_deferred;
-	struct fuse_data *data;
+	struct fuse_data *data = ftick->tk_data;
 	bool interrupted = false;
 
-	if (fsess_isimpl(ftick->tk_data->mp, FUSE_INTERRUPT)) {
+	if (fsess_isimpl(ftick->tk_data->mp, FUSE_INTERRUPT) &&
+	    data->dataflags & FSESS_INTR) {
 		SIGEMPTYSET(blockedset);
 	} else {
 		/* Block all signals except (implicitly) SIGKILL */
@@ -456,7 +457,6 @@ fticket_wait_answer(struct fuse_ticket *ftick)
 	if (fticket_answered(ftick)) {
 		goto out;
 	}
-	data = ftick->tk_data;
 
 	if (fdata_get_dead(data)) {
 		err = ENOTCONN;

sys/fs/fuse/fuse_ipc.h

@@ -232,9 +232,10 @@ struct fuse_data {
 #define FSESS_ASYNC_READ          0x1000 /* allow multiple reads of some file */
 #define FSESS_POSIX_LOCKS         0x2000 /* daemon supports POSIX locks */
 #define FSESS_EXPORT_SUPPORT      0x10000 /* daemon supports NFS-style lookups */
+#define FSESS_INTR                0x20000 /* interruptible mounts */
 #define FSESS_MNTOPTS_MASK	( \
 	FSESS_DAEMON_CAN_SPY | FSESS_PUSH_SYMLINKS_IN | \
-	FSESS_DEFAULT_PERMISSIONS)
+	FSESS_DEFAULT_PERMISSIONS | FSESS_INTR)
 
 extern int fuse_data_cache_mode;
 

sys/fs/fuse/fuse_vfsops.c

@@ -336,6 +336,7 @@ fuse_vfsop_mount(struct mount *mp)
 	FUSE_FLAGOPT(allow_other, FSESS_DAEMON_CAN_SPY);
 	FUSE_FLAGOPT(push_symlinks_in, FSESS_PUSH_SYMLINKS_IN);
 	FUSE_FLAGOPT(default_permissions, FSESS_DEFAULT_PERMISSIONS);
+	FUSE_FLAGOPT(intr, FSESS_INTR);
 
 	(void)vfs_scanopt(opts, "max_read=", "%u", &max_read);
 	if (vfs_scanopt(opts, "timeout=", "%u", &daemon_timeout) == 1) {

sys/sys/param.h

@@ -60,7 +60,7 @@
  *		in the range 5 to 9.
  */
 #undef __FreeBSD_version
-#define __FreeBSD_version 1300034	/* Master, propagated to newvers */
+#define __FreeBSD_version 1300035	/* Master, propagated to newvers */
 
 /*
  * __FreeBSD_kernel__ indicates that this system uses the kernel of FreeBSD,

tests/sys/fs/fusefs/interrupt.cc

@@ -185,6 +185,15 @@ void TearDown() {
 }
 };
 
+class Intr: public Interrupt {};
+
+class Nointr: public Interrupt {
+	void SetUp() {
+		m_nointr = true;
+		Interrupt::SetUp();
+	}
+};
+
 static void* mkdir0(void* arg __unused) {
 	ssize_t r;
 
@@ -213,7 +222,7 @@ static void* read1(void* arg) {
  * complete should generate an EAGAIN response.
  */
 /* https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=236530 */
-TEST_F(Interrupt, already_complete)
+TEST_F(Intr, already_complete)
 {
 	uint64_t ino = 42;
 	pthread_t self;
@@ -271,7 +280,7 @@ TEST_F(Interrupt, already_complete)
  * kernel should not attempt to interrupt any other operations on that mount
  * point.
  */
-TEST_F(Interrupt, enosys)
+TEST_F(Intr, enosys)
 {
 	uint64_t ino0 = 42, ino1 = 43;;
 	uint64_t mkdir_unique;
@@ -356,7 +365,7 @@ TEST_F(Interrupt, enosys)
  * complete the original operation whenever it damn well pleases.
  */
 /* https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=236530 */
-TEST_F(Interrupt, ignore)
+TEST_F(Intr, ignore)
 {
 	uint64_t ino = 42;
 	pthread_t self;
@@ -392,7 +401,7 @@ TEST_F(Interrupt, ignore)
  * that hasn't yet been sent to userland can be interrupted without sending
  * FUSE_INTERRUPT, and will be automatically restarted.
  */
-TEST_F(Interrupt, in_kernel_restartable)
+TEST_F(Intr, in_kernel_restartable)
 {
 	const char FULLPATH1[] = "mountpoint/other_file.txt";
 	const char RELPATH1[] = "other_file.txt";
@@ -462,7 +471,7 @@ TEST_F(Interrupt, in_kernel_restartable)
  * without sending FUSE_INTERRUPT.  If it's a non-restartable operation (write
  * or setextattr) it will return EINTR.
  */
-TEST_F(Interrupt, in_kernel_nonrestartable)
+TEST_F(Intr, in_kernel_nonrestartable)
 {
 	const char FULLPATH1[] = "mountpoint/other_file.txt";
 	const char RELPATH1[] = "other_file.txt";
@@ -533,7 +542,7 @@ TEST_F(Interrupt, in_kernel_nonrestartable)
  * return EINTR to userspace
  */
 /* https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=236530 */
-TEST_F(Interrupt, in_progress)
+TEST_F(Intr, in_progress)
 {
 	pthread_t self;
 	uint64_t mkdir_unique;
@@ -563,7 +572,7 @@ TEST_F(Interrupt, in_progress)
 }
 
 /* Reads should also be interruptible */
-TEST_F(Interrupt, in_progress_read)
+TEST_F(Intr, in_progress_read)
 {
 	const char FULLPATH[] = "mountpoint/some_file.txt";
 	const char RELPATH[] = "some_file.txt";
@@ -601,8 +610,56 @@ TEST_F(Interrupt, in_progress_read)
 	EXPECT_EQ(EINTR, errno);
 }
 
+/*
+ * When mounted with -o nointr, fusefs will block signals while waiting for the
+ * server.
+ */
+TEST_F(Nointr, block)
+{
+	uint64_t ino = 42;
+	pthread_t self;
+	sem_t sem0;
+
+	ASSERT_EQ(0, sem_init(&sem0, 0, 0)) << strerror(errno);
+	signaled_semaphore = &sem0;
+	self = pthread_self();
+
+	EXPECT_LOOKUP(FUSE_ROOT_ID, RELDIRPATH0)
+	.WillOnce(Invoke(ReturnErrno(ENOENT)));
+	EXPECT_CALL(*m_mock, process(
+		ResultOf([=](auto in) {
+			return (in.header.opcode == FUSE_MKDIR);
+		}, Eq(true)),
+		_)
+	).WillOnce(Invoke(ReturnImmediate([&](auto in __unused, auto& out) {
+		/* Let the killer proceed */
+		sem_post(blocked_semaphore);
+
+		/* Wait until after the signal has been sent */
+		sem_wait(signaled_semaphore);
+		/* Allow time for the mkdir thread to receive the signal */
+		nap();
+
+		/* Finally, complete the original op */
+		SET_OUT_HEADER_LEN(out, entry);
+		out.body.create.entry.attr.mode = S_IFDIR | MODE;
+		out.body.create.entry.nodeid = ino;
+	})));
+	EXPECT_CALL(*m_mock, process(
+		ResultOf([&](auto in) {
+			return (in.header.opcode == FUSE_INTERRUPT);
+		}, Eq(true)),
+		_)
+	).Times(0);
+
+	setup_interruptor(self);
+	ASSERT_EQ(0, mkdir(FULLDIRPATH0, MODE)) << strerror(errno);
+
+	sem_destroy(&sem0);
+}
+
 /* FUSE_INTERRUPT operations should take priority over other pending ops */
-TEST_F(Interrupt, priority)
+TEST_F(Intr, priority)
 {
 	Sequence seq;
 	uint64_t ino1 = 43;
@@ -688,7 +745,7 @@ TEST_F(Interrupt, priority)
  * successfully interrupts the original
  */
 /* https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=236530 */
-TEST_F(Interrupt, too_soon)
+TEST_F(Intr, too_soon)
 {
 	Sequence seq;
 	pthread_t self;

tests/sys/fs/fusefs/mockfs.cc

@@ -348,7 +348,7 @@ void MockFS::debug_response(const mockfs_buf_out &out) {
 MockFS::MockFS(int max_readahead, bool allow_other, bool default_permissions,
 	bool push_symlinks_in, bool ro, enum poll_method pm, uint32_t flags,
 	uint32_t kernel_minor_version, uint32_t max_write, bool async,
-	bool noclusterr, unsigned time_gran)
+	bool noclusterr, unsigned time_gran, bool nointr)
 {
 	struct sigaction sa;
 	struct iovec *iov = NULL;
@@ -426,6 +426,13 @@ MockFS::MockFS(int max_readahead, bool allow_other, bool default_permissions,
 		build_iovec(&iov, &iovlen, "noclusterr",
 			__DECONST(void*, &trueval), sizeof(bool));
 	}
+	if (nointr) {
+		build_iovec(&iov, &iovlen, "nointr",
+			__DECONST(void*, &trueval), sizeof(bool));
+	} else {
+		build_iovec(&iov, &iovlen, "intr",
+			__DECONST(void*, &trueval), sizeof(bool));
+	}
 	if (nmount(iov, iovlen, 0))
 		throw(std::system_error(errno, std::system_category(),
 			"Couldn't mount filesystem"));

tests/sys/fs/fusefs/mockfs.hh

@@ -323,7 +323,7 @@ class MockFS {
 		bool default_permissions, bool push_symlinks_in, bool ro,
 		enum poll_method pm, uint32_t flags,
 		uint32_t kernel_minor_version, uint32_t max_write, bool async,
-		bool no_clusterr, unsigned time_gran);
+		bool no_clusterr, unsigned time_gran, bool nointr);
 
 	virtual ~MockFS();
 

tests/sys/fs/fusefs/utils.cc

@@ -117,7 +117,8 @@ void FuseTest::SetUp() {
 		m_mock = new MockFS(m_maxreadahead, m_allow_other,
 			m_default_permissions, m_push_symlinks_in, m_ro,
 			m_pm, m_init_flags, m_kernel_minor_version,
-			m_maxwrite, m_async, m_noclusterr, m_time_gran);
+			m_maxwrite, m_async, m_noclusterr, m_time_gran,
+			m_nointr);
 		/* 
 		 * 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

@@ -57,6 +57,7 @@ class FuseTest : public ::testing::Test {
 	bool m_ro;
 	bool m_async;
 	bool m_noclusterr;
+	bool m_nointr;
 	unsigned m_time_gran;
 	MockFS *m_mock = NULL;
 	const static uint64_t FH = 0xdeadbeef1a7ebabe;
@@ -77,6 +78,7 @@ class FuseTest : public ::testing::Test {
 		m_ro(false),
 		m_async(false),
 		m_noclusterr(false),
+		m_nointr(false),
 		m_time_gran(1)
 	{}