git: c858f3d89b3c - stable/12 - fusefs: don't panic if FUSE_GETATTR fails durint VOP_GETPAGES
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Thu, 07 Oct 2021 20:00:36 UTC
The branch stable/12 has been updated by asomers:
URL: https://cgit.FreeBSD.org/src/commit/?id=c858f3d89b3c92cf3c0962c976dc6bd322ad8b9b
commit c858f3d89b3c92cf3c0962c976dc6bd322ad8b9b
Author: Konstantin Belousov <kib@FreeBSD.org>
AuthorDate: 2021-09-16 23:53:58 +0000
Commit: Alan Somers <asomers@FreeBSD.org>
CommitDate: 2021-10-07 19:51:35 +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)
buffer pager: allow get_blksize method to return error
Reported and reviewed by: asomers
Sponsored by: The FreeBSD Foundation
Differential revision: https://reviews.freebsd.org/D31998
(cherry picked from commit 197a4f29f39e6ae6215a6dbd28ef449d305e6d49)
---
sys/fs/cd9660/cd9660_vnops.c | 5 +-
sys/fs/fuse/fuse_vnops.c | 21 +++---
sys/fs/msdosfs/msdosfs_vnops.c | 5 +-
sys/fs/nfsclient/nfs_clbio.c | 5 +-
sys/kern/vfs_bio.c | 16 +++--
sys/sys/buf.h | 2 +-
sys/ufs/ffs/ffs_vnops.c | 5 +-
tests/sys/fs/fusefs/read.cc | 156 +++++++++++++++++++++++++++++++++++++++++
8 files changed, 189 insertions(+), 26 deletions(-)
diff --git a/sys/fs/cd9660/cd9660_vnops.c b/sys/fs/cd9660/cd9660_vnops.c
index 27a186964a80..2007b62087a4 100644
--- a/sys/fs/cd9660/cd9660_vnops.c
+++ b/sys/fs/cd9660/cd9660_vnops.c
@@ -859,12 +859,13 @@ cd9660_gbp_getblkno(struct vnode *vp, vm_ooffset_t off)
}
static int
-cd9660_gbp_getblksz(struct vnode *vp, daddr_t lbn)
+cd9660_gbp_getblksz(struct vnode *vp, daddr_t lbn, long *sz)
{
struct iso_node *ip;
ip = VTOI(vp);
- return (blksize(ip->i_mnt, ip, lbn));
+ *sz = blksize(ip->i_mnt, ip, lbn);
+ return (0);
}
static int
diff --git a/sys/fs/fuse/fuse_vnops.c b/sys/fs/fuse/fuse_vnops.c
index 9e79a376cccd..d13ae257e0a8 100644
--- a/sys/fs/fuse/fuse_vnops.c
+++ b/sys/fs/fuse/fuse_vnops.c
@@ -1961,25 +1961,24 @@ fuse_gbp_getblkno(struct vnode *vp, vm_ooffset_t off)
}
static int
-fuse_gbp_getblksz(struct vnode *vp, daddr_t lbn)
+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;
}
- return (blksz);
+ return (0);
}
/*
diff --git a/sys/fs/msdosfs/msdosfs_vnops.c b/sys/fs/msdosfs/msdosfs_vnops.c
index d700f6a62d5c..c93754a0e251 100644
--- a/sys/fs/msdosfs/msdosfs_vnops.c
+++ b/sys/fs/msdosfs/msdosfs_vnops.c
@@ -1805,10 +1805,11 @@ msdosfs_gbp_getblkno(struct vnode *vp, vm_ooffset_t off)
}
static int
-msdosfs_gbp_getblksz(struct vnode *vp, daddr_t lbn)
+msdosfs_gbp_getblksz(struct vnode *vp, daddr_t lbn, long *sz)
{
- return (VTODE(vp)->de_pmp->pm_bpcluster);
+ *sz = VTODE(vp)->de_pmp->pm_bpcluster;
+ return (0);
}
static int
diff --git a/sys/fs/nfsclient/nfs_clbio.c b/sys/fs/nfsclient/nfs_clbio.c
index 5ba75491a800..c61cb1db9b95 100644
--- a/sys/fs/nfsclient/nfs_clbio.c
+++ b/sys/fs/nfsclient/nfs_clbio.c
@@ -94,7 +94,7 @@ ncl_gbp_getblkno(struct vnode *vp, vm_ooffset_t off)
}
static int
-ncl_gbp_getblksz(struct vnode *vp, daddr_t lbn)
+ncl_gbp_getblksz(struct vnode *vp, daddr_t lbn, long *sz)
{
struct nfsnode *np;
u_quad_t nsize;
@@ -111,7 +111,8 @@ ncl_gbp_getblksz(struct vnode *vp, daddr_t lbn)
bcount = 0;
else if ((off_t)(lbn + 1) * biosize > nsize)
bcount = nsize - (off_t)lbn * biosize;
- return (bcount);
+ *sz = bcount;
+ return (0);
}
int
diff --git a/sys/kern/vfs_bio.c b/sys/kern/vfs_bio.c
index 6c5286d30871..d58a13cf43e1 100644
--- a/sys/kern/vfs_bio.c
+++ b/sys/kern/vfs_bio.c
@@ -5143,8 +5143,8 @@ vfs_bio_getpages(struct vnode *vp, vm_page_t *ma, int count,
struct mount *mp;
daddr_t lbn, lbnp;
vm_ooffset_t la, lb, poff, poffe;
- long bsize;
- int bo_bs, br_flags, error, i, pgsin, pgsin_a, pgsin_b;
+ long bo_bs, bsize;
+ int br_flags, error, i, pgsin, pgsin_a, pgsin_b;
bool redo, lpart;
object = vp->v_object;
@@ -5161,7 +5161,10 @@ vfs_bio_getpages(struct vnode *vp, vm_page_t *ma, int count,
*/
la += PAGE_SIZE;
lpart = la > object->un_pager.vnp.vnp_size;
- bo_bs = get_blksize(vp, get_lblkno(vp, IDX_TO_OFF(ma[0]->pindex)));
+ error = get_blksize(vp, get_lblkno(vp, IDX_TO_OFF(ma[0]->pindex)),
+ &bo_bs);
+ if (error != 0)
+ return (VM_PAGER_ERROR);
/*
* Calculate read-ahead, behind and total pages.
@@ -5219,9 +5222,10 @@ again:
goto next_page;
lbnp = lbn;
- bsize = get_blksize(vp, lbn);
- error = bread_gb(vp, lbn, bsize, curthread->td_ucred,
- br_flags, &bp);
+ error = get_blksize(vp, lbn, &bsize);
+ if (error == 0)
+ error = bread_gb(vp, lbn, bsize,
+ curthread->td_ucred, br_flags, &bp);
if (error != 0)
goto end_pages;
if (bp->b_rcred == curthread->td_ucred) {
diff --git a/sys/sys/buf.h b/sys/sys/buf.h
index 89c11a36bb42..2530a3540c23 100644
--- a/sys/sys/buf.h
+++ b/sys/sys/buf.h
@@ -576,7 +576,7 @@ void bwait(struct buf *, u_char, const char *);
void bdone(struct buf *);
typedef daddr_t (vbg_get_lblkno_t)(struct vnode *, vm_ooffset_t);
-typedef int (vbg_get_blksize_t)(struct vnode *, daddr_t);
+typedef int (vbg_get_blksize_t)(struct vnode *, daddr_t, long *);
int vfs_bio_getpages(struct vnode *vp, struct vm_page **ma, int count,
int *rbehind, int *rahead, vbg_get_lblkno_t get_lblkno,
vbg_get_blksize_t get_blksize);
diff --git a/sys/ufs/ffs/ffs_vnops.c b/sys/ufs/ffs/ffs_vnops.c
index 9dae437f9a76..b7357cf0ecab 100644
--- a/sys/ufs/ffs/ffs_vnops.c
+++ b/sys/ufs/ffs/ffs_vnops.c
@@ -1739,10 +1739,11 @@ ffs_gbp_getblkno(struct vnode *vp, vm_ooffset_t off)
}
static int
-ffs_gbp_getblksz(struct vnode *vp, daddr_t lbn)
+ffs_gbp_getblksz(struct vnode *vp, daddr_t lbn, long *sz)
{
- return (blksize(VFSTOUFS(vp->v_mount)->um_fs, VTOI(vp), lbn));
+ *sz = blksize(VFSTOUFS(vp->v_mount)->um_fs, VTOI(vp), lbn);
+ return (0);
}
static int
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