Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 8 Apr 2021 21:34:32 GMT
From:      Alan Somers <asomers@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org
Subject:   git: 9a9c9e744b51 - stable/12 - fusefs: fix two bugs regarding fcntl file locks
Message-ID:  <202104082134.138LYWL6015513@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch stable/12 has been updated by asomers:

URL: https://cgit.FreeBSD.org/src/commit/?id=9a9c9e744b51e00f3446afb922500e40845cddfa

commit 9a9c9e744b51e00f3446afb922500e40845cddfa
Author:     Alan Somers <asomers@FreeBSD.org>
AuthorDate: 2021-03-18 20:27:27 +0000
Commit:     Alan Somers <asomers@FreeBSD.org>
CommitDate: 2021-04-08 21:33:55 +0000

    fusefs: fix two bugs regarding fcntl file locks
    
    1) F_SETLKW (blocking) operations would be sent to the FUSE server as
       F_SETLK (non-blocking).
    
    2) Release operations, F_SETLK with lk_type = F_UNLCK, would simply
       return EINVAL.
    
    PR:             253500
    Reported by:    John Millikin <jmillikin@gmail.com>
    
    (cherry picked from commit 929acdb19acb67cc0e6ee5439df98e28a84d4772)
    
    fusefs: fix a dead store in fuse_vnop_advlock
    
    kevans actually caught this in the original review and I fixed it, but
    then I committed an older copy of the branch.  Whoops.
    
    Reported by:    kevans
    Differential Revision:  https://reviews.freebsd.org/D29031
    
    (cherry picked from commit 9c5aac8f2e84ca4bbdf82514302c08c0453ec59b)
---
 sys/fs/fuse/fuse_vnops.c       |  9 ++++++---
 tests/sys/fs/fusefs/flush.cc   | 12 ++++++++++-
 tests/sys/fs/fusefs/locks.cc   | 45 +++++++++++++++++++++++++++++++++++++++++-
 tests/sys/fs/fusefs/release.cc | 12 ++++++++++-
 4 files changed, 72 insertions(+), 6 deletions(-)

diff --git a/sys/fs/fuse/fuse_vnops.c b/sys/fs/fuse/fuse_vnops.c
index eb99e1cc09c0..d30fc32d1dc5 100644
--- a/sys/fs/fuse/fuse_vnops.c
+++ b/sys/fs/fuse/fuse_vnops.c
@@ -412,10 +412,13 @@ fuse_vnop_advlock(struct vop_advlock_args *ap)
 		op = FUSE_GETLK;
 		break;
 	case F_SETLK:
-		op = FUSE_SETLK;
+		if (flags & F_WAIT)
+			op = FUSE_SETLKW;
+		else
+			op = FUSE_SETLK;
 		break;
-	case F_SETLKW:
-		op = FUSE_SETLKW;
+	case F_UNLCK:
+		op = FUSE_SETLK;
 		break;
 	default:
 		return EINVAL;
diff --git a/tests/sys/fs/fusefs/flush.cc b/tests/sys/fs/fusefs/flush.cc
index 4d5a87bd66d5..d31a9fdaa386 100644
--- a/tests/sys/fs/fusefs/flush.cc
+++ b/tests/sys/fs/fusefs/flush.cc
@@ -210,6 +210,16 @@ TEST_F(FlushWithLocks, unlock_on_close)
 		ResultOf([=](auto in) {
 			return (in.header.opcode == FUSE_SETLK &&
 				in.header.nodeid == ino &&
+				in.body.setlk.lk.type == F_RDLCK &&
+				in.body.setlk.fh == FH);
+		}, Eq(true)),
+		_)
+	).WillOnce(Invoke(ReturnErrno(0)));
+	EXPECT_CALL(*m_mock, process(
+		ResultOf([=](auto in) {
+			return (in.header.opcode == FUSE_SETLK &&
+				in.header.nodeid == ino &&
+				in.body.setlk.lk.type == F_UNLCK &&
 				in.body.setlk.fh == FH);
 		}, Eq(true)),
 		_)
@@ -224,7 +234,7 @@ TEST_F(FlushWithLocks, unlock_on_close)
 	fl.l_type = F_RDLCK;
 	fl.l_whence = SEEK_SET;
 	fl.l_sysid = 0;
-	ASSERT_NE(-1, fcntl(fd, F_SETLKW, &fl)) << strerror(errno);
+	ASSERT_NE(-1, fcntl(fd, F_SETLK, &fl)) << strerror(errno);
 
 	fd2 = open(FULLPATH, O_WRONLY);
 	ASSERT_LE(0, fd2) << strerror(errno);
diff --git a/tests/sys/fs/fusefs/locks.cc b/tests/sys/fs/fusefs/locks.cc
index f3f1d42637d9..49f495412259 100644
--- a/tests/sys/fs/fusefs/locks.cc
+++ b/tests/sys/fs/fusefs/locks.cc
@@ -72,6 +72,23 @@ void expect_setlk(uint64_t ino, pid_t pid, uint64_t start, uint64_t end,
 			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 == start &&
+				in.body.setlk.lk.end == end &&
+				in.body.setlk.lk.type == type &&
+				in.body.setlk.lk.pid == (uint64_t)pid);
+		}, Eq(true)),
+		_)
+	).WillOnce(Invoke(ReturnErrno(err)));
+}
+void expect_setlkw(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_SETLKW &&
+				in.header.nodeid == ino &&
+				in.body.setlkw.fh == FH &&
 				in.body.setlkw.owner == (uint32_t)pid &&
 				in.body.setlkw.lk.start == start &&
 				in.body.setlkw.lk.end == end &&
@@ -343,6 +360,32 @@ TEST_F(SetlkFallback, local)
 	leak(fd);
 }
 
