From nobody Thu Oct 07 21:54:36 2021 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 692BC12D53EB; Thu, 7 Oct 2021 21:54:36 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4HQQ8X2SPbz3D5M; Thu, 7 Oct 2021 21:54:36 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 343EA1B6AD; Thu, 7 Oct 2021 21:54:36 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 197Lsaa0043221; Thu, 7 Oct 2021 21:54:36 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 197LsaIO043220; Thu, 7 Oct 2021 21:54:36 GMT (envelope-from git) Date: Thu, 7 Oct 2021 21:54:36 GMT Message-Id: <202110072154.197LsaIO043220@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org From: Alan Somers Subject: git: 5dc6fedeb40a - stable/13 - fusefs: don't panic if FUSE_GETATTR fails durint VOP_GETPAGES List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: asomers X-Git-Repository: src X-Git-Refname: refs/heads/stable/13 X-Git-Reftype: branch X-Git-Commit: 5dc6fedeb40a2a5a99ef35ed390ba2dbdc28c0bf Auto-Submitted: auto-generated X-ThisMailContainsUnwantedMimeParts: N The branch stable/13 has been updated by asomers: URL: https://cgit.FreeBSD.org/src/commit/?id=5dc6fedeb40a2a5a99ef35ed390ba2dbdc28c0bf commit 5dc6fedeb40a2a5a99ef35ed390ba2dbdc28c0bf Author: Alan Somers AuthorDate: 2021-09-16 19:19:21 +0000 Commit: Alan Somers 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 #include #include +#include +#include #include } @@ -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