From nobody Tue Dec 07 05:17:53 2021 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 83CE618CB73F; Tue, 7 Dec 2021 05:17:53 +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 4J7T8K1qXxz57Lv; Tue, 7 Dec 2021 05:17:53 +0000 (UTC) (envelope-from git@FreeBSD.org) 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 1EA2619B60; Tue, 7 Dec 2021 05:17:53 +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 1B75Hrrf009653; Tue, 7 Dec 2021 05:17:53 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 1B75HrN0009652; Tue, 7 Dec 2021 05:17:53 GMT (envelope-from git) Date: Tue, 7 Dec 2021 05:17:53 GMT Message-Id: <202112070517.1B75HrN0009652@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: 7adb46c70f2e - stable/13 - fusefs: Fix a bug during VOP_STRATEGY when the server changes file 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: 7adb46c70f2e050386422ffdda38ada3b61cd998 Auto-Submitted: auto-generated ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1638854273; 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=DTU6j1CEmqxYeVHzNTQDYqZsgfZSZ01fYSucXtO5ggQ=; b=oKotRaM7RtLeLNASbgY7lVwLguRy82Frt6V+FV9BQHEwSjlpJsJjYkZRzz9a5Ju/V5c0hc JI9xTw0S8TedjlTLFAo4+UaM2uGE33zajPMd9/a3Mvx1+2rEFwQXwCGN9yN/NSdgdO6qmd 2OhaMWjrqMZcpN/eAWWTC8/S1XfzVEKsBU8+sXT2/pvVfTLVQ6L8N8JB98mC9+rCsOxlrQ W8Im9JRVpZNaeaxtFa77vxpAHX7i/LjU0k/agmYEvrpF8ljLHxcB16Zc2w4IhdglTlnzf8 +vMwWmUm6q8wpDklCToX5H5N0svfPY5liqn51mY3M24TtGH/kMIhCIZSB7DxXQ== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1638854273; a=rsa-sha256; cv=none; b=fl5BPcVslU3jH9RTiBzu/PFN7nfSHQJMwpEjcMqBM/1Pm2o6Sx1rg7WGczFvUpu0z1T53Y xWv1qzIM+eGs4TdITI287Azrps6Pr2UHdkY9emz/Ok9eVdzNVKqaMVO+a6FVeMywbNB4A+ clb1YHtTPg0dry54RBI6aaVBisZQjKvMSbA05g6eNDLIfTWp4tajcxQDJZDtMTTNEuE5le FFLvlh0nvouZpVkZL9XTLPn2HIb/GcYguSDK3blkULOXcKV2uzGMxfcOULY5aaeSkSY4KT lFOFEwEficJLTQhp2jnXUpw/yyrLLs2gAcVfuIhkdcjX5S4g8uP2CBSay4uzMg== 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=7adb46c70f2e050386422ffdda38ada3b61cd998 commit 7adb46c70f2e050386422ffdda38ada3b61cd998 Author: Alan Somers AuthorDate: 2021-10-03 17:51:14 +0000 Commit: Alan Somers CommitDate: 2021-12-07 05:14:10 +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 Tested by: Agata Reviewed by: pfg Differential Revision: https://reviews.freebsd.org/D32332 (cherry picked from commit 032a5bd55b3a003d3560435422a95f27f91685fe) --- 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 26f3039f0e14..a240217fcc08 100644 --- a/sys/fs/fuse/fuse_io.c +++ b/sys/fs/fuse/fuse_io.c @@ -1006,13 +1006,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 +{}; + 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