From owner-svn-src-projects@freebsd.org Wed Mar 27 02:58:02 2019 Return-Path: Delivered-To: svn-src-projects@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 33060156796A for ; Wed, 27 Mar 2019 02:58:02 +0000 (UTC) (envelope-from asomers@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) server-signature RSA-PSS (4096 bits) client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id C211D91A06; Wed, 27 Mar 2019 02:58:01 +0000 (UTC) (envelope-from asomers@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id AED28FD5; Wed, 27 Mar 2019 02:58:01 +0000 (UTC) (envelope-from asomers@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id x2R2w1l0051964; Wed, 27 Mar 2019 02:58:01 GMT (envelope-from asomers@FreeBSD.org) Received: (from asomers@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id x2R2w0Mk051957; Wed, 27 Mar 2019 02:58:00 GMT (envelope-from asomers@FreeBSD.org) Message-Id: <201903270258.x2R2w0Mk051957@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: asomers set sender to asomers@FreeBSD.org using -f From: Alan Somers Date: Wed, 27 Mar 2019 02:58:00 +0000 (UTC) To: src-committers@freebsd.org, svn-src-projects@freebsd.org Subject: svn commit: r345566 - in projects/fuse2: sys/fs/fuse tests/sys/fs/fusefs X-SVN-Group: projects X-SVN-Commit-Author: asomers X-SVN-Commit-Paths: in projects/fuse2: sys/fs/fuse tests/sys/fs/fusefs X-SVN-Commit-Revision: 345566 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Rspamd-Queue-Id: C211D91A06 X-Spamd-Bar: -- Authentication-Results: mx1.freebsd.org X-Spamd-Result: default: False [-2.95 / 15.00]; local_wl_from(0.00)[FreeBSD.org]; NEURAL_HAM_MEDIUM(-0.99)[-0.994,0]; NEURAL_HAM_SHORT(-0.95)[-0.952,0]; NEURAL_HAM_LONG(-1.00)[-1.000,0]; ASN(0.00)[asn:11403, ipnet:2610:1c1:1::/48, country:US] X-BeenThere: svn-src-projects@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "SVN commit messages for the src " projects" tree" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 27 Mar 2019 02:58:02 -0000 Author: asomers Date: Wed Mar 27 02:57:59 2019 New Revision: 345566 URL: https://svnweb.freebsd.org/changeset/base/345566 Log: fusefs: correctly set fuse_release_in.flags in an error path fuse_vnop_create must close the newly created file if it can't allocate a vnode. When it does so, it must use the same file flags for FUSE_RELEASE as it used for FUSE_OPEN or FUSE_CREATE. Reported by: Coverity Coverity CID: 1066204 Sponsored by: The FreeBSD Foundation Modified: projects/fuse2/sys/fs/fuse/fuse_vnops.c projects/fuse2/tests/sys/fs/fusefs/allow_other.cc projects/fuse2/tests/sys/fs/fusefs/fsync.cc projects/fuse2/tests/sys/fs/fusefs/readdir.cc projects/fuse2/tests/sys/fs/fusefs/release.cc projects/fuse2/tests/sys/fs/fusefs/utils.cc projects/fuse2/tests/sys/fs/fusefs/utils.hh Modified: projects/fuse2/sys/fs/fuse/fuse_vnops.c ============================================================================== --- projects/fuse2/sys/fs/fuse/fuse_vnops.c Wed Mar 27 02:01:34 2019 (r345565) +++ projects/fuse2/sys/fs/fuse/fuse_vnops.c Wed Mar 27 02:57:59 2019 (r345566) @@ -436,7 +436,7 @@ fuse_vnop_create(struct vop_create_args *ap) fdisp_make(fdip, FUSE_RELEASE, mp, nodeid, td, cred); fri = fdip->indata; fri->fh = fh_id; - fri->flags = OFLAGS(mode); + fri->flags = fuse_filehandle_xlate_to_oflags(FUFH_RDWR); fuse_insert_callback(fdip->tick, fuse_internal_forget_callback); fuse_insert_message(fdip->tick); goto out; Modified: projects/fuse2/tests/sys/fs/fusefs/allow_other.cc ============================================================================== --- projects/fuse2/tests/sys/fs/fusefs/allow_other.cc Wed Mar 27 02:01:34 2019 (r345565) +++ projects/fuse2/tests/sys/fs/fusefs/allow_other.cc Wed Mar 27 02:57:59 2019 (r345566) @@ -136,7 +136,7 @@ TEST_F(AllowOther, allowed) */ expect_lookup(RELPATH, ino, S_IFREG | 0644, 0, 1); expect_open(ino, 0, 1); - expect_release(ino, 1, 0, 0); + expect_release(ino); /* Until the attr cache is working, we may send an additional * GETATTR */ expect_getattr(ino, 0); Modified: projects/fuse2/tests/sys/fs/fusefs/fsync.cc ============================================================================== --- projects/fuse2/tests/sys/fs/fusefs/fsync.cc Wed Mar 27 02:01:34 2019 (r345565) +++ projects/fuse2/tests/sys/fs/fusefs/fsync.cc Wed Mar 27 02:57:59 2019 (r345566) @@ -139,7 +139,7 @@ TEST_F(Fsync, close) }, Eq(true)), _) ).Times(0); - expect_release(ino, 1, 0, 0); + expect_release(ino); fd = open(FULLPATH, O_RDWR); ASSERT_LE(0, fd) << strerror(errno); Modified: projects/fuse2/tests/sys/fs/fusefs/readdir.cc ============================================================================== --- projects/fuse2/tests/sys/fs/fusefs/readdir.cc Wed Mar 27 02:01:34 2019 (r345565) +++ projects/fuse2/tests/sys/fs/fusefs/readdir.cc Wed Mar 27 02:57:59 2019 (r345566) @@ -218,7 +218,7 @@ TEST_F(Readdir, getdirentries) r = getdirentries(fd, buf, sizeof(buf), 0); ASSERT_EQ(0, r); - /* Deliberately leak dir. RELEASEDIR will be tested separately */ + /* Deliberately leak fd. RELEASEDIR will be tested separately */ } /* Modified: projects/fuse2/tests/sys/fs/fusefs/release.cc ============================================================================== --- projects/fuse2/tests/sys/fs/fusefs/release.cc Wed Mar 27 02:01:34 2019 (r345565) +++ projects/fuse2/tests/sys/fs/fusefs/release.cc Wed Mar 27 02:57:59 2019 (r345566) @@ -45,6 +45,22 @@ void expect_lookup(const char *relpath, uint64_t ino, { FuseTest::expect_lookup(relpath, ino, S_IFREG | 0644, 0, times); } + +void expect_release(uint64_t ino, uint64_t lock_owner, + uint32_t flags, int error) +{ + EXPECT_CALL(*m_mock, process( + ResultOf([=](auto in) { + return (in->header.opcode == FUSE_RELEASE && + in->header.nodeid == ino && + in->body.release.lock_owner == lock_owner && + in->body.release.fh == FH && + in->body.release.flags == flags); + }, Eq(true)), + _) + ).WillOnce(Invoke(ReturnErrno(error))) + .RetiresOnSaturation(); +} }; class ReleaseWithLocks: public Release { @@ -66,7 +82,7 @@ TEST_F(Release, dup) expect_lookup(RELPATH, ino, 1); expect_open(ino, 0, 1); expect_getattr(ino, 0); - expect_release(ino, 1, 0, 0); + expect_release(ino, 0, O_RDONLY, 0); fd = open(FULLPATH, O_RDONLY); EXPECT_LE(0, fd) << strerror(errno); @@ -95,7 +111,7 @@ TEST_F(Release, eio) expect_lookup(RELPATH, ino, 1); expect_open(ino, 0, 1); expect_getattr(ino, 0); - expect_release(ino, 1, 0, EIO); + expect_release(ino, 0, O_WRONLY, EIO); fd = open(FULLPATH, O_WRONLY); EXPECT_LE(0, fd) << strerror(errno); @@ -104,6 +120,28 @@ TEST_F(Release, eio) } /* + * FUSE_RELEASE should contain the same flags used for FUSE_OPEN + */ +/* https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=236340 */ +TEST_F(Release, DISABLED_flags) +{ + const char FULLPATH[] = "mountpoint/some_file.txt"; + const char RELPATH[] = "some_file.txt"; + uint64_t ino = 42; + int fd; + + expect_lookup(RELPATH, ino, 1); + expect_open(ino, 0, 1); + expect_getattr(ino, 0); + expect_release(ino, 0, O_RDWR | O_APPEND, 0); + + fd = open(FULLPATH, O_RDWR | O_APPEND); + EXPECT_LE(0, fd) << strerror(errno); + + ASSERT_EQ(0, close(fd)) << strerror(errno); +} + +/* * fuse(4) will issue multiple FUSE_OPEN operations for the same file if it's * opened with different modes. Each FUSE_OPEN should get its own * FUSE_RELEASE. @@ -118,11 +156,12 @@ TEST_F(Release, multiple_opens) expect_lookup(RELPATH, ino, 2); expect_open(ino, 0, 2); expect_getattr(ino, 0); - expect_release(ino, 2, 0, 0); + expect_release(ino, 0, O_RDONLY, 0); fd = open(FULLPATH, O_RDONLY); EXPECT_LE(0, fd) << strerror(errno); + expect_release(ino, 0, O_WRONLY, 0); fd2 = open(FULLPATH, O_WRONLY); EXPECT_LE(0, fd2) << strerror(errno); @@ -140,7 +179,7 @@ TEST_F(Release, ok) expect_lookup(RELPATH, ino, 1); expect_open(ino, 0, 1); expect_getattr(ino, 0); - expect_release(ino, 1, 0, 0); + expect_release(ino, 0, O_RDONLY, 0); fd = open(FULLPATH, O_RDONLY); EXPECT_LE(0, fd) << strerror(errno); @@ -173,7 +212,7 @@ TEST_F(ReleaseWithLocks, DISABLED_unlock_on_close) SET_OUT_HEADER_LEN(out, setlk); out->body.setlk.lk = in->body.setlk.lk; }))); - expect_release(ino, 1, (uint64_t)pid, 0); + expect_release(ino, (uint64_t)pid, O_RDWR, 0); fd = open(FULLPATH, O_RDWR); ASSERT_LE(0, fd) << strerror(errno); Modified: projects/fuse2/tests/sys/fs/fusefs/utils.cc ============================================================================== --- projects/fuse2/tests/sys/fs/fusefs/utils.cc Wed Mar 27 02:01:34 2019 (r345565) +++ projects/fuse2/tests/sys/fs/fusefs/utils.cc Wed Mar 27 02:57:59 2019 (r345566) @@ -199,20 +199,18 @@ void FuseTest::expect_read(uint64_t ino, uint64_t offs }))).RetiresOnSaturation(); } -void FuseTest::expect_release(uint64_t ino, int times, uint64_t lock_owner, - int error) +void FuseTest::expect_release(uint64_t ino) { EXPECT_CALL(*m_mock, process( ResultOf([=](auto in) { return (in->header.opcode == FUSE_RELEASE && in->header.nodeid == ino && - in->body.release.lock_owner == lock_owner && in->body.release.fh == FH); }, Eq(true)), _) - ).Times(times) - .WillRepeatedly(Invoke(ReturnErrno(error))); + ).WillOnce(Invoke(ReturnErrno(0))); } + void FuseTest::expect_write(uint64_t ino, uint64_t offset, uint64_t isize, uint64_t osize, uint32_t flags, const void *contents) { Modified: projects/fuse2/tests/sys/fs/fusefs/utils.hh ============================================================================== --- projects/fuse2/tests/sys/fs/fusefs/utils.hh Wed Mar 27 02:01:34 2019 (r345565) +++ projects/fuse2/tests/sys/fs/fusefs/utils.hh Wed Mar 27 02:57:59 2019 (r345566) @@ -112,11 +112,10 @@ class FuseTest : public ::testing::Test { uint64_t osize, const void *contents); /* - * Create an expectation that FUSE_RELEASE will be called times times - * for the given inode, returning error error + * Create an expectation that FUSE_RELEASE will be called exactly once + * for the given inode, returning success */ - void expect_release(uint64_t ino, int times, uint64_t lock_owner, - int error); + void expect_release(uint64_t ino); /* * Create an expectation that FUSE_WRITE will be called exactly once