git: 5dc6fedeb40a - stable/13 - fusefs: don't panic if FUSE_GETATTR fails durint VOP_GETPAGES

From: Alan Somers <asomers_at_FreeBSD.org>
Date: Thu, 07 Oct 2021 21:54:36 UTC
The branch stable/13 has been updated by asomers:

URL: https://cgit.FreeBSD.org/src/commit/?id=5dc6fedeb40a2a5a99ef35ed390ba2dbdc28c0bf

commit 5dc6fedeb40a2a5a99ef35ed390ba2dbdc28c0bf
Author:     Alan Somers <asomers@FreeBSD.org>
AuthorDate: 2021-09-16 19:19:21 +0000
Commit:     Alan Somers <asomers@FreeBSD.org>
CommitDate: 2021-10-07 21:43:45 +0000

    fusefs: don't panic if FUSE_GETATTR fails durint VOP_GETPAGES
    
    During VOP_GETPAGES, fusefs needs to determine the file's length, which
    could require a FUSE_GETATTR operation.  If that fails, it's better to
    SIGBUS than panic.
    
    Sponsored by:   Axcient
    Reviewed by:    markj, kib
    Differential Revision: https://reviews.freebsd.org/D31994
    
    (cherry picked from commit 4f917847c9037d9b76de188c03e13b81224431b2)
---
 sys/fs/fuse/fuse_vnops.c    |  20 +++---
 tests/sys/fs/fusefs/read.cc | 156 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 165 insertions(+), 11 deletions(-)

diff --git a/sys/fs/fuse/fuse_vnops.c b/sys/fs/fuse/fuse_vnops.c
index 1216c1252f2b..4c8c8c1d4461 100644
--- a/sys/fs/fuse/fuse_vnops.c
+++ b/sys/fs/fuse/fuse_vnops.c
@@ -2199,25 +2199,23 @@ fuse_gbp_getblkno(struct vnode *vp, vm_ooffset_t off)
 }
 
 static int
-fuse_gbp_getblksz(struct vnode *vp, daddr_t lbn, long *sz)
+fuse_gbp_getblksz(struct vnode *vp, daddr_t lbn, long *blksz)
 {
 	off_t filesize;
-	int blksz, err;
+	int err;
 	const int biosize = fuse_iosize(vp);
 
 	err = fuse_vnode_size(vp, &filesize, NULL, NULL);
-	KASSERT(err == 0, ("vfs_bio_getpages can't handle errors here"));
-	if (err)
-		return biosize;
-
-	if ((off_t)lbn * biosize >= filesize) {
-		blksz = 0;
+	if (err) {
+		/* This will turn into a SIGBUS */
+		return (EIO);
+	} else if ((off_t)lbn * biosize >= filesize) {
+		*blksz = 0;
 	} else if ((off_t)(lbn + 1) * biosize > filesize) {
-		blksz = filesize - (off_t)lbn *biosize;
+		*blksz = filesize - (off_t)lbn *biosize;
 	} else {
-		blksz = biosize;
+		*blksz = biosize;
 	}
-	*sz = blksz;
 	return (0);
 }
 
diff --git a/tests/sys/fs/fusefs/read.cc b/tests/sys/fs/fusefs/read.cc
index 22a26c2701fd..cb82d0a43b06 100644
--- a/tests/sys/fs/fusefs/read.cc
+++ b/tests/sys/fs/fusefs/read.cc
@@ -40,6 +40,8 @@ extern "C" {
 #include <aio.h>
 #include <fcntl.h>
 #include <semaphore.h>
+#include <setjmp.h>
+#include <signal.h>
 #include <unistd.h>
 }
 
@@ -103,6 +105,33 @@ class ReadAhead: public Read,
 	}
 };
 
+class ReadSigbus: public Read
+{
+public:
+static jmp_buf s_jmpbuf;
+static sig_atomic_t s_si_addr;
+
+void TearDown() {
+	struct sigaction sa;
+
+	bzero(&sa, sizeof(sa));
+	sa.sa_handler = SIG_DFL;
+	sigaction(SIGBUS, &sa, NULL);
+
+	FuseTest::TearDown();
+}
+
+};
+
+static void
+handle_sigbus(int signo __unused, siginfo_t *info, void *uap __unused) {
+	ReadSigbus::s_si_addr = (sig_atomic_t)info->si_addr;
+	longjmp(ReadSigbus::s_jmpbuf, 1);
+}
+
+jmp_buf ReadSigbus::s_jmpbuf;
+sig_atomic_t ReadSigbus::s_si_addr;
+
 /* AIO reads need to set the header's pid field correctly */
 /* https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=236379 */
 TEST_F(AioRead, aio_read)
@@ -584,6 +613,56 @@ TEST_F(Read, mmap)
 	leak(fd);
 }
 
