Date: Wed, 27 Mar 2019 02:58:00 +0000 (UTC) From: Alan Somers <asomers@FreeBSD.org> To: src-committers@freebsd.org, svn-src-projects@freebsd.org Subject: svn commit: r345566 - in projects/fuse2: sys/fs/fuse tests/sys/fs/fusefs Message-ID: <201903270258.x2R2w0Mk051957@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201903270258.x2R2w0Mk051957>