[PATCH] flock() files even with a permissive share mode.

[Matthew Stickney]

I've only just begun poking around the samba code, so apologies if I've 
missed something obvious (or if this is the wrong list for this sort of 
question).

In doing some tests for a project I'm working on, I've noticed that if I 
acquire an advisory lock on a file in a share, with e.g.

 > $ flock -x /shares/test/lock_test.txt cat

a windows machine is able to read this file without any contention. I 
strace'd the smbd process in question, and the file isn't flock()ed 
until it's being closed.

Looking at the source for kernel_flock() in source3/lib/system.c, 
flock()ing is only done if the share mode is something more restrictive 
than FILE_SHARE_READ|FILE_SHARE_WRITE (the default mode on windows). But 
that will ignore any more restrictive advisory locks that have been made 
on that file -- I haven't tested this, but I assume that would include 
other users having the file open in the share. It's certainly the case 
for local filesystem clients.

The attached patch alters kernel_flock() to always flock() the file, 
even with a permissive share mode. I'm not sure if there might be 
unintended consequences of doing that, though (a possible performance 
hit comes to mind).

Having done a bit of testing, this problem actually looks weirder: I've 
been unable to get LOCK_MAND flocks to conflict with either traditional 
or other LOCK_MAND flocks. Looking at the kernel source for 
flock_locks_conflict() in fs/locks.c, it looks like LOCK_MAND locks 
never conflict with anything, including each other. On top of that, the 
lock type is always coerced to LOCK_MAND|LOCK_RW in 
flock_translate_cmd(). Given that this has been non-functional for the 
last 14 years, should we just drop the flock()ing altogether?

-Matt Stickney

-------------- next part --------------
From e64d71b181224979aab3151037f779d708eeb9b8 Mon Sep 17 00:00:00 2001
From: Matthew Stickney <mtstickney at gmail.com>
Date: Fri, 1 Feb 2019 20:31:31 -0500
Subject: [PATCH] flock() files even with a permissive share mode.

If another process has set a more restrictive flock() on the file, we
don't want to ignore it because the requested share mode is permissive.
Instead of second-guessing the kernel, always flock() the file and let the
kernel deal with whether the lock conflicts or not.
---
 source3/lib/system.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/source3/lib/system.c b/source3/lib/system.c
index 9dd04ca5e3d..f98f1b60099 100644
--- a/source3/lib/system.c
+++ b/source3/lib/system.c
@@ -525,17 +525,16 @@ int sys_fallocate(int fd, uint32_t mode, off_t offset, off_t len)
 void kernel_flock(int fd, uint32_t share_mode, uint32_t access_mask)
 {
 #ifdef HAVE_KERNEL_SHARE_MODES
-	int kernel_mode = 0;
-	if (share_mode == FILE_SHARE_WRITE) {
-		kernel_mode = LOCK_MAND|LOCK_WRITE;
-	} else if (share_mode == FILE_SHARE_READ) {
-		kernel_mode = LOCK_MAND|LOCK_READ;
-	} else if (share_mode == FILE_SHARE_NONE) {
-		kernel_mode = LOCK_MAND;
+	int kernel_mode = LOCK_MAND;
+
+	if (share_mode & FILE_SHARE_WRITE) {
+		kernel_mode |= LOCK_WRITE;
 	}
-	if (kernel_mode) {
-		flock(fd, kernel_mode);
+	if (share_mode & FILE_SHARE_READ) {
+		kernel_mode = LOCK_READ;
 	}
+
+	flock(fd, kernel_mode);
 #endif
 	;
 }
-- 
2.20.1