[PATCH] correctly guard SMB_VFS_KERNEL_FLOCK with lp_kernel_share_modes (bug 11919)

[Michael Adam]

On 2016-05-16 at 01:18 +0200, Michael Adam wrote:
> On 2016-05-16 at 00:24 +0200, Stefan Metzmacher wrote:
> > Hi Michael,
> > 
> > > Please review attached patch.
> > 
> > I think we better remember if we called it at open time,
> > In theory the value of lp_kernel_share_modes() can change
> > during an reload of smb.conf.
> 
> Ah, good point, thanks!
> I guess that means we'll remember it on the fsp, right?

like attached


-------------- next part --------------
From 5ff5b814b4e7437ae0389519da0db8b7fe6d92ce Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Mon, 16 May 2016 01:39:09 +0200
Subject: [PATCH 1/2] s3:vfs: add 'kernel_share_modes_taken' to files_struct

This will allow to track whether kernel share modes have been
taken at open and correclty remove them again on close.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=11919

Signed-off-by: Michael Adam <obnox at samba.org>
---
 source3/include/vfs.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/source3/include/vfs.h b/source3/include/vfs.h
index 9360802..a849443 100644
--- a/source3/include/vfs.h
+++ b/source3/include/vfs.h
@@ -259,6 +259,8 @@ typedef struct files_struct {
 	uint32_t access_mask;		/* NTCreateX access bits (FILE_READ_DATA etc.) */
 	uint32_t share_access;		/* NTCreateX share constants (FILE_SHARE_READ|FILE_SHARE_WRITE|FILE_SHARE_DELETE). */
 
+	bool kernel_share_modes_taken;
+
 	bool update_write_time_triggered;
 	struct tevent_timer *update_write_time_event;
 	bool update_write_time_on_close;
-- 
2.5.5


From 9406cb8f5ea4e5831bc7e3ff1cd0c0bc810de15b Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Sun, 15 May 2016 23:24:08 +0200
Subject: [PATCH 2/2] smbd:close: only remove kernel share modes if they had
 been taken at open

This avoids errors due to 'not implemented' for SMB_VFS_KERNEL_FLOCK
on some file systems like glusterfs (with the vfs module). The only
other code path where SMB_VFS_KERNEL_FLOCK is called, is already protected.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=11919

Signed-off-by: Michael Adam <obnox at samba.org>
---
 source3/smbd/close.c | 17 ++++++++++-------
 source3/smbd/open.c  |  2 ++
 2 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/source3/smbd/close.c b/source3/smbd/close.c
index 0302c67..9d1f1a9 100644
--- a/source3/smbd/close.c
+++ b/source3/smbd/close.c
@@ -250,7 +250,6 @@ static NTSTATUS close_remove_share_mode(files_struct *fsp,
 	const struct security_token *del_nt_token = NULL;
 	bool got_tokens = false;
 	bool normal_close;
-	int ret_flock;
 
 	/* Ensure any pending write time updates are done. */
 	if (fsp->update_write_time_event) {
@@ -474,12 +473,16 @@ static NTSTATUS close_remove_share_mode(files_struct *fsp,
 		pop_sec_ctx();
 	}
 
-	/* remove filesystem sharemodes */
-	ret_flock = SMB_VFS_KERNEL_FLOCK(fsp, 0, 0);
-	if (ret_flock == -1) {
-		DEBUG(2, ("close_remove_share_mode: removing kernel flock for "
-					"%s failed: %s\n", fsp_str_dbg(fsp),
-					strerror(errno)));
+	if (fsp->kernel_share_modes_taken) {
+		int ret_flock;
+
+		/* remove filesystem sharemodes */
+		ret_flock = SMB_VFS_KERNEL_FLOCK(fsp, 0, 0);
+		if (ret_flock == -1) {
+			DEBUG(2, ("close_remove_share_mode: removing kernel "
+				  "flock for %s failed: %s\n",
+				  fsp_str_dbg(fsp), strerror(errno)));
+		}
 	}
 
 	if (!del_share_mode(lck, fsp)) {
diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index d111254..af1c1de 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -3102,6 +3102,8 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn,
 
 			return NT_STATUS_SHARING_VIOLATION;
 		}
+
+		fsp->kernel_share_modes_taken = true;
 	}
 
 	/*
-- 
2.5.5

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20160516/8bd6a84a/signature.sig>