+/* Read of an mmap()ed file fails */
+TEST_F(ReadSigbus, mmap_eio)
+{
+	const char FULLPATH[] = "mountpoint/some_file.txt";
+	const char RELPATH[] = "some_file.txt";
+	const char *CONTENTS = "abcdefgh";
+	struct sigaction sa;
+	uint64_t ino = 42;
+	int fd;
+	ssize_t len;
+	size_t bufsize = strlen(CONTENTS);
+	void *p;
+
+	len = getpagesize();
+
+	expect_lookup(RELPATH, ino, bufsize);
+	expect_open(ino, 0, 1);
+	EXPECT_CALL(*m_mock, process(
+		ResultOf([=](auto in) {
+			return (in.header.opcode == FUSE_READ &&
+				in.header.nodeid == ino &&
+				in.body.read.fh == Read::FH);
+		}, Eq(true)),
+		_)
+	).WillRepeatedly(Invoke(ReturnErrno(EIO)));
+
+	fd = open(FULLPATH, O_RDONLY);
+	ASSERT_LE(0, fd) << strerror(errno);
+
+	p = mmap(NULL, len, PROT_READ, MAP_SHARED, fd, 0);
+	ASSERT_NE(MAP_FAILED, p) << strerror(errno);
+
+	/* Accessing the mapped page should return SIGBUS.  */
+
+	bzero(&sa, sizeof(sa));
+	sa.sa_handler = SIG_DFL;
+	sa.sa_sigaction = handle_sigbus;
+	sa.sa_flags = SA_RESETHAND | SA_SIGINFO;
+	ASSERT_EQ(0, sigaction(SIGBUS, &sa, NULL)) << strerror(errno);
+	if (setjmp(ReadSigbus::s_jmpbuf) == 0) {
+		atomic_signal_fence(std::memory_order::memory_order_seq_cst);
+		volatile char x __unused = *(volatile char*)p;
+		FAIL() << "shouldn't get here";
+	}
+
+	ASSERT_EQ(p, (void*)ReadSigbus::s_si_addr);
+	ASSERT_EQ(0, munmap(p, len)) << strerror(errno);
+	leak(fd);
+}
+
 /* 
  * A read via mmap comes up short, indicating that the file was truncated
  * server-side.
@@ -634,6 +713,83 @@ TEST_F(Read, mmap_eof)
 	leak(fd);
 }
 
+/*
+ * During VOP_GETPAGES, the FUSE server fails a FUSE_GETATTR operation.  This
+ * almost certainly indicates a buggy FUSE server, and our goal should be not
+ * to panic.  Instead, generate SIGBUS.
+ */
+TEST_F(ReadSigbus, mmap_getblksz_fail)
+{
+	const char FULLPATH[] = "mountpoint/some_file.txt";
+	const char RELPATH[] = "some_file.txt";
+	const char *CONTENTS = "abcdefgh";
+	struct sigaction sa;
+	Sequence seq;
+	uint64_t ino = 42;
+	int fd;
+	ssize_t len;
+	size_t bufsize = strlen(CONTENTS);
+	mode_t mode = S_IFREG | 0644;
+	void *p;
+
+	len = getpagesize();
+
+	FuseTest::expect_lookup(RELPATH, ino, mode, bufsize, 1, 0);
+	/* Expect two GETATTR calls that succeed, followed by one that fail. */
+	EXPECT_CALL(*m_mock, process(
+		ResultOf([=](auto in) {
+			return (in.header.opcode == FUSE_GETATTR &&
+				in.header.nodeid == ino);
+		}, Eq(true)),
+		_)
+	).Times(2)
+	.InSequence(seq)
+	.WillRepeatedly(Invoke(ReturnImmediate([=](auto i __unused, auto& out) {
+		SET_OUT_HEADER_LEN(out, attr);
+		out.body.attr.attr.ino = ino;
+		out.body.attr.attr.mode = mode;
+		out.body.attr.attr.size = bufsize;
+		out.body.attr.attr_valid = 0;
+	})));
+	EXPECT_CALL(*m_mock, process(
+		ResultOf([=](auto in) {
+			return (in.header.opcode == FUSE_GETATTR &&
+				in.header.nodeid == ino);
+		}, Eq(true)),
+		_)
+	).InSequence(seq)
+	.WillRepeatedly(Invoke(ReturnErrno(EIO)));
+	expect_open(ino, 0, 1);
+	EXPECT_CALL(*m_mock, process(
+		ResultOf([=](auto in) {
+			return (in.header.opcode == FUSE_READ);
+		}, Eq(true)),
+		_)
+	).Times(0);
+
+	fd = open(FULLPATH, O_RDONLY);
+	ASSERT_LE(0, fd) << strerror(errno);
+
+	p = mmap(NULL, len, PROT_READ, MAP_SHARED, fd, 0);
+	ASSERT_NE(MAP_FAILED, p) << strerror(errno);
+
+	/* Accessing the mapped page should return SIGBUS.  */
+	bzero(&sa, sizeof(sa));
+	sa.sa_handler = SIG_DFL;
+	sa.sa_sigaction = handle_sigbus;
+	sa.sa_flags = SA_RESETHAND | SA_SIGINFO;
+	ASSERT_EQ(0, sigaction(SIGBUS, &sa, NULL)) << strerror(errno);
+	if (setjmp(ReadSigbus::s_jmpbuf) == 0) {
+		atomic_signal_fence(std::memory_order::memory_order_seq_cst);
+		volatile char x __unused = *(volatile char*)p;
+		FAIL() << "shouldn't get here";
+	}
+
+	ASSERT_EQ(p, (void*)ReadSigbus::s_si_addr);
+	ASSERT_EQ(0, munmap(p, len)) << strerror(errno);
+	leak(fd);
+}
+
 /*
  * Just as when FOPEN_DIRECT_IO is used, reads with O_DIRECT should bypass
  * cache and to straight to the daemon