git: 155ac516c60f - main - fusefs: validate servers' error values

The branch main has been updated by asomers:

URL: https://cgit.FreeBSD.org/src/commit/?id=155ac516c60f20573d15c54bafabfd0e52d21fa6

commit 155ac516c60f20573d15c54bafabfd0e52d21fa6
Author:     Alan Somers <asomers@FreeBSD.org>
AuthorDate: 2022-04-15 19:04:24 +0000
Commit:     Alan Somers <asomers@FreeBSD.org>
CommitDate: 2022-04-15 19:57:32 +0000

    fusefs: validate servers' error values
    
    Formerly fusefs would pass up the stack any error value returned by the
    fuse server.  However, some values aren't valid for userland, but have
    special meanings within the kernel.  One of these, EJUSTRETURN, could
    cause a kernel page fault if the server returned it in response to
    FUSE_LOOKUP.  Fix by validating all errors returned by the server.
    
    Also, fix a data lifetime bug in the FUSE_DESTROY test.
    
    PR:             263220
    Reported by:    Robert Morris <rtm@lcs.mit.edu>
    MFC after:      3 weeks
    Sponsored by:   Axcient
    Reviewed by:    emaste
    Differential Revision: https://reviews.freebsd.org/D34931
---
 sys/fs/fuse/fuse_device.c     | 14 ++++++++++++--
 tests/sys/fs/fusefs/lookup.cc | 21 +++++++++++++++++++++
 tests/sys/fs/fusefs/mockfs.cc |  9 ++++++++-
 tests/sys/fs/fusefs/mockfs.hh |  3 +++
 tests/sys/fs/fusefs/utils.cc  |  2 +-
 5 files changed, 45 insertions(+), 4 deletions(-)

diff --git a/sys/fs/fuse/fuse_device.c b/sys/fs/fuse/fuse_device.c
index 157c3802ec7e..7d1afb88edb7 100644
--- a/sys/fs/fuse/fuse_device.c
+++ b/sys/fs/fuse/fuse_device.c
@@ -518,8 +518,18 @@ fuse_device_write(struct cdev *dev, struct uio *uio, int ioflag)
 				"pass ticket to a callback");
 			/* Sanitize the linuxism of negative errnos */
 			ohead.error *= -1;
-			memcpy(&tick->tk_aw_ohead, &ohead, sizeof(ohead));
-			err = tick->tk_aw_handler(tick, uio);
+			if (ohead.error < 0 || ohead.error > ELAST) {
+				/* Illegal error code */
+				ohead.error = EIO;
+				memcpy(&tick->tk_aw_ohead, &ohead,
+					sizeof(ohead));
+				tick->tk_aw_handler(tick, uio);
+				err = EINVAL;
+			} else {
+				memcpy(&tick->tk_aw_ohead, &ohead,
+					sizeof(ohead));
+				err = tick->tk_aw_handler(tick, uio);
+			}
 		} else {
 			/* pretender doesn't wanna do anything with answer */
 			SDT_PROBE2(fusefs, , device, trace, 1,
diff --git a/tests/sys/fs/fusefs/lookup.cc b/tests/sys/fs/fusefs/lookup.cc
index 32e2a08eb949..0ec02913f66a 100644
--- a/tests/sys/fs/fusefs/lookup.cc
+++ b/tests/sys/fs/fusefs/lookup.cc
@@ -276,6 +276,27 @@ TEST_F(Lookup, dotdot_no_parent_nid)
 	EXPECT_EQ(ESTALE, errno);
 }
 
+/*
+ * A daemon that returns an illegal error value should be handled gracefully.
+ * Regression test for https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=263220
+ */
+TEST_F(Lookup, ejustreturn)
+{
+	const char FULLPATH[] = "mountpoint/does_not_exist";
+	const char RELPATH[] = "does_not_exist";
+
+	EXPECT_LOOKUP(FUSE_ROOT_ID, RELPATH)
+	.WillOnce(Invoke(ReturnImmediate([=](auto in __unused, auto& out) {
+		out.header.len = sizeof(out.header);
+		out.header.error = 2;
+		m_mock->m_expected_write_errno = EINVAL;
+	})));
+
+	EXPECT_NE(0, access(FULLPATH, F_OK));
+
+	EXPECT_EQ(EIO, errno);
+}
+
 TEST_F(Lookup, enoent)
 {
 	const char FULLPATH[] = "mountpoint/does_not_exist";
diff --git a/tests/sys/fs/fusefs/mockfs.cc b/tests/sys/fs/fusefs/mockfs.cc
index 231f46b18aba..ddfb5527ef13 100644
--- a/tests/sys/fs/fusefs/mockfs.cc
+++ b/tests/sys/fs/fusefs/mockfs.cc
@@ -418,6 +418,7 @@ MockFS::MockFS(int max_readahead, bool allow_other, bool default_permissions,
 	const bool trueval = true;
 
 	m_daemon_id = NULL;
+	m_expected_write_errno = 0;
 	m_kernel_minor_version = kernel_minor_version;
 	m_maxreadahead = max_readahead;
 	m_maxwrite = MIN(max_write, max_max_write);
@@ -779,6 +780,7 @@ void MockFS::loop() {
 
 		bzero(in.get(), sizeof(*in));
 		read_request(*in, buflen);
+		m_expected_write_errno = 0;
 		if (m_quit)
 			break;
 		if (verbosity > 0)
@@ -1011,7 +1013,12 @@ void MockFS::write_response(const mockfs_buf_out &out) {
 		FAIL() << "not yet implemented";
 	}
 	r = write(m_fuse_fd, &out, out.header.len);
-	ASSERT_TRUE(r > 0 || errno == EAGAIN) << strerror(errno);
+	if (m_expected_write_errno) {
+		ASSERT_EQ(-1, r);
+		ASSERT_EQ(m_expected_write_errno, errno) << strerror(errno);
+	} else {
+		ASSERT_TRUE(r > 0 || errno == EAGAIN) << strerror(errno);
+	}
 }
 
 void* MockFS::service(void *pthr_data) {
diff --git a/tests/sys/fs/fusefs/mockfs.hh b/tests/sys/fs/fusefs/mockfs.hh
index e35f2efb8dae..d471491ea455 100644
--- a/tests/sys/fs/fusefs/mockfs.hh
+++ b/tests/sys/fs/fusefs/mockfs.hh
@@ -340,6 +340,9 @@ class MockFS {
 	/* pid of child process, for two-process test cases */
 	pid_t m_child_pid;
 
+	/* the expected errno of the next write to /dev/fuse */
+	int m_expected_write_errno;
+
 	/* Maximum size of a FUSE_WRITE write */
 	uint32_t m_maxwrite;
 
diff --git a/tests/sys/fs/fusefs/utils.cc b/tests/sys/fs/fusefs/utils.cc
index 508c3af2828f..65738f4b19a4 100644
--- a/tests/sys/fs/fusefs/utils.cc
+++ b/tests/sys/fs/fusefs/utils.cc
@@ -217,7 +217,7 @@ FuseTest::expect_destroy(int error)
 			return (in.header.opcode == FUSE_DESTROY);
 		}, Eq(true)),
 		_)
-	).WillOnce(Invoke( ReturnImmediate([&](auto in, auto& out) {
+	).WillOnce(Invoke(ReturnImmediate([=](auto in, auto& out) {
 		m_mock->m_quit = true;
 		out.header.len = sizeof(out.header);
 		out.header.unique = in.header.unique;