[PATCH 0/2] v3: F_OFD_GETLK extension to read lock info

[PATCH 0/2] v3: F_OFD_GETLK extension to read lock info

[Stas Sergeev]

This extension allows to use F_UNLCK on query, which currently returns
EINVAL. Instead it can be used to query the locks on a particular fd -
something that is not currently possible. The basic idea is that on
F_OFD_GETLK, F_UNLCK would "conflict" with (or query) any types of the
lock on the same fd, and ignore any locks on other fds.

Use-cases:

1. CRIU-alike scenario when you want to read the locking info from an
fd for the later reconstruction. This can now be done by setting
l_start and l_len to 0 to cover entire file range, and do F_OFD_GETLK.
In the loop you need to advance l_start past the returned lock ranges,
to eventually collect all locked ranges.

2. Implementing the lock checking/enforcing policy.
Say you want to implement an "auditor" module in your program,
that checks that the I/O is done only after the proper locking is
applied on a file region. In this case you need to know if the
particular region is locked on that fd, and if so - with what type
of the lock. If you would do that currently (without this extension)
then you can only check for the write locks, and for that you need to
probe the lock on your fd and then open the same file via another fd and
probe there. That way you can identify the write lock on a particular
fd, but such trick is non-atomic and complex. As for finding out the
read lock on a particular fd - impossible.
This extension allows to do such queries without any extra efforts.

3. Implementing the mandatory locking policy.
Suppose you want to make a policy where the write lock inhibits any
unlocked readers and writers. Currently you need to check if the
write lock is present on some other fd, and if it is not there - allow
the I/O operation. But because the write lock can appear at any moment,
you need to do that under some global lock, which can be released only
when the I/O operation is finished.
With the proposed extension you can instead just check the write lock
on your own fd first, and if it is there - allow the I/O operation on
that fd without using any global lock. Only if there is no write lock
on this fd, then you need to take global lock and check for a write
lock on other fds.


The second patch adds a test-case for OFD locks.
It tests both the generic things and the proposed extension.


The third patch is a proposed man page update for fcntl(2)
(not for the linux source tree)


Changes in v3:
- Move selftest to selftests/filelock

Changes in v2:
- Dropped the l_pid extension patch and updated test-case accordingly.

Stas Sergeev (2):
  fs/locks: F_UNLCK extension for F_OFD_GETLK
  selftests: add OFD lock tests

 fs/locks.c                                  |  23 +++-
 tools/testing/selftests/filelock/Makefile   |   5 +
 tools/testing/selftests/filelock/ofdlocks.c | 132 ++++++++++++++++++++
 3 files changed, 157 insertions(+), 3 deletions(-)
 create mode 100644 tools/testing/selftests/filelock/Makefile
 create mode 100644 tools/testing/selftests/filelock/ofdlocks.c

CC: Jeff Layton <jlayton@kernel.org>
CC: Chuck Lever <chuck.lever@oracle.com>
CC: Alexander Viro <viro@zeniv.linux.org.uk>
CC: Christian Brauner <brauner@kernel.org>
CC: linux-fsdevel@vger.kernel.org
CC: linux-kernel@vger.kernel.org
CC: Shuah Khan <shuah@kernel.org>
CC: linux-kselftest@vger.kernel.org
CC: linux-api@vger.kernel.org

-- 
2.39.2

[PATCH 1/2] fs/locks: F_UNLCK extension for F_OFD_GETLK

[Stas Sergeev]

Currently F_UNLCK with F_OFD_GETLK returns -EINVAL.
The proposed extension allows to use it for getting the lock
information from the particular fd.

Signed-off-by: Stas Sergeev <stsp2@yandex.ru>

CC: Jeff Layton <jlayton@kernel.org>
CC: Chuck Lever <chuck.lever@oracle.com>
CC: Alexander Viro <viro@zeniv.linux.org.uk>
CC: Christian Brauner <brauner@kernel.org>
CC: linux-fsdevel@vger.kernel.org
CC: linux-kernel@vger.kernel.org
CC: Shuah Khan <shuah@kernel.org>
CC: linux-kselftest@vger.kernel.org
CC: linux-api@vger.kernel.org

