Date: Thu, 7 Oct 2021 21:54:36 GMT From: Alan Somers <asomers@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org Subject: git: 5dc6fedeb40a - stable/13 - fusefs: don't panic if FUSE_GETATTR fails durint VOP_GETPAGES Message-ID: <202110072154.197LsaIO043220@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202110072154.197LsaIO043220>