From 5df1cc70442c7b2b575d20f869980a861ecda214 Mon Sep 17 00:00:00 2001 From: asomers Date: Tue, 28 May 2019 00:03:46 +0000 Subject: [PATCH] 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 --- sys/fs/fuse/fuse_vnops.c | 4 + tests/sys/fs/fusefs/locks.cc | 186 ++++++++++++++++++++++++----------- 2 files changed, 135 insertions(+), 55 deletions(-) diff --git a/sys/fs/fuse/fuse_vnops.c b/sys/fs/fuse/fuse_vnops.c index 66a05d21d6e..451fb2a164f 100644 --- a/sys/fs/fuse/fuse_vnops.c +++ b/sys/fs/fuse/fuse_vnops.c @@ -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; @@ -398,6 +399,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); if (err) diff --git a/tests/sys/fs/fusefs/locks.cc b/tests/sys/fs/fusefs/locks.cc index 6d4c3c5d8bc..dc6c3e41770 100644 --- a/tests/sys/fs/fusefs/locks.cc +++ b/tests/sys/fs/fusefs/locks.cc @@ -29,6 +29,7 @@ */ extern "C" { +#include #include } @@ -59,12 +60,135 @@ 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 @@ -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); -- 2.45.0