git: 36ba360558a7 - stable/13 - Fix a race in fusefs that can corrupt a file's size.

From: Alan Somers <asomers_at_FreeBSD.org>
Date: Tue, 18 Jan 2022 01:02:22 UTC
The branch stable/13 has been updated by asomers:

URL: https://cgit.FreeBSD.org/src/commit/?id=36ba360558a752dfa3832ec19bb182e5cf1876d7

commit 36ba360558a752dfa3832ec19bb182e5cf1876d7
Author:     Alan Somers <asomers@FreeBSD.org>
AuthorDate: 2021-11-29 02:17:34 +0000
Commit:     Alan Somers <asomers@FreeBSD.org>
CommitDate: 2022-01-18 00:59:03 +0000

    Fix a race in fusefs that can corrupt a file's size.
    
    VOPs like VOP_SETATTR can change a file's size, with the vnode
    exclusively locked.  But VOPs like VOP_LOOKUP look up the file size from
    the server without the vnode locked.  So a race is possible.  For
    example:
    
    1) One thread calls VOP_SETATTR to truncate a file.  It locks the vnode
       and sends FUSE_SETATTR to the server.
    2) A second thread calls VOP_LOOKUP and fetches the file's attributes from
       the server.  Then it blocks trying to acquire the vnode lock.
    3) FUSE_SETATTR returns and the first thread releases the vnode lock.
    4) The second thread acquires the vnode lock and caches the file's
       attributes, which are now out-of-date.
    
    Fix this race by recording a timestamp in the vnode of the last time
    that its filesize was modified.  Check that timestamp during VOP_LOOKUP
    and VFS_VGET.  If it's newer than the time at which FUSE_LOOKUP was
    issued to the server, ignore the attributes returned by FUSE_LOOKUP.
    
    PR:             259071
    Reported by:    Agata <chogata@moosefs.pro>
    Reviewed by:    pfg
    Differential Revision: https://reviews.freebsd.org/D33158
    
    (cherry picked from commit 13d593a5b060cf7be40acfa2ca9dc9e0e2339a31)
---
 sys/fs/fuse/fuse_internal.c              |   5 +
 sys/fs/fuse/fuse_io.c                    |   5 +-
 sys/fs/fuse/fuse_node.c                  |   5 +-
 sys/fs/fuse/fuse_node.h                  |   6 +
 sys/fs/fuse/fuse_vfsops.c                |  13 +-
 sys/fs/fuse/fuse_vnops.c                 |  21 +-
 tests/sys/fs/fusefs/Makefile             |   1 +
 tests/sys/fs/fusefs/last_local_modify.cc | 469 +++++++++++++++++++++++++++++++
 8 files changed, 516 insertions(+), 9 deletions(-)

