From owner-svn-src-projects@freebsd.org Tue May 28 00:03:47 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 6988015AF09F for ; Tue, 28 May 2019 00:03:47 +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 08A8A8C64A; Tue, 28 May 2019 00:03:47 +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 D15DBA5C6; Tue, 28 May 2019 00:03:46 +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 x4S03k77014683; Tue, 28 May 2019 00:03:46 GMT (envelope-from asomers@FreeBSD.org) Received: (from asomers@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id x4S03kcv014681; Tue, 28 May 2019 00:03:46 GMT (envelope-from asomers@FreeBSD.org) Message-Id: <201905280003.x4S03kcv014681@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: asomers set sender to asomers@FreeBSD.org using -f From: Alan Somers Date: Tue, 28 May 2019 00:03:46 +0000 (UTC) To: src-committers@freebsd.org, svn-src-projects@freebsd.org Subject: svn commit: r348317 - 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: 348317 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Rspamd-Queue-Id: 08A8A8C64A X-Spamd-Bar: -- Authentication-Results: mx1.freebsd.org X-Spamd-Result: default: False [-2.98 / 15.00]; local_wl_from(0.00)[FreeBSD.org]; NEURAL_HAM_MEDIUM(-1.00)[-1.000,0]; NEURAL_HAM_LONG(-1.00)[-1.000,0]; NEURAL_HAM_SHORT(-0.98)[-0.984,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: Tue, 28 May 2019 00:03:47 -0000 Author: asomers Date: Tue May 28 00:03:46 2019 New Revision: 348317 URL: https://svnweb.freebsd.org/changeset/base/348317 Log: fusefs: flock(2) locks must be implemented in-kernel If a FUSE file system sets the FUSE_POSIX_LOCKS flag then it can support fcntl(2)-style locks directly. However, the protocol does not adequately support flock(2)-style locks until revision 7.17. They must be implemented locally in-kernel instead. This unfortunately breaks the interoperability of fcntl(2) and flock(2) locks for file systems that support the former. C'est la vie. Prior to this commit flock(2) would get sent to the server as a fcntl(2)-style lock with the lock owner field set to stack garbage. Sponsored by: The FreeBSD Foundation Modified: projects/fuse2/sys/fs/fuse/fuse_vnops.c projects/fuse2/tests/sys/fs/fusefs/locks.cc Modified: projects/fuse2/sys/fs/fuse/fuse_vnops.c ============================================================================== --- projects/fuse2/sys/fs/fuse/fuse_vnops.c Mon May 27 23:25:19 2019 (r348316) +++ projects/fuse2/sys/fs/fuse/fuse_vnops.c Tue May 28 00:03:46 2019 (r348317) @@ -389,6 +389,7 @@ fuse_vnop_advlock(struct vop_advlock_args *ap) struct fuse_lk_out *flo; enum fuse_opcode op; int dataflags, err; + int flags = ap->a_flags; dataflags = fuse_get_mpdata(vnode_mount(vp))->dataflags; @@ -397,6 +398,9 @@ fuse_vnop_advlock(struct vop_advlock_args *ap) } if (!(dataflags & FSESS_POSIX_LOCKS)) + return vop_stdadvlock(ap); + /* FUSE doesn't properly support flock until protocol 7.17 */ + if (flags & F_FLOCK) return vop_stdadvlock(ap); err = fuse_filehandle_get_anyflags(vp, &fufh, cred, pid); Modified: projects/fuse2/tests/sys/fs/fusefs/locks.cc ============================================================================== --- projects/fuse2/tests/sys/fs/fusefs/locks.cc Mon May 27 23:25:19 2019 (r348316) +++ projects/fuse2/tests/sys/fs/fusefs/locks.cc Tue May 28 00:03:46 2019 (r348317) @@ -29,6 +29,7 @@ */ extern "C" { +#include #include } @@ -59,14 +60,137 @@ class Locks: public Fallback { } }; +class Fcntl: public Locks { +public: +void expect_setlk(uint64_t ino, pid_t pid, uint64_t start, uint64_t end, + uint32_t type, int err) +{ + EXPECT_CALL(*m_mock, process( + ResultOf([=](auto in) { + return (in.header.opcode == FUSE_SETLK && + in.header.nodeid == ino && + in.body.setlk.fh == FH && + in.body.setlkw.owner == (uint32_t)pid && + in.body.setlkw.lk.start == start && + in.body.setlkw.lk.end == end && + in.body.setlkw.lk.type == type && + in.body.setlkw.lk.pid == (uint64_t)pid); + }, Eq(true)), + _) + ).WillOnce(Invoke(ReturnErrno(err))); +} +}; + +class Flock: public Locks { +public: +void expect_setlk(uint64_t ino, uint32_t type, int err) +{ + EXPECT_CALL(*m_mock, process( + ResultOf([=](auto in) { + return (in.header.opcode == FUSE_SETLK && + in.header.nodeid == ino && + in.body.setlk.fh == FH && + /* + * The owner should be set to the address of + * the vnode. That's hard to verify. + */ + /* in.body.setlk.owner == ??? && */ + in.body.setlk.lk.type == type); + }, Eq(true)), + _) + ).WillOnce(Invoke(ReturnErrno(err))); +} +}; + +class FlockFallback: public Fallback {}; class GetlkFallback: public Fallback {}; -class Getlk: public Locks {}; +class Getlk: public Fcntl {}; class SetlkFallback: public Fallback {}; -class Setlk: public Locks {}; +class Setlk: public Fcntl {}; class SetlkwFallback: public Fallback {}; -class Setlkw: public Locks {}; +class Setlkw: public Fcntl {}; /* + * If the fuse filesystem does not support flock locks, then the kernel should + * fall back to local locks. + */ +TEST_F(FlockFallback, local) +{ + const char FULLPATH[] = "mountpoint/some_file.txt"; + const char RELPATH[] = "some_file.txt"; + uint64_t ino = 42; + int fd; + + expect_lookup(RELPATH, ino); + expect_open(ino, 0, 1); + + fd = open(FULLPATH, O_RDWR); + ASSERT_LE(0, fd) << strerror(errno); + ASSERT_EQ(0, flock(fd, LOCK_EX)) << strerror(errno); + /* Deliberately leak fd. close(2) will be tested in release.cc */ +} + +/* + * Even if the fuse file system supports POSIX locks, we must implement flock + * locks locally until protocol 7.17. Protocol 7.9 added partial buggy support + * but we won't implement that. + */ +TEST_F(Flock, local) +{ + const char FULLPATH[] = "mountpoint/some_file.txt"; + const char RELPATH[] = "some_file.txt"; + uint64_t ino = 42; + int fd; + + expect_lookup(RELPATH, ino); + expect_open(ino, 0, 1); + + fd = open(FULLPATH, O_RDWR); + ASSERT_LE(0, fd) << strerror(errno); + ASSERT_EQ(0, flock(fd, LOCK_EX)) << strerror(errno); + /* Deliberately leak fd. close(2) will be tested in release.cc */ +} + +/* Set a new flock lock with FUSE_SETLK */ +/* TODO: enable after upgrading to protocol 7.17 */ +TEST_F(Flock, DISABLED_set) +{ + const char FULLPATH[] = "mountpoint/some_file.txt"; + const char RELPATH[] = "some_file.txt"; + uint64_t ino = 42; + int fd; + + expect_lookup(RELPATH, ino); + expect_open(ino, 0, 1); + expect_setlk(ino, F_WRLCK, 0); + + fd = open(FULLPATH, O_RDWR); + ASSERT_LE(0, fd) << strerror(errno); + ASSERT_EQ(0, flock(fd, LOCK_EX)) << strerror(errno); + /* Deliberately leak fd. close(2) will be tested in release.cc */ +} + +/* Fail to set a flock lock in non-blocking mode */ +/* TODO: enable after upgrading to protocol 7.17 */ +TEST_F(Flock, DISABLED_eagain) +{ + const char FULLPATH[] = "mountpoint/some_file.txt"; + const char RELPATH[] = "some_file.txt"; + uint64_t ino = 42; + int fd; + + expect_lookup(RELPATH, ino); + expect_open(ino, 0, 1); + expect_setlk(ino, F_WRLCK, EAGAIN); + + fd = open(FULLPATH, O_RDWR); + ASSERT_LE(0, fd) << strerror(errno); + ASSERT_NE(0, flock(fd, LOCK_EX | LOCK_NB)); + ASSERT_EQ(EAGAIN, errno); + /* Deliberately leak fd. close(2) will be tested in release.cc */ +} + +/* * If the fuse filesystem does not support posix file locks, then the kernel * should fall back to local locks. */ @@ -229,19 +353,7 @@ TEST_F(Setlk, set) expect_lookup(RELPATH, ino); expect_open(ino, 0, 1); - EXPECT_CALL(*m_mock, process( - ResultOf([=](auto in) { - return (in.header.opcode == FUSE_SETLK && - in.header.nodeid == ino && - in.body.setlk.fh == FH && - in.body.setlk.owner == (uint32_t)pid && - in.body.setlk.lk.start == 10 && - in.body.setlk.lk.end == 1009 && - in.body.setlk.lk.type == F_RDLCK && - in.body.setlk.lk.pid == (uint64_t)pid); - }, Eq(true)), - _) - ).WillOnce(Invoke(ReturnErrno(0))); + expect_setlk(ino, pid, 10, 1009, F_RDLCK, 0); fd = open(FULLPATH, O_RDWR); ASSERT_LE(0, fd) << strerror(errno); @@ -267,19 +379,7 @@ TEST_F(Setlk, set_eof) expect_lookup(RELPATH, ino); expect_open(ino, 0, 1); - EXPECT_CALL(*m_mock, process( - ResultOf([=](auto in) { - return (in.header.opcode == FUSE_SETLK && - in.header.nodeid == ino && - in.body.setlk.fh == FH && - in.body.setlk.owner == (uint32_t)pid && - in.body.setlk.lk.start == 10 && - in.body.setlk.lk.end == OFFSET_MAX && - in.body.setlk.lk.type == F_RDLCK && - in.body.setlk.lk.pid == (uint64_t)pid); - }, Eq(true)), - _) - ).WillOnce(Invoke(ReturnErrno(0))); + expect_setlk(ino, pid, 10, OFFSET_MAX, F_RDLCK, 0); fd = open(FULLPATH, O_RDWR); ASSERT_LE(0, fd) << strerror(errno); @@ -305,19 +405,7 @@ TEST_F(Setlk, eagain) expect_lookup(RELPATH, ino); expect_open(ino, 0, 1); - EXPECT_CALL(*m_mock, process( - ResultOf([=](auto in) { - return (in.header.opcode == FUSE_SETLK && - in.header.nodeid == ino && - in.body.setlk.fh == FH && - in.body.setlk.owner == (uint32_t)pid && - in.body.setlk.lk.start == 10 && - in.body.setlk.lk.end == 1009 && - in.body.setlk.lk.type == F_RDLCK && - in.body.setlk.lk.pid == (uint64_t)pid); - }, Eq(true)), - _) - ).WillOnce(Invoke(ReturnErrno(EAGAIN))); + expect_setlk(ino, pid, 10, 1009, F_RDLCK, EAGAIN); fd = open(FULLPATH, O_RDWR); ASSERT_LE(0, fd) << strerror(errno); @@ -375,19 +463,7 @@ TEST_F(Setlkw, set) expect_lookup(RELPATH, ino); expect_open(ino, 0, 1); - EXPECT_CALL(*m_mock, process( - ResultOf([=](auto in) { - return (in.header.opcode == FUSE_SETLK && - in.header.nodeid == ino && - in.body.setlkw.fh == FH && - in.body.setlkw.owner == (uint32_t)pid && - in.body.setlkw.lk.start == 10 && - in.body.setlkw.lk.end == 1009 && - in.body.setlkw.lk.type == F_RDLCK && - in.body.setlkw.lk.pid == (uint64_t)pid); - }, Eq(true)), - _) - ).WillOnce(Invoke(ReturnErrno(0))); + expect_setlk(ino, pid, 10, 1009, F_RDLCK, 0); fd = open(FULLPATH, O_RDWR); ASSERT_LE(0, fd) << strerror(errno);