aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAlan Somers <asomers@FreeBSD.org>2021-10-03 17:51:14 +0000
committerAlan Somers <asomers@FreeBSD.org>2021-10-12 05:41:25 +0000
commit955a418bd3036dc055d97cf98faec0be137b3b11 (patch)
tree1c809393b85cf8a5503c2799dad41f3900ca814b
parent2ccb87689fdf15b62e1a7d078ccd21db53a07e9f (diff)
downloadsrc-955a418bd3036dc055d97cf98faec0be137b3b11.tar.gz
src-955a418bd3036dc055d97cf98faec0be137b3b11.zip
fusefs: Fix a bug during VOP_STRATEGY when the server changes file size
If the FUSE server tells the kernel that a file's size has changed, then the kernel must invalidate any portion of that file in cache. But the kernel can't do that during VOP_STRATEGY, because the file's buffers are already locked. Instead, proceed with the write. PR: 256937 Reported by: Agata <chogata@moosefs.pro> Tested by: Agata <chogata@moosefs.pro> Reviewed by: pfg Differential Revision: https://reviews.freebsd.org/D32332 (cherry picked from commit 032a5bd55b3a003d3560435422a95f27f91685fe)
-rw-r--r--sys/fs/fuse/fuse_io.c19
-rw-r--r--tests/sys/fs/fusefs/write.cc82
2 files changed, 94 insertions, 7 deletions
diff --git a/sys/fs/fuse/fuse_io.c b/sys/fs/fuse/fuse_io.c
index 96ee2496fd54..e163ddedb482 100644
--- a/sys/fs/fuse/fuse_io.c
+++ b/sys/fs/fuse/fuse_io.c
@@ -1047,13 +1047,18 @@ fuse_io_strategy(struct vnode *vp, struct buf *bp)
/*
* Setup for actual write
*/
- error = fuse_vnode_size(vp, &filesize, cred, curthread);
- if (error) {
- bp->b_ioflags |= BIO_ERROR;
- bp->b_error = error;
- bufdone(bp);
- return (error);
- }
+ /*
+ * If the file's size is cached, use that value, even if the
+ * cache is expired. At this point we're already committed to
+ * writing something. If the FUSE server has changed the
+ * file's size behind our back, it's too late for us to do
+ * anything about it. In particular, we can't invalidate any
+ * part of the file's buffers because VOP_STRATEGY is called
+ * with them already locked.
+ */
+ filesize = fvdat->cached_attrs.va_size;
+ /* filesize must've been cached by fuse_vnop_open. */
+ KASSERT(filesize != VNOVAL, ("filesize should've been cached"));
if ((off_t)bp->b_lblkno * biosize + bp->b_dirtyend > filesize)
bp->b_dirtyend = filesize -
diff --git a/tests/sys/fs/fusefs/write.cc b/tests/sys/fs/fusefs/write.cc
index e3fbbf072911..3654a063b765 100644
--- a/tests/sys/fs/fusefs/write.cc
+++ b/tests/sys/fs/fusefs/write.cc
@@ -194,6 +194,10 @@ virtual void SetUp() {
}
};
+
+class WriteEofDuringVnopStrategy: public Write, public WithParamInterface<int>
+{};
+
void sigxfsz_handler(int __unused sig) {
Write::s_sigxfsz = 1;
}
@@ -516,6 +520,84 @@ TEST_F(Write, eof_during_rmw)
}
/*
+ * VOP_STRATEGY should not query the server for the file's size, even if its
+ * cached attributes have expired.
+ * Regression test for https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=256937
+ */
+TEST_P(WriteEofDuringVnopStrategy, eof_during_vop_strategy)
+{
+ const char FULLPATH[] = "mountpoint/some_file.txt";
+ const char RELPATH[] = "some_file.txt";
+ Sequence seq;
+ const off_t filesize = 2 * m_maxbcachebuf;
+ void *contents;
+ uint64_t ino = 42;
+ uint64_t attr_valid = 0;
+ uint64_t attr_valid_nsec = 0;
+ mode_t mode = S_IFREG | 0644;
+ int fd;
+ int ngetattrs;
+
+ ngetattrs = GetParam();
+ contents = calloc(1, filesize);
+
+ EXPECT_LOOKUP(FUSE_ROOT_ID, RELPATH)
+ .WillRepeatedly(Invoke(
+ ReturnImmediate([=](auto in __unused, auto& out) {
+ SET_OUT_HEADER_LEN(out, entry);
+ out.body.entry.attr.mode = mode;
+ out.body.entry.nodeid = ino;
+ out.body.entry.attr.nlink = 1;
+ out.body.entry.attr.size = filesize;
+ out.body.entry.attr_valid = attr_valid;
+ out.body.entry.attr_valid_nsec = attr_valid_nsec;
+ })));
+ expect_open(ino, 0, 1);
+ EXPECT_CALL(*m_mock, process(
+ ResultOf([=](auto in) {
+ return (in.header.opcode == FUSE_GETATTR &&
+ in.header.nodeid == ino);
+ }, Eq(true)),
+ _)
+ ).Times(Between(ngetattrs - 1, ngetattrs))
+ .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_valid = attr_valid;
+ out.body.attr.attr_valid_nsec = attr_valid_nsec;
+ out.body.attr.attr.size = filesize;
+ })));
+ EXPECT_CALL(*m_mock, process(
+ ResultOf([=](auto in) {
+ return (in.header.opcode == FUSE_GETATTR &&
+ in.header.nodeid == ino);
+ }, Eq(true)),
+ _)
+ ).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_valid = attr_valid;
+ out.body.attr.attr_valid_nsec = attr_valid_nsec;
+ out.body.attr.attr.size = filesize / 2;
+ })));
+ expect_write(ino, 0, filesize / 2, filesize / 2, contents);
+
+ fd = open(FULLPATH, O_RDWR);
+ ASSERT_LE(0, fd) << strerror(errno);
+ ASSERT_EQ(filesize / 2, write(fd, contents, filesize / 2))
+ << strerror(errno);
+
+}
+
+INSTANTIATE_TEST_CASE_P(W, WriteEofDuringVnopStrategy,
+ Values(1, 2, 3)
+);
+
+/*
* If the kernel cannot be sure which uid, gid, or pid was responsible for a
* write, then it must set the FUSE_WRITE_CACHE bit
*/