diff --git a/sys/fs/fuse/fuse_internal.c b/sys/fs/fuse/fuse_internal.c
index b9df64921fc8..218d072566ac 100644
--- a/sys/fs/fuse/fuse_internal.c
+++ b/sys/fs/fuse/fuse_internal.c
@@ -932,6 +932,8 @@ fuse_internal_do_getattr(struct vnode *vp, struct vattr *vap,
 	enum vtype vtyp;
 	int err;
 
+	ASSERT_VOP_LOCKED(vp, __func__);
+
 	fdisp_init(&fdi, sizeof(*fgai));
 	fdisp_make_vp(&fdi, FUSE_GETATTR, vp, td, cred);
 	fgai = fdi.indata;
@@ -1170,6 +1172,8 @@ int fuse_internal_setattr(struct vnode *vp, struct vattr *vap,
 	int sizechanged = -1;
 	uint64_t newsize = 0;
 
+	ASSERT_VOP_ELOCKED(vp, __func__);
+
 	mp = vnode_mount(vp);
 	fvdat = VTOFUD(vp);
 	data = fuse_get_mpdata(mp);
@@ -1272,6 +1276,7 @@ int fuse_internal_setattr(struct vnode *vp, struct vattr *vap,
 		fuse_vnode_undirty_cached_timestamps(vp, true);
 		fuse_internal_cache_attrs(vp, &fao->attr, fao->attr_valid,
 			fao->attr_valid_nsec, NULL, false);
+		getnanouptime(&fvdat->last_local_modify);
 	}
 
 out:
diff --git a/sys/fs/fuse/fuse_io.c b/sys/fs/fuse/fuse_io.c
index 609d08f9e3d5..991def5ba3eb 100644
--- a/sys/fs/fuse/fuse_io.c
+++ b/sys/fs/fuse/fuse_io.c
@@ -399,8 +399,10 @@ retry:
 		diff = fwi->size - fwo->size;
 		as_written_offset = uio->uio_offset - diff;
 
-		if (as_written_offset - diff > filesize)
+		if (as_written_offset - diff > filesize) {
 			fuse_vnode_setsize(vp, as_written_offset, false);
+			getnanouptime(&fvdat->last_local_modify);
+		}
 		if (as_written_offset - diff >= filesize)
 			fvdat->flag &= ~FN_SIZECHANGE;
 
@@ -546,6 +548,7 @@ again:
 			 */
 			err = fuse_vnode_setsize(vp, uio->uio_offset + n, false);
 			filesize = uio->uio_offset + n;
+			getnanouptime(&fvdat->last_local_modify);
 			fvdat->flag |= FN_SIZECHANGE;
 			if (err) {
 				brelse(bp);
diff --git a/sys/fs/fuse/fuse_node.c b/sys/fs/fuse/fuse_node.c
index b5f46daf025d..450760a624ff 100644
--- a/sys/fs/fuse/fuse_node.c
+++ b/sys/fs/fuse/fuse_node.c
@@ -164,6 +164,7 @@ fuse_vnode_init(struct vnode *vp, struct fuse_vnode_data *fvdat,
 	}
 	vp->v_type = vtyp;
 	vp->v_data = fvdat;
+	timespecclear(&fvdat->last_local_modify);
 
 	counter_u64_add(fuse_node_count, 1);
 }
@@ -384,8 +385,10 @@ fuse_vnode_savesize(struct vnode *vp, struct ucred *cred, pid_t pid)
 	}
 	err = fdisp_wait_answ(&fdi);
 	fdisp_destroy(&fdi);
-	if (err == 0)
+	if (err == 0) {
+		getnanouptime(&fvdat->last_local_modify);
 		fvdat->flag &= ~FN_SIZECHANGE;
+	}
 
 	return err;
 }
diff --git a/sys/fs/fuse/fuse_node.h b/sys/fs/fuse/fuse_node.h
index 3a33c57001dc..05edede50cc7 100644
--- a/sys/fs/fuse/fuse_node.h
+++ b/sys/fs/fuse/fuse_node.h
@@ -116,6 +116,12 @@ struct fuse_vnode_data {
 	 * by nodeid instead of pathname.
 	 */
 	struct bintime	entry_cache_timeout;
+	/*
+	 * Monotonic time of the last FUSE operation that modified the file
+	 * size.  Used to avoid races between mutator ops like VOP_SETATTR and
+	 * unlocked accessor ops like VOP_LOOKUP.
+	 */
+	struct timespec	last_local_modify;
 	struct vattr	cached_attrs;
 	uint64_t	nlookup;
 	enum vtype	vtype;
diff --git a/sys/fs/fuse/fuse_vfsops.c b/sys/fs/fuse/fuse_vfsops.c
index 3d5e168bcde2..6d988f9b3136 100644
--- a/sys/fs/fuse/fuse_vfsops.c
+++ b/sys/fs/fuse/fuse_vfsops.c
@@ -538,6 +538,7 @@ fuse_vfsop_vget(struct mount *mp, ino_t ino, int flags, struct vnode **vpp)
 	struct fuse_dispatcher fdi;
 	struct fuse_entry_out *feo;
 	struct fuse_vnode_data *fvdat;
+	struct timespec now;
 	const char dot[] = ".";
 	enum vtype vtyp;
 	int error;
@@ -555,6 +556,8 @@ fuse_vfsop_vget(struct mount *mp, ino_t ino, int flags, struct vnode **vpp)
 	if (error || *vpp != NULL)
 		return error;
 
+	getnanouptime(&now);
+
 	/* Do a LOOKUP, using nodeid as the parent and "." as filename */
 	fdisp_init(&fdi, sizeof(dot));
 	fdisp_make(&fdi, FUSE_LOOKUP, mp, nodeid, td, td->td_ucred);
@@ -577,8 +580,14 @@ fuse_vfsop_vget(struct mount *mp, ino_t ino, int flags, struct vnode **vpp)
 		goto out;
 	fvdat = VTOFUD(*vpp);
 
-	fuse_internal_cache_attrs(*vpp, &feo->attr, feo->attr_valid,
-		feo->attr_valid_nsec, NULL, true);
+	if (timespeccmp(&now, &fvdat->last_local_modify, >)) {
+		/*
+		 * Attributes from the server are definitely newer than the
+		 * last attributes we sent to the server, so cache them.
+		 */
+		fuse_internal_cache_attrs(*vpp, &feo->attr, feo->attr_valid,
+			feo->attr_valid_nsec, NULL, true);
+	}
 	fuse_validity_2_bintime(feo->entry_valid, feo->entry_valid_nsec,
 		&fvdat->entry_cache_timeout);
 out:
diff --git a/sys/fs/fuse/fuse_vnops.c b/sys/fs/fuse/fuse_vnops.c
index 7fe3f8271f4f..1655c3200eed 100644
--- a/sys/fs/fuse/fuse_vnops.c
+++ b/sys/fs/fuse/fuse_vnops.c
@@ -803,8 +803,10 @@ fuse_vnop_copy_file_range(struct vop_copy_file_range_args *ap)
 		*ap->a_inoffp += fwo->size;
 		*ap->a_outoffp += fwo->size;
 		fuse_internal_clear_suid_on_write(outvp, outcred, td);
-		if (*ap->a_outoffp > outfvdat->cached_attrs.va_size)
-			fuse_vnode_setsize(outvp, *ap->a_outoffp, false);
+		if (*ap->a_outoffp > outfvdat->cached_attrs.va_size) {
+                        fuse_vnode_setsize(outvp, *ap->a_outoffp, false);
+			getnanouptime(&outfvdat->last_local_modify);
+		}
 	}
 	fdisp_destroy(&fdi);
 
