git: 032a5bd55b3a - main - fusefs: Fix a bug during VOP_STRATEGY when the server changes file size

The branch main has been updated by asomers:

URL: https://cgit.FreeBSD.org/src/commit/?id=032a5bd55b3a003d3560435422a95f27f91685fe

commit 032a5bd55b3a003d3560435422a95f27f91685fe
Author:     Alan Somers <asomers@FreeBSD.org>
AuthorDate: 2021-10-03 17:51:14 +0000
Commit:     Alan Somers <asomers@FreeBSD.org>
CommitDate: 2021-10-06 20:07:33 +0000

    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>
    MFC after:      2 weeks
    Reviewed by:    pfg
    Differential Revision: https://reviews.freebsd.org/D32332
---
 sys/fs/fuse/fuse_io.c        | 19 +++++++----
 tests/sys/fs/fusefs/write.cc | 81 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 93 insertions(+), 7 deletions(-)

diff --git a/sys/fs/fuse/fuse_io.c b/sys/fs/fuse/fuse_io.c
index 637c36424d3f..f818fbd52869 100644
--- a/sys/fs/fuse/fuse_io.c
+++ b/sys/fs/fuse/fuse_io.c
@@ -1007,13 +1007,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 fadf4648d31c..db5bb3fe4441 100644
--- a/tests/sys/fs/fusefs/write.cc
+++ b/tests/sys/fs/fusefs/write.cc
@@ -208,6 +208,9 @@ virtual void SetUp() {
 }
 };
 
+class WriteEofDuringVnopStrategy: public Write, public WithParamInterface<int>
+{};
+
 void sigxfsz_handler(int __unused sig) {
 	Write::s_sigxfsz = 1;
 }
@@ -526,6 +529,84 @@ TEST_F(Write, eof_during_rmw)
 	leak(fd);
 }
 
+/*
+ * 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