From nobody Tue Jan 18 01:02:22 2022 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 F0302194D83B; Tue, 18 Jan 2022 01:02:22 +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 4Jd9V65qhkz3qKr; Tue, 18 Jan 2022 01:02:22 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1642467742; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=aXsmv1xJfLW/pAS3qGePntkOCassujxLuOcPzWeGeSM=; b=rvKaWXNIZbRrDxIrwW/VOzNSCk9lgjasqvSxApnonMKox3Q6mOwipR6GtIaV0ykv4PHD7h +cNQUNCn+fhuxk6oihakBW8b5Mp1KMv6W8CGzt8kn6fnNLsKlnBwaJibWSZ/lYx8Rjx7Ud 8H9U4YuTA7qCmRi7+fBQlaBwIjjISgrI64zA3R/KlCo0AShOw0xfIkDw4y6NJKiMMxK929 Axdgyxk6V73OvAjIz5cmOiNFKL2nvGniBh8UrMD6dEskerV5PzUEuFsJVM/MFhDY4mJ/gl SABLG3tmK5hG/L5NZMiiX22v3g/du4RR/21GzlIZsVQYsRAQCLh6wJdOz67cFQ== 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 A869C1811B; Tue, 18 Jan 2022 01:02:22 +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 20I12M82014808; Tue, 18 Jan 2022 01:02:22 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 20I12MRJ014806; Tue, 18 Jan 2022 01:02:22 GMT (envelope-from git) Date: Tue, 18 Jan 2022 01:02:22 GMT Message-Id: <202201180102.20I12MRJ014806@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: 36ba360558a7 - stable/13 - Fix a race in fusefs that can corrupt a file's size. 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: 36ba360558a752dfa3832ec19bb182e5cf1876d7 Auto-Submitted: auto-generated ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1642467742; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=aXsmv1xJfLW/pAS3qGePntkOCassujxLuOcPzWeGeSM=; b=vwMMu1uyn9jcsb7vKtOMSCbSNr/dJylPw6y9Jb5kEerufeGUwY4eXopVZosfk/QnWRNwAm 5hfvht1a8y4P12ZHdsY/uBJiIhOESwMSLn6/UkILkMYwtMaMmJVPQfdie2b/ZZGXy+3dQE wp/sFQ8f3EEb9jM3jCD0DJe6dpmrCONGRbVXv0zUr9JCBD8JsDASIQuafinARM1cbCE3NK C+7KvwHG58Trfc6VZlD2BmLvo8IvzX8WLXd26Qhk0yUoD5ZpbI8dzO37wtlb693ER9KNt5 sDtju5v12icaeXCjkFvvXvB/NMBw/5GMgofH4sWYUfFZEXgZlJTeepTA4LmFAQ== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1642467742; a=rsa-sha256; cv=none; b=eN7H4BceWHxwyMnHpKx005W+eOjPgw9p6d2FF5MHee0A8alqoYXujA5FfkNQQM7EPWwuSz bNR0K0AzpOBRLeeybH6rwib7yfemogqBqnlOcB/nZqF7RyT8FM0N7tNcYCP20rCzHtwe0z CdWWuaFC3SzLaWJ8WPkLfgR8dE6huaGDxOJjPQM6sNgrqhWSgUoNiPDeRYh8UON3Hh2SGz g1PTvPsO69FyQglBASVYsD/y+EQVTQPSfbCXspDrrQTtzHMHHFquHOoG6CLemEyiCG9451 ObyJh/jOGhwoiifUbYdA/0LM3eAu96THM77FPNd91RdDvNBxhbK/uWW6wsD/Ug== ARC-Authentication-Results: i=1; mx1.freebsd.org; none X-ThisMailContainsUnwantedMimeParts: N The branch stable/13 has been updated by asomers: URL: https://cgit.FreeBSD.org/src/commit/?id=36ba360558a752dfa3832ec19bb182e5cf1876d7 commit 36ba360558a752dfa3832ec19bb182e5cf1876d7 Author: Alan Somers AuthorDate: 2021-11-29 02:17:34 +0000 Commit: Alan Somers 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 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 +#include +#include + +#include +#include +#include +} + +#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 { +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 out0(new mockfs_buf_out); + std::unique_ptr 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 out0(new mockfs_buf_out); + std::unique_ptr 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") +);