@@ -1279,6 +1281,7 @@ fuse_vnop_lookup(struct vop_lookup_args *ap)
 	struct componentname *cnp = ap->a_cnp;
 	struct thread *td = cnp->cn_thread;
 	struct ucred *cred = cnp->cn_cred;
+	struct timespec now;
 
 	int nameiop = cnp->cn_nameiop;
 	int flags = cnp->cn_flags;
@@ -1330,7 +1333,7 @@ fuse_vnop_lookup(struct vop_lookup_args *ap)
 		vtyp = VDIR;
 		filesize = 0;
 	} else {
-		struct timespec now, timeout;
+		struct timespec timeout;
 		int ncpticks; /* here to accomodate for API contract */
 
 		err = cache_lookup(dvp, vpp, cnp, &timeout, &ncpticks);
@@ -1454,8 +1457,16 @@ fuse_vnop_lookup(struct vop_lookup_args *ap)
 			fvdat = VTOFUD(vp);
 
 			MPASS(feo != NULL);
-			fuse_internal_cache_attrs(*vpp, &feo->attr,
-				feo->attr_valid, feo->attr_valid_nsec, NULL, true);
+			if (timespeccmp(&now, &fvdat->last_local_modify, >)) {
+				/*
+				 * Attributes from the server are definitely
+				 * newer than the last attributes we sent to
+				 * the server, so cache them.
+				 */
+				fuse_internal_cache_attrs(*vpp, &feo->attr,
+					feo->attr_valid, feo->attr_valid_nsec,
+					NULL, true);
+			}
 			fuse_validity_2_bintime(feo->entry_valid,
 				feo->entry_valid_nsec,
 				&fvdat->entry_cache_timeout);
diff --git a/tests/sys/fs/fusefs/Makefile b/tests/sys/fs/fusefs/Makefile
index c3706e940840..e508d3edbdea 100644
--- a/tests/sys/fs/fusefs/Makefile
+++ b/tests/sys/fs/fusefs/Makefile
@@ -27,6 +27,7 @@ GTESTS+=	fsyncdir
 GTESTS+=	getattr
 GTESTS+=	interrupt
 GTESTS+=	io
+GTESTS+=	last_local_modify
 GTESTS+=	link
 GTESTS+=	locks
 GTESTS+=	lookup
diff --git a/tests/sys/fs/fusefs/last_local_modify.cc b/tests/sys/fs/fusefs/last_local_modify.cc
new file mode 100644
index 000000000000..a203a02e922b
--- /dev/null
+++ b/tests/sys/fs/fusefs/last_local_modify.cc
@@ -0,0 +1,469 @@
+/*-
+ * SPDX-License-Identifier: BSD-2-Clause-FreeBSD
+ *
+ * Copyright (c) 2021 Alan Somers
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
+ * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
+ * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+ * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+ * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
+ * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ *
+ * $FreeBSD$
+ */
+
+extern "C" {
+#include <sys/param.h>
+#include <sys/mount.h>
+#include <sys/stat.h>
+
+#include <fcntl.h>
+#include <pthread.h>
+#include <semaphore.h>
+}
+
+#include "mockfs.hh"
+#include "utils.hh"
+
+using namespace testing;
+
+/*
+ * "Last Local Modify" bugs
+ *
+ * This file tests a class of race conditions caused by one thread fetching a
+ * file's size with FUSE_LOOKUP while another thread simultaneously modifies it
+ * with FUSE_SETATTR, FUSE_WRITE, FUSE_COPY_FILE_RANGE or similar.  It's
+ * possible for the second thread to start later yet finish first. If that
+ * happens, the first thread must not override the size set by the second
+ * thread.
+ *
+ * FUSE_GETATTR is not vulnerable to the same race, because it is always called
+ * with the vnode lock held.
+ *
+ * A few other operations like FUSE_LINK can also trigger the same race but
+ * with the file's ctime instead of size.  However, the consequences of an
+ * incorrect ctime are much less disastrous than an incorrect size, so fusefs
+ * does not attempt to prevent such races.
+ */
+
+enum Mutator {
+	VOP_SETATTR,
+	VOP_WRITE,
+	VOP_COPY_FILE_RANGE
+};
+
+/*
+ * Translate a poll method's string representation to the enum value.
+ * Using strings with ::testing::Values gives better output with
+ * --gtest_list_tests
+ */
+enum Mutator writer_from_str(const char* s) {
+	if (0 == strcmp("VOP_SETATTR", s))
+		return VOP_SETATTR;
+	else if (0 == strcmp("VOP_WRITE", s))
+		return VOP_WRITE;
+	else
+		return VOP_COPY_FILE_RANGE;
+}
+
+uint32_t fuse_op_from_mutator(enum Mutator mutator) {
+	switch(mutator) {
+	case VOP_SETATTR:
+		return(FUSE_SETATTR);
+	case VOP_WRITE:
+		return(FUSE_WRITE);
+	case VOP_COPY_FILE_RANGE:
+		return(FUSE_COPY_FILE_RANGE);
+	}
+}
+
+class LastLocalModify: public FuseTest, public WithParamInterface<const char*> {
+public:
+virtual void SetUp() {
+	m_init_flags = FUSE_EXPORT_SUPPORT;
+
+	FuseTest::SetUp();
+}
+};
+
+static void* copy_file_range_th(void* arg) {
+	ssize_t r;
+	int fd;
+	sem_t *sem = (sem_t*) arg;
+	off_t off_in = 0;
+	off_t off_out = 10;
+	ssize_t len = 5;
+
+	if (sem)
+		sem_wait(sem);
+	fd = open("mountpoint/some_file.txt", O_RDWR);
+	if (fd < 0)
+		return (void*)(intptr_t)errno;
+
+	r = copy_file_range(fd, &off_in, fd, &off_out, len, 0);
+	if (r >= 0) {
+		LastLocalModify::leak(fd);
+		return 0;
+	} else
+		return (void*)(intptr_t)errno;
+}
+
+static void* setattr_th(void* arg) {
+	int fd;
+	ssize_t r;
+	sem_t *sem = (sem_t*) arg;
+
+	if (sem)
+		sem_wait(sem);
+
+	fd = open("mountpoint/some_file.txt", O_RDWR);
+	if (fd < 0)
+		return (void*)(intptr_t)errno;
+
+	r = ftruncate(fd, 15);
+	if (r >= 0)
+		return 0;
+	else
+		return (void*)(intptr_t)errno;
+}
+
+static void* write_th(void* arg) {
+	ssize_t r;
+	int fd;
+	sem_t *sem = (sem_t*) arg;
+	const char BUF[] = "abcdefghijklmn";
+
+	if (sem)
+		sem_wait(sem);
+	fd = open("mountpoint/some_file.txt", O_RDWR);
+	if (fd < 0)
+		return (void*)(intptr_t)errno;
+
+	r = write(fd, BUF, sizeof(BUF));
+	if (r >= 0) {
+		LastLocalModify::leak(fd);
+		return 0;
+	} else
+		return (void*)(intptr_t)errno;
+}
+
+/*
+ * VOP_LOOKUP should discard attributes returned by the server if they were
+ * modified by another VOP while the VOP_LOOKUP was in progress.
+ *
+ * Sequence of operations:
+ * * Thread 1 calls a mutator like ftruncate, which acquires the vnode lock
+ *   exclusively.
+ * * Thread 2 calls stat, which does VOP_LOOKUP, which sends FUSE_LOOKUP to the
+ *   server.  The server replies with the old file length.  Thread 2 blocks
+ *   waiting for the vnode lock.
+ * * Thread 1 sends the mutator operation like FUSE_SETATTR that changes the
+ *   file's size and updates the attribute cache.  Then it releases the vnode
+ *   lock.
+ * * Thread 2 acquires the vnode lock.  At this point it must not add the
+ *   now-stale file size to the attribute cache.
+ *
+ * Regression test for https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=259071
+ */
+TEST_P(LastLocalModify, lookup)
+{
+	const char FULLPATH[] = "mountpoint/some_file.txt";
+	const char RELPATH[] = "some_file.txt";
+	Sequence seq;
+	uint64_t ino = 3;
+	uint64_t mutator_unique;
+	const uint64_t oldsize = 10;
+	const uint64_t newsize = 15;
+	pthread_t th0;
+	void *thr0_value;
+	struct stat sb;
+	static sem_t sem;
+	Mutator mutator;
+	uint32_t mutator_op;
+	size_t mutator_size;
+
+	mutator = writer_from_str(GetParam());
+	mutator_op = fuse_op_from_mutator(mutator);
+
+	ASSERT_EQ(0, sem_init(&sem, 0, 0)) << strerror(errno);
+
+	EXPECT_LOOKUP(FUSE_ROOT_ID, RELPATH)
+	.InSequence(seq)
+	.WillOnce(Invoke(ReturnImmediate([=](auto in __unused, auto& out) {
+		/* Called by the mutator, caches attributes but not entries */
+		SET_OUT_HEADER_LEN(out, entry);
+		out.body.entry.nodeid = ino;
+		out.body.entry.attr.size = oldsize;
+		out.body.entry.nodeid = ino;
+		out.body.entry.attr_valid_nsec = NAP_NS / 2;
+		out.body.entry.attr.ino = ino;
+		out.body.entry.attr.mode = S_IFREG | 0644;
+	})));
+	expect_open(ino, 0, 1);
+	EXPECT_CALL(*m_mock, process(
+		ResultOf([=](auto in) {
+			return (in.header.opcode == mutator_op &&
+				in.header.nodeid == ino);
+		}, Eq(true)),
+		_)
+	).InSequence(seq)
+	.WillOnce(Invoke([&](auto in, auto &out __unused) {
+		/*
+		 * The mutator changes the file size, but in order to simulate
+		 * a race, don't reply.  Instead, just save the unique for
+		 * later.
+		 */
+		mutator_unique = in.header.unique;
+		switch(mutator) {
+		case VOP_WRITE:
+			mutator_size = in.body.write.size;
+			break;
+		case VOP_COPY_FILE_RANGE:
+			mutator_size = in.body.copy_file_range.len;
+			break;
+		default:
+			break;
+		}
+		/* Allow the lookup thread to proceed */
+		sem_post(&sem);
+	}));
+	EXPECT_LOOKUP(FUSE_ROOT_ID, RELPATH)
+	.InSequence(seq)
+	.WillOnce(Invoke([&](auto in __unused, auto& out) {
+		std::unique_ptr<mockfs_buf_out> out0(new mockfs_buf_out);
+		std::unique_ptr<mockfs_buf_out> out1(new mockfs_buf_out);
+
+		/* First complete the lookup request, returning the old size */
+		out0->header.unique = in.header.unique;
+		SET_OUT_HEADER_LEN(*out0, entry);
+		out0->body.entry.attr.mode = S_IFREG | 0644;
+		out0->body.entry.nodeid = ino;
+		out0->body.entry.entry_valid = UINT64_MAX;
+		out0->body.entry.attr_valid = UINT64_MAX;
+		out0->body.entry.attr.size = oldsize;
+		out.push_back(std::move(out0));
+
+		/* Then, respond to the mutator request */
+		out1->header.unique = mutator_unique;
+		switch(mutator) {
+		case VOP_SETATTR:
+			SET_OUT_HEADER_LEN(*out1, attr);
+			out1->body.attr.attr.ino = ino;
+			out1->body.attr.attr.mode = S_IFREG | 0644;
+			out1->body.attr.attr.size = newsize;	// Changed size
+			out1->body.attr.attr_valid = UINT64_MAX;
+			break;
+		case VOP_WRITE:
+			SET_OUT_HEADER_LEN(*out1, write);
+			out1->body.write.size = mutator_size;
+			break;
+		case VOP_COPY_FILE_RANGE:
+			SET_OUT_HEADER_LEN(*out1, write);
+			out1->body.write.size = mutator_size;
+			break;
+		}
+		out.push_back(std::move(out1));
+	}));
+
+	/* Start the mutator thread */
+	switch(mutator) {
+	case VOP_SETATTR:
+		ASSERT_EQ(0, pthread_create(&th0, NULL, setattr_th, NULL))
+			<< strerror(errno);
+		break;
+	case VOP_WRITE:
+		ASSERT_EQ(0, pthread_create(&th0, NULL, write_th, NULL))
+			<< strerror(errno);
+		break;
+	case VOP_COPY_FILE_RANGE:
+		ASSERT_EQ(0, pthread_create(&th0, NULL, copy_file_range_th,
+			NULL)) << strerror(errno);
+		break;
+	}
+
+
+	/* Wait for FUSE_SETATTR to be sent */
+	sem_wait(&sem);
+
+	/* Lookup again, which will race with setattr */
+	ASSERT_EQ(0, stat(FULLPATH, &sb)) << strerror(errno);
+	ASSERT_EQ((off_t)newsize, sb.st_size);
+
+	/* ftruncate should've completed without error */
+	pthread_join(th0, &thr0_value);
+	EXPECT_EQ(0, (intptr_t)thr0_value);
+}
+
+/*
+ * VFS_VGET should discard attributes returned by the server if they were
+ * modified by another VOP while the VFS_VGET was in progress.
+ *
+ * Sequence of operations:
+ * * Thread 1 calls fhstat, entering VFS_VGET, and issues FUSE_LOOKUP
+ * * Thread 2 calls a mutator like ftruncate, which acquires the vnode lock
+ *   exclusively and issues a FUSE op like FUSE_SETATTR.
+ * * Thread 1's FUSE_LOOKUP returns with the old size, but the thread blocks
+ *   waiting for the vnode lock.
+ * * Thread 2's FUSE op returns, and that thread sets the file's new size
+ *   in the attribute cache.  Finally it releases the vnode lock.
+ * * The vnode lock acquired, thread 1 must not overwrite the attr cache's size
+ *   with the old value.
+ *
+ * Regression test for https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=259071
+ */
+TEST_P(LastLocalModify, vfs_vget)
+{
+	const char FULLPATH[] = "mountpoint/some_file.txt";
+	const char RELPATH[] = "some_file.txt";
+	Sequence seq;
+	uint64_t ino = 3;
+	uint64_t lookup_unique;
+	const uint64_t oldsize = 10;
+	const uint64_t newsize = 15;
+	pthread_t th0;
+	void *thr0_value;
+	struct stat sb;
+	static sem_t sem;
+	fhandle_t fhp;
+	Mutator mutator;
+	uint32_t mutator_op;
+
+	if (geteuid() != 0)
+		GTEST_SKIP() << "This test requires a privileged user";
+
+	mutator = writer_from_str(GetParam());
+	mutator_op = fuse_op_from_mutator(mutator);
+
+	ASSERT_EQ(0, sem_init(&sem, 0, 0)) << strerror(errno);
+
+	EXPECT_LOOKUP(FUSE_ROOT_ID, RELPATH)
+	.Times(1)
+	.InSequence(seq)
+	.WillOnce(Invoke(ReturnImmediate([=](auto in __unused, auto& out)
+	{
+		/* Called by getfh, caches attributes but not entries */
+		SET_OUT_HEADER_LEN(out, entry);
+		out.body.entry.nodeid = ino;
+		out.body.entry.attr.size = oldsize;
+		out.body.entry.nodeid = ino;
+		out.body.entry.attr_valid_nsec = NAP_NS / 2;
+		out.body.entry.attr.ino = ino;
+		out.body.entry.attr.mode = S_IFREG | 0644;
+	})));
+	EXPECT_LOOKUP(ino, ".")
+	.InSequence(seq)
+	.WillOnce(Invoke([&](auto in, auto &out __unused) {
+		/* Called by fhstat.  Block to simulate a race */
+		lookup_unique = in.header.unique;
+		sem_post(&sem);
+	}));
+
+	EXPECT_LOOKUP(FUSE_ROOT_ID, RELPATH)
+	.Times(1)
+	.InSequence(seq)
+	.WillRepeatedly(Invoke(ReturnImmediate([=](auto in __unused, auto& out)
+	{
+		/* Called by VOP_SETATTR, caches attributes but not entries */
+		SET_OUT_HEADER_LEN(out, entry);
+		out.body.entry.nodeid = ino;
+		out.body.entry.attr.size = oldsize;
+		out.body.entry.nodeid = ino;
+		out.body.entry.attr_valid_nsec = NAP_NS / 2;
+		out.body.entry.attr.ino = ino;
+		out.body.entry.attr.mode = S_IFREG | 0644;
+	})));
+
+	/* Called by the mutator thread */
+	expect_open(ino, 0, 1);
+
+	EXPECT_CALL(*m_mock, process(
+		ResultOf([=](auto in) {
+			return (in.header.opcode == mutator_op &&
+				in.header.nodeid == ino);
+		}, Eq(true)),
+		_)
+	).InSequence(seq)
+	.WillOnce(Invoke([&](auto in __unused, auto& out) {
+		std::unique_ptr<mockfs_buf_out> out0(new mockfs_buf_out);
+		std::unique_ptr<mockfs_buf_out> out1(new mockfs_buf_out);
+
+		/* First complete the lookup request, returning the old size */
+		out0->header.unique = lookup_unique;
+		SET_OUT_HEADER_LEN(*out0, entry);
+		out0->body.entry.attr.mode = S_IFREG | 0644;
+		out0->body.entry.nodeid = ino;
+		out0->body.entry.entry_valid = UINT64_MAX;
+		out0->body.entry.attr_valid = UINT64_MAX;
+		out0->body.entry.attr.size = oldsize;
+		out.push_back(std::move(out0));
+
+		/* Then, respond to the mutator request */
+		out1->header.unique = in.header.unique;
+		switch(mutator) {
+		case VOP_SETATTR:
+			SET_OUT_HEADER_LEN(*out1, attr);
+			out1->body.attr.attr.ino = ino;
+			out1->body.attr.attr.mode = S_IFREG | 0644;
+			out1->body.attr.attr.size = newsize;	// Changed size
+			out1->body.attr.attr_valid = UINT64_MAX;
+			break;
+		case VOP_WRITE:
+			SET_OUT_HEADER_LEN(*out1, write);
+			out1->body.write.size = in.body.write.size;
+			break;
+		case VOP_COPY_FILE_RANGE:
+			SET_OUT_HEADER_LEN(*out1, write);
+			out1->body.write.size = in.body.copy_file_range.len;
+			break;
+		}
+		out.push_back(std::move(out1));
+	}));
+
+	/* First get a file handle */
+	ASSERT_EQ(0, getfh(FULLPATH, &fhp)) << strerror(errno);
+
+	/* Start the mutator thread */
+	switch(mutator) {
+	case VOP_SETATTR:
+		ASSERT_EQ(0, pthread_create(&th0, NULL, setattr_th,
+			(void*)&sem)) << strerror(errno);
+		break;
+	case VOP_WRITE:
+		ASSERT_EQ(0, pthread_create(&th0, NULL, write_th, (void*)&sem))
+			<< strerror(errno);
+		break;
+	case VOP_COPY_FILE_RANGE:
+		ASSERT_EQ(0, pthread_create(&th0, NULL, copy_file_range_th,
+			(void*)&sem)) << strerror(errno);
+		break;
+	}
+
+	/* Lookup again, which will race with setattr */
+	ASSERT_EQ(0, fhstat(&fhp, &sb)) << strerror(errno);
+
+	ASSERT_EQ((off_t)newsize, sb.st_size);
+
+	/* mutator should've completed without error */
+	pthread_join(th0, &thr0_value);
+	EXPECT_EQ(0, (intptr_t)thr0_value);
+}
+
+
+INSTANTIATE_TEST_CASE_P(LLM, LastLocalModify,
+	Values("VOP_SETATTR", "VOP_WRITE", "VOP_COPY_FILE_RANGE")
+);