Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 27 Jul 2021 17:40:26 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: 7d9771228715 - stable/12 - fusefs: correctly set lock owner during FUSE_SETLK
Message-ID:  <202107271740.16RHeQ7Z046222@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=7d9771228715bf2a32c7db323accecfb7f2f861c

commit 7d9771228715bf2a32c7db323accecfb7f2f861c
Author:     Alan Somers <asomers@FreeBSD.org>
AuthorDate: 2021-05-20 01:10:15 +0000
Commit:     Alan Somers <asomers@FreeBSD.org>
CommitDate: 2021-07-27 17:34:08 +0000

    fusefs: correctly set lock owner during FUSE_SETLK
    
    During FUSE_SETLK, the owner field should uniquely identify the calling
    process.  The fusefs module now sets it to the process's pid.
    Previously, it expected the calling process to set it directly, which
    was wrong.
    
    libfuse also apparently expects the owner field to be set during
    FUSE_GETLK, though I'm not sure why.
    
    PR:             256005
    Reported by:    Agata <chogata@moosefs.pro>
    Reviewed by:    pfg
    Differential Revision: https://reviews.freebsd.org/D30622
    
    (cherry picked from commit 18b19f8c6e04935a63a951afe0e540674bc94455)
---
 sys/fs/fuse/fuse_vnops.c     |  4 ++--
 tests/sys/fs/fusefs/locks.cc | 50 ++++++++++++++++++++++++++------------------
 2 files changed, 32 insertions(+), 22 deletions(-)

diff --git a/sys/fs/fuse/fuse_vnops.c b/sys/fs/fuse/fuse_vnops.c
index 1c1ac7680f0e..9e79a376cccd 100644
--- a/sys/fs/fuse/fuse_vnops.c
+++ b/sys/fs/fuse/fuse_vnops.c
@@ -427,14 +427,14 @@ fuse_vnop_advlock(struct vop_advlock_args *ap)
 	fdisp_make_vp(&fdi, op, vp, td, cred);
 	fli = fdi.indata;
 	fli->fh = fufh->fh_id;
-	fli->owner = fl->l_pid;
+	fli->owner = td->td_proc->p_pid;
 	fli->lk.start = fl->l_start;
 	if (fl->l_len != 0)
 		fli->lk.end = fl->l_start + fl->l_len - 1;
 	else
 		fli->lk.end = INT64_MAX;
 	fli->lk.type = fl->l_type;
-	fli->lk.pid = fl->l_pid;
+	fli->lk.pid = td->td_proc->p_pid;
 
 	err = fdisp_wait_answ(&fdi);
 	fdisp_destroy(&fdi);
diff --git a/tests/sys/fs/fusefs/locks.cc b/tests/sys/fs/fusefs/locks.cc
index 49f495412259..cace779e981a 100644
--- a/tests/sys/fs/fusefs/locks.cc
+++ b/tests/sys/fs/fusefs/locks.cc
@@ -228,7 +228,7 @@ TEST_F(GetlkFallback, local)
 	ASSERT_LE(0, fd) << strerror(errno);
 	fl.l_start = 10;
 	fl.l_len = 1000;
-	fl.l_pid = getpid();
+	fl.l_pid = 0;
 	fl.l_type = F_RDLCK;
 	fl.l_whence = SEEK_SET;
 	fl.l_sysid = 0;
@@ -247,7 +247,7 @@ TEST_F(Getlk, no_locks)
 	uint64_t ino = 42;
 	struct flock fl;
 	int fd;
-	pid_t pid = 1234;
+	pid_t pid = getpid();
 
 	expect_lookup(RELPATH, ino);
 	expect_open(ino, 0, 1);