+/* Clear a lock with FUSE_SETLK */
+TEST_F(Setlk, clear)
+{
+	const char FULLPATH[] = "mountpoint/some_file.txt";
+	const char RELPATH[] = "some_file.txt";
+	uint64_t ino = 42;
+	struct flock fl;
+	int fd;
+	pid_t pid = 1234;
+
+	expect_lookup(RELPATH, ino);
+	expect_open(ino, 0, 1);
+	expect_setlk(ino, pid, 10, 1009, F_UNLCK, 0);
+
+	fd = open(FULLPATH, O_RDWR);
+	ASSERT_LE(0, fd) << strerror(errno);
+	fl.l_start = 10;
+	fl.l_len = 1000;
+	fl.l_pid = pid;
+	fl.l_type = F_UNLCK;
+	fl.l_whence = SEEK_SET;
+	fl.l_sysid = 0;
+	ASSERT_NE(-1, fcntl(fd, F_SETLK, &fl)) << strerror(errno);
+	leak(fd);
+}
+
 /* Set a new lock with FUSE_SETLK */
 TEST_F(Setlk, set)
 {
@@ -465,7 +508,7 @@ TEST_F(Setlkw, set)
 
 	expect_lookup(RELPATH, ino);
 	expect_open(ino, 0, 1);
-	expect_setlk(ino, pid, 10, 1009, F_RDLCK, 0);
+	expect_setlkw(ino, pid, 10, 1009, F_RDLCK, 0);
 
 	fd = open(FULLPATH, O_RDWR);
 	ASSERT_LE(0, fd) << strerror(errno);
diff --git a/tests/sys/fs/fusefs/release.cc b/tests/sys/fs/fusefs/release.cc
index 3caf8589352d..daa3ce39f573 100644
--- a/tests/sys/fs/fusefs/release.cc
+++ b/tests/sys/fs/fusefs/release.cc
@@ -206,6 +206,16 @@ TEST_F(ReleaseWithLocks, unlock_on_close)
 		ResultOf([=](auto in) {
 			return (in.header.opcode == FUSE_SETLK &&
 				in.header.nodeid == ino &&
+				in.body.setlk.lk.type == F_RDLCK &&
+				in.body.setlk.fh == FH);
+		}, Eq(true)),
+		_)
+	).WillOnce(Invoke(ReturnErrno(0)));
+	EXPECT_CALL(*m_mock, process(
+		ResultOf([=](auto in) {
+			return (in.header.opcode == FUSE_SETLK &&
+				in.header.nodeid == ino &&
+				in.body.setlk.lk.type == F_UNLCK &&
 				in.body.setlk.fh == FH);
 		}, Eq(true)),
 		_)
@@ -221,7 +231,7 @@ TEST_F(ReleaseWithLocks, unlock_on_close)
 	fl.l_type = F_RDLCK;
 	fl.l_whence = SEEK_SET;
 	fl.l_sysid = 0;
-	ASSERT_NE(-1, fcntl(fd, F_SETLKW, &fl)) << strerror(errno);
+	ASSERT_NE(-1, fcntl(fd, F_SETLK, &fl)) << strerror(errno);
 
 	ASSERT_EQ(0, close(fd)) << strerror(errno);
 }



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202104082134.138LYWL6015513>