---
 fs/locks.c | 23 ++++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

diff --git a/fs/locks.c b/fs/locks.c
index df8b26a42524..210766007e63 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -868,6 +868,21 @@ static bool posix_locks_conflict(struct file_lock *caller_fl,
 	return locks_conflict(caller_fl, sys_fl);
 }
 
+/* Determine if lock sys_fl blocks lock caller_fl. Used on xx_GETLK
+ * path so checks for additional GETLK-specific things like F_UNLCK.
+ */
+static bool posix_test_locks_conflict(struct file_lock *caller_fl,
+				      struct file_lock *sys_fl)
+{
+	/* F_UNLCK checks any locks on the same fd. */
+	if (caller_fl->fl_type == F_UNLCK) {
+		if (!posix_same_owner(caller_fl, sys_fl))
+			return false;
+		return locks_overlap(caller_fl, sys_fl);
+	}
+	return posix_locks_conflict(caller_fl, sys_fl);
+}
+
 /* Determine if lock sys_fl blocks lock caller_fl. FLOCK specific
  * checking before calling the locks_conflict().
  */
@@ -901,7 +916,7 @@ posix_test_lock(struct file *filp, struct file_lock *fl)
 retry:
 	spin_lock(&ctx->flc_lock);
 	list_for_each_entry(cfl, &ctx->flc_posix, fl_list) {
-		if (!posix_locks_conflict(fl, cfl))
+		if (!posix_test_locks_conflict(fl, cfl))
 			continue;
 		if (cfl->fl_lmops && cfl->fl_lmops->lm_lock_expirable
 			&& (*cfl->fl_lmops->lm_lock_expirable)(cfl)) {
@@ -2207,7 +2222,8 @@ int fcntl_getlk(struct file *filp, unsigned int cmd, struct flock *flock)
 	if (fl == NULL)
 		return -ENOMEM;
 	error = -EINVAL;
-	if (flock->l_type != F_RDLCK && flock->l_type != F_WRLCK)
+	if (cmd != F_OFD_GETLK && flock->l_type != F_RDLCK
+			&& flock->l_type != F_WRLCK)
 		goto out;
 
 	error = flock_to_posix_lock(filp, fl, flock);
@@ -2414,7 +2430,8 @@ int fcntl_getlk64(struct file *filp, unsigned int cmd, struct flock64 *flock)
 		return -ENOMEM;
 
 	error = -EINVAL;
-	if (flock->l_type != F_RDLCK && flock->l_type != F_WRLCK)
+	if (cmd != F_OFD_GETLK && flock->l_type != F_RDLCK
+			&& flock->l_type != F_WRLCK)
 		goto out;
 
 	error = flock64_to_posix_lock(filp, fl, flock);
-- 
2.39.2

[PATCH 2/2] selftests: add OFD lock tests

[Stas Sergeev]

Test the basic locking stuff on 2 fds: multiple read locks,
conflicts between read and write locks, use of len==0 for queries.
Also tests for F_UNLCK F_OFD_GETLK extension.

Signed-off-by: Stas Sergeev <stsp2@yandex.ru>

CC: Jeff Layton <jlayton@kernel.org>
CC: Chuck Lever <chuck.lever@oracle.com>
CC: Alexander Viro <viro@zeniv.linux.org.uk>
CC: Christian Brauner <brauner@kernel.org>
CC: linux-fsdevel@vger.kernel.org
CC: linux-kernel@vger.kernel.org
CC: Shuah Khan <shuah@kernel.org>
CC: linux-kselftest@vger.kernel.org
CC: linux-api@vger.kernel.org

---
 tools/testing/selftests/filelock/Makefile   |   5 +
 tools/testing/selftests/filelock/ofdlocks.c | 132 ++++++++++++++++++++
 2 files changed, 137 insertions(+)
 create mode 100644 tools/testing/selftests/filelock/Makefile
 create mode 100644 tools/testing/selftests/filelock/ofdlocks.c

diff --git a/tools/testing/selftests/filelock/Makefile b/tools/testing/selftests/filelock/Makefile
new file mode 100644
index 000000000000..478e82f8b464
--- /dev/null
+++ b/tools/testing/selftests/filelock/Makefile
@@ -0,0 +1,5 @@
+# SPDX-License-Identifier: GPL-2.0
+
+TEST_GEN_PROGS := ofdlocks
+
+include ../lib.mk
diff --git a/tools/testing/selftests/filelock/ofdlocks.c b/tools/testing/selftests/filelock/ofdlocks.c
new file mode 100644
index 000000000000..1ccb2b9b5ead
--- /dev/null
+++ b/tools/testing/selftests/filelock/ofdlocks.c
@@ -0,0 +1,132 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#define _GNU_SOURCE
+#include <fcntl.h>
+#include <assert.h>
+#include <stdio.h>
+#include <unistd.h>
+#include <string.h>
+#include "../kselftest.h"
+
+static int lock_set(int fd, struct flock *fl)
+{
+	int ret;
+
+	fl->l_pid = 0;		// needed for OFD locks
+	fl->l_whence = SEEK_SET;
+	ret = fcntl(fd, F_OFD_SETLK, fl);
+	if (ret)
+		perror("fcntl()");
+	return ret;
+}
+
+static int lock_get(int fd, struct flock *fl)
+{
+	int ret;
+
+	fl->l_pid = 0;		// needed for OFD locks
+	fl->l_whence = SEEK_SET;
+	ret = fcntl(fd, F_OFD_GETLK, fl);
+	if (ret)
+		perror("fcntl()");
+	return ret;
+}
+
+int main(void)
+{
+	int rc;
+	struct flock fl, fl2;
+	int fd = open("/tmp/aa", O_RDWR | O_CREAT | O_EXCL, 0600);
+	int fd2 = open("/tmp/aa", O_RDONLY);
+
+	unlink("aa");
+	assert(fd != -1);
+	assert(fd2 != -1);
+	ksft_print_msg("[INFO] opened fds %i %i\n", fd, fd2);
+
+	/* Set some read lock */
+	fl.l_type = F_RDLCK;
+	fl.l_start = 5;
+	fl.l_len = 3;
+	rc = lock_set(fd, &fl);
+	if (rc == 0) {
+		ksft_print_msg
+		    ("[SUCCESS] set OFD read lock on first fd\n");
+	} else {
+		ksft_print_msg("[FAIL] to set OFD read lock on first fd\n");
+		return -1;
+	}
+	/* Make sure read locks do not conflict on different fds. */
+	fl.l_type = F_RDLCK;
+	fl.l_start = 5;
+	fl.l_len = 1;
+	rc = lock_get(fd2, &fl);
+	if (rc != 0)
+		return -1;
+	if (fl.l_type != F_UNLCK) {
+		ksft_print_msg("[FAIL] read locks conflicted\n");
+		return -1;
+	}
+	/* Make sure read/write locks do conflict on different fds. */
+	fl.l_type = F_WRLCK;
+	fl.l_start = 5;
+	fl.l_len = 1;
+	rc = lock_get(fd2, &fl);
+	if (rc != 0)
+		return -1;
+	if (fl.l_type != F_UNLCK) {
+		ksft_print_msg
+		    ("[SUCCESS] read and write locks conflicted\n");
+	} else {
+		ksft_print_msg
+		    ("[SUCCESS] read and write locks not conflicted\n");
+		return -1;
+	}
+	/* Get info about the lock on first fd. */
+	fl.l_type = F_UNLCK;
+	fl.l_start = 5;
+	fl.l_len = 1;
+	rc = lock_get(fd, &fl);
+	if (rc != 0) {
+		ksft_print_msg
+		    ("[FAIL] F_OFD_GETLK with F_UNLCK not supported\n");
+		return -1;
+	}
+	if (fl.l_type != F_UNLCK) {
+		ksft_print_msg
+		    ("[SUCCESS] F_UNLCK test returns: locked, type %i pid %i len %zi\n",
+		     fl.l_type, fl.l_pid, fl.l_len);
+	} else {
+		ksft_print_msg
+		    ("[FAIL] F_OFD_GETLK with F_UNLCK did not return lock info\n");
+		return -1;
+	}
+	/* Try the same but by locking everything by len==0. */
+	fl2.l_type = F_UNLCK;
+	fl2.l_start = 0;
+	fl2.l_len = 0;
+	rc = lock_get(fd, &fl2);
+	if (rc != 0) {
+		ksft_print_msg
+		    ("[FAIL] F_OFD_GETLK with F_UNLCK not supported\n");
+		return -1;
+	}
+	if (memcmp(&fl, &fl2, sizeof(fl))) {
+		ksft_print_msg
+		    ("[FAIL] F_UNLCK test returns: locked, type %i pid %i len %zi\n",
+		     fl.l_type, fl.l_pid, fl.l_len);
+		return -1;
+	}
+	ksft_print_msg("[SUCCESS] F_UNLCK with len==0 returned the same\n");
+	/* Get info about the lock on second fd - no locks on it. */
+	fl.l_type = F_UNLCK;
+	fl.l_start = 0;
+	fl.l_len = 0;
+	lock_get(fd2, &fl);
+	if (fl.l_type != F_UNLCK) {
+		ksft_print_msg
+		    ("[FAIL] F_OFD_GETLK with F_UNLCK return lock info from another fd\n");
+		return -1;
+	}
+	return 0;
+}
-- 
2.39.2

[PATCH] fcntl.2: document F_UNLCK F_OFD_GETLK extension

[Stas Sergeev]

F_UNLCK has the special meaning when used as a lock type on input.
It returns the information about any lock found in the specified
region on that particular file descriptor. Locks on other file
descriptors are ignored by F_UNLCK.

Signed-off-by: Stas Sergeev <stsp2@yandex.ru>

CC: Jeff Layton <jlayton@kernel.org>
CC: Chuck Lever <chuck.lever@oracle.com>
CC: Alexander Viro <viro@zeniv.linux.org.uk>
CC: Christian Brauner <brauner@kernel.org>
CC: linux-fsdevel@vger.kernel.org
CC: linux-kernel@vger.kernel.org
CC: Shuah Khan <shuah@kernel.org>
CC: linux-kselftest@vger.kernel.org
CC: linux-api@vger.kernel.org

---
 man2/fcntl.2 | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/man2/fcntl.2 b/man2/fcntl.2
index 7b5604e3a..e3e3e7b8c 100644
--- a/man2/fcntl.2
+++ b/man2/fcntl.2
@@ -604,6 +604,13 @@ then details about one of these locks are returned via
 .IR lock ,
 as described above for
 .BR F_GETLK .
+.B F_UNLCK
+has the special meaning when put into
+.I l_type
+as an input. It returns the information about any lock in the specified
+range on that particular file descriptor. The locks on other file
+descriptors are ignored by
+.BR F_UNLCK .
 .PP
 In the current implementation,
 .\" commit 57b65325fe34ec4c917bc4e555144b4a94d9e1f7
-- 
2.39.2

Re: [PATCH 0/2] v3: F_OFD_GETLK extension to read lock info

[Jeff Layton]

On Thu, 2023-06-22 at 21:52 +0500, Stas Sergeev wrote:
> This extension allows to use F_UNLCK on query, which currently returns
> EINVAL. Instead it can be used to query the locks on a particular fd -
> something that is not currently possible. The basic idea is that on
> F_OFD_GETLK, F_UNLCK would "conflict" with (or query) any types of the
> lock on the same fd, and ignore any locks on other fds.
> 
> Use-cases:
> 
> 1. CRIU-alike scenario when you want to read the locking info from an
> fd for the later reconstruction. This can now be done by setting
> l_start and l_len to 0 to cover entire file range, and do F_OFD_GETLK.
> In the loop you need to advance l_start past the returned lock ranges,
> to eventually collect all locked ranges.
> 
> 2. Implementing the lock checking/enforcing policy.
> Say you want to implement an "auditor" module in your program,
> that checks that the I/O is done only after the proper locking is
> applied on a file region. In this case you need to know if the
> particular region is locked on that fd, and if so - with what type
> of the lock. If you would do that currently (without this extension)
> then you can only check for the write locks, and for that you need to
> probe the lock on your fd and then open the same file via another fd and
> probe there. That way you can identify the write lock on a particular
> fd, but such trick is non-atomic and complex. As for finding out the
> read lock on a particular fd - impossible.
> This extension allows to do such queries without any extra efforts.
> 
> 3. Implementing the mandatory locking policy.
> Suppose you want to make a policy where the write lock inhibits any
> unlocked readers and writers. Currently you need to check if the
> write lock is present on some other fd, and if it is not there - allow
> the I/O operation. But because the write lock can appear at any moment,
> you need to do that under some global lock, which can be released only
> when the I/O operation is finished.
> With the proposed extension you can instead just check the write lock
> on your own fd first, and if it is there - allow the I/O operation on
> that fd without using any global lock. Only if there is no write lock
> on this fd, then you need to take global lock and check for a write
> lock on other fds.
> 
> 
> The second patch adds a test-case for OFD locks.
> It tests both the generic things and the proposed extension.
> 
> 
> The third patch is a proposed man page update for fcntl(2)
> (not for the linux source tree)
> 
> 
> Changes in v3:
> - Move selftest to selftests/filelock
> 
> Changes in v2:
> - Dropped the l_pid extension patch and updated test-case accordingly.
> 
> Stas Sergeev (2):
>   fs/locks: F_UNLCK extension for F_OFD_GETLK
>   selftests: add OFD lock tests
> 
>  fs/locks.c                                  |  23 +++-
>  tools/testing/selftests/filelock/Makefile   |   5 +
>  tools/testing/selftests/filelock/ofdlocks.c | 132 ++++++++++++++++++++
>  3 files changed, 157 insertions(+), 3 deletions(-)
>  create mode 100644 tools/testing/selftests/filelock/Makefile
>  create mode 100644 tools/testing/selftests/filelock/ofdlocks.c
> 
> CC: Jeff Layton <jlayton@kernel.org>
> CC: Chuck Lever <chuck.lever@oracle.com>
> CC: Alexander Viro <viro@zeniv.linux.org.uk>
> CC: Christian Brauner <brauner@kernel.org>
> CC: linux-fsdevel@vger.kernel.org
> CC: linux-kernel@vger.kernel.org
> CC: Shuah Khan <shuah@kernel.org>
> CC: linux-kselftest@vger.kernel.org
> CC: linux-api@vger.kernel.org
> 

I've taken the first two patches into my locks-next branch, so they
should end up in linux-next soon. Adding support for testing this to
fstests is a hard requirement before this will be merged into mainline.

Thanks,
-- 
Jeff Layton <jlayton@kernel.org>

Re: [PATCH 0/2] v3: F_OFD_GETLK extension to read lock info

[stsp]

27.06.2023 21:23, Jeff Layton пишет:
> I've taken the first two patches into my locks-next branch, so they
> should end up in linux-next soon. Adding support for testing this to
> fstests is a hard requirement before this will be merged into mainline.
Yes, it is _hard_...
I am still trying to set up the buildroot
environment for these tests:

https://bugs.busybox.net/show_bug.cgi?id=15665

And this will take some time, if at all
successful.
Additionally, these tests simply compare
the output with the pre-defined textual
patterns, so I have no idea what to do
after I figured "this feature is unsupported
on this kernel".
I sketched the tests, but the above problems
are holding things.

Re: [PATCH 0/2] v3: F_OFD_GETLK extension to read lock info

[stsp]

27.06.2023 21:23, Jeff Layton пишет:
> I've taken the first two patches into my locks-next branch, so they
> should end up in linux-next soon. Adding support for testing this to
> fstests is a hard requirement before this will be merged into mainline.
The test-suite is entirely broken.
I posted the patch:

https://marc.info/?l=fstests&m=168811805324487&w=2

And the question:
https://marc.info/?l=fstests&m=168811862324941&w=2

But no reaction.
Unless someone helps with reviewing,
nothing will likely happen.