@@ -256,11 +256,16 @@ TEST_F(Getlk, no_locks)
 			return (in.header.opcode == FUSE_GETLK &&
 				in.header.nodeid == ino &&
 				in.body.getlk.fh == FH &&
+				/*
+				 * Though it seems useless, libfuse expects the
+				 * owner and pid fields to be set during
+				 * FUSE_GETLK.
+				 */
 				in.body.getlk.owner == (uint32_t)pid &&
+				in.body.getlk.lk.pid == (uint64_t)pid &&
 				in.body.getlk.lk.start == 10 &&
 				in.body.getlk.lk.end == 1009 &&
-				in.body.getlk.lk.type == F_RDLCK &&
-				in.body.getlk.lk.pid == (uint64_t)pid);
+				in.body.getlk.lk.type == F_RDLCK);
 		}, Eq(true)),
 		_)
 	).WillOnce(Invoke(ReturnImmediate([=](auto in, auto& out) {
@@ -273,7 +278,7 @@ TEST_F(Getlk, no_locks)
 	ASSERT_LE(0, fd) << strerror(errno);
 	fl.l_start = 10;
 	fl.l_len = 1000;
-	fl.l_pid = pid;
+	fl.l_pid = 0;
 	fl.l_type = F_RDLCK;
 	fl.l_whence = SEEK_SET;
 	fl.l_sysid = 0;
@@ -290,7 +295,7 @@ TEST_F(Getlk, lock_exists)
 	uint64_t ino = 42;
 	struct flock fl;
 	int fd;
-	pid_t pid = 1234;
+	pid_t pid = getpid();
 	pid_t pid2 = 1235;
 
 	expect_lookup(RELPATH, ino);
@@ -300,11 +305,16 @@ TEST_F(Getlk, lock_exists)
 			return (in.header.opcode == FUSE_GETLK &&
 				in.header.nodeid == ino &&
 				in.body.getlk.fh == FH &&
+				/*
+				 * Though it seems useless, libfuse expects the
+				 * owner and pid fields to be set during
+				 * FUSE_GETLK.
+				 */
 				in.body.getlk.owner == (uint32_t)pid &&
+				in.body.getlk.lk.pid == (uint64_t)pid &&
 				in.body.getlk.lk.start == 10 &&
 				in.body.getlk.lk.end == 1009 &&
-				in.body.getlk.lk.type == F_RDLCK &&
-				in.body.getlk.lk.pid == (uint64_t)pid);
+				in.body.getlk.lk.type == F_RDLCK);
 		}, Eq(true)),
 		_)
 	).WillOnce(Invoke(ReturnImmediate([=](auto in __unused, auto& out) {
@@ -319,7 +329,7 @@ TEST_F(Getlk, lock_exists)
 	ASSERT_LE(0, fd) << strerror(errno);
 	fl.l_start = 10;
 	fl.l_len = 1000;
-	fl.l_pid = pid;
+	fl.l_pid = 0;
 	fl.l_type = F_RDLCK;
 	fl.l_whence = SEEK_SET;
 	fl.l_sysid = 0;
@@ -368,7 +378,7 @@ TEST_F(Setlk, clear)
 	uint64_t ino = 42;
 	struct flock fl;
 	int fd;
-	pid_t pid = 1234;
+	pid_t pid = getpid();
 
 	expect_lookup(RELPATH, ino);
 	expect_open(ino, 0, 1);
@@ -378,7 +388,7 @@ TEST_F(Setlk, clear)
 	ASSERT_LE(0, fd) << strerror(errno);
 	fl.l_start = 10;
 	fl.l_len = 1000;
-	fl.l_pid = pid;
+	fl.l_pid = 0;
 	fl.l_type = F_UNLCK;
 	fl.l_whence = SEEK_SET;
 	fl.l_sysid = 0;
@@ -394,7 +404,7 @@ TEST_F(Setlk, set)
 	uint64_t ino = 42;
 	struct flock fl;
 	int fd;
-	pid_t pid = 1234;
+	pid_t pid = getpid();
 
 	expect_lookup(RELPATH, ino);
 	expect_open(ino, 0, 1);
@@ -404,7 +414,7 @@ TEST_F(Setlk, set)
 	ASSERT_LE(0, fd) << strerror(errno);
 	fl.l_start = 10;
 	fl.l_len = 1000;
-	fl.l_pid = pid;
+	fl.l_pid = 0;
 	fl.l_type = F_RDLCK;
 	fl.l_whence = SEEK_SET;
 	fl.l_sysid = 0;
@@ -420,7 +430,7 @@ TEST_F(Setlk, set_eof)
 	uint64_t ino = 42;
 	struct flock fl;
 	int fd;
-	pid_t pid = 1234;
+	pid_t pid = getpid();
 
 	expect_lookup(RELPATH, ino);
 	expect_open(ino, 0, 1);
@@ -430,7 +440,7 @@ TEST_F(Setlk, set_eof)
 	ASSERT_LE(0, fd) << strerror(errno);
 	fl.l_start = 10;
 	fl.l_len = 0;
-	fl.l_pid = pid;
+	fl.l_pid = 0;
 	fl.l_type = F_RDLCK;
 	fl.l_whence = SEEK_SET;
 	fl.l_sysid = 0;
@@ -446,7 +456,7 @@ TEST_F(Setlk, eagain)
 	uint64_t ino = 42;
 	struct flock fl;
 	int fd;
-	pid_t pid = 1234;
+	pid_t pid = getpid();
 
 	expect_lookup(RELPATH, ino);
 	expect_open(ino, 0, 1);
@@ -456,7 +466,7 @@ TEST_F(Setlk, eagain)
 	ASSERT_LE(0, fd) << strerror(errno);
 	fl.l_start = 10;
 	fl.l_len = 1000;
-	fl.l_pid = pid;
+	fl.l_pid = 0;
 	fl.l_type = F_RDLCK;
 	fl.l_whence = SEEK_SET;
 	fl.l_sysid = 0;
@@ -484,7 +494,7 @@ TEST_F(SetlkwFallback, local)
 	ASSERT_LE(0, fd) << strerror(errno);
 	fl.l_start = 10;
 	fl.l_len = 1000;
-	fl.l_pid = getpid();
+	fl.l_pid = 0;
 	fl.l_type = F_RDLCK;
 	fl.l_whence = SEEK_SET;
 	fl.l_sysid = 0;
@@ -504,7 +514,7 @@ TEST_F(Setlkw, set)
 	uint64_t ino = 42;
 	struct flock fl;
 	int fd;
-	pid_t pid = 1234;
+	pid_t pid = getpid();
 
 	expect_lookup(RELPATH, ino);
 	expect_open(ino, 0, 1);
@@ -514,7 +524,7 @@ TEST_F(Setlkw, set)
 	ASSERT_LE(0, fd) << strerror(errno);
 	fl.l_start = 10;
 	fl.l_len = 1000;
-	fl.l_pid = pid;
+	fl.l_pid = 0;
 	fl.l_type = F_RDLCK;
 	fl.l_whence = SEEK_SET;
 	fl.l_sysid = 0;



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