10959 intended nbmand locking functionality is confused

Reviewed by: Matt Barden <matt.barden@nexenta.com>
Reviewed by: Evan Layton <evan.layton@nexenta.com>
Reviewed by: Yuri Pankov <yuri.pankov@nexenta.com>
Approved by: Joshua M. Clulow <josh@sysmgr.org>

4 files changed, 35 insertions, 1 deletions

diff --git a/usr/src/lib/smbsrv/libfksmbsrv/common/fksmb_init.c b/usr/src/lib/smbsrv/libfksmbsrv/common/fksmb_init.c
index f3e7463cb7..80d62d0534 100644
--- a/usr/src/lib/smbsrv/libfksmbsrv/common/fksmb_init.c
+++ b/usr/src/lib/smbsrv/libfksmbsrv/common/fksmb_init.c
@@ -69,6 +69,17 @@ uint_t	smb_audit_flags =
 #endif
 
 /*
+ * We don't normally have nbmand support in the test share
+ * used by fksmbd, but we'd still like the locking code
+ * to be testable.  Intereactions with NFS etc. are not a
+ * concern in fksmbd, so allow it to use advisory locks.
+ *
+ * Should fix the fksmbd test share so it supports nbmand,
+ * and then set this to zero like the real server.
+ */
+int smb_allow_advisory_locks = 1;	/* See smb_vops.c */
+
+/*
  * Maximum number of simultaneous authentication, share mapping, pipe open
  * requests to be processed.
  */
diff --git a/usr/src/uts/common/fs/smbsrv/smb_init.c b/usr/src/uts/common/fs/smbsrv/smb_init.c
index 024a2b5c6c..dd4a15b948 100644
--- a/usr/src/uts/common/fs/smbsrv/smb_init.c
+++ b/usr/src/uts/common/fs/smbsrv/smb_init.c
@@ -78,6 +78,8 @@ uint_t	smb_audit_flags =
     0;
 #endif
 
+int smb_allow_advisory_locks = 0;	/* See smb_vops.c */
+
 /*
  * Maximum number of simultaneous authentication, share mapping, pipe open
  * requests to be processed.
diff --git a/usr/src/uts/common/fs/smbsrv/smb_vops.c b/usr/src/uts/common/fs/smbsrv/smb_vops.c
index 8e6c93f4b5..49b83a0cee 100644
--- a/usr/src/uts/common/fs/smbsrv/smb_vops.c
+++ b/usr/src/uts/common/fs/smbsrv/smb_vops.c
@@ -1448,11 +1448,31 @@ smb_vop_unshrlock(vnode_t *vp, uint32_t uniq_fid, cred_t *cr)
 	return (VOP_SHRLOCK(vp, F_UNSHARE, &shr, 0, cr, NULL));
 }
 
+/*
+ * Note about mandatory vs advisory locks:
+ *
+ * The SMB server really should always request mandatory locks, and
+ * if the file system does not support them, the SMB server should
+ * just tell the client it could not get the lock. If we were to
+ * tell the SMB client "you got the lock" when what they really
+ * got was only an advisory lock, we would be lying to the client
+ * about their having exclusive access to the locked range, which
+ * could easily lead to data corruption.  If someone really wants
+ * the (dangerous) behavior they can set: smb_allow_advisory_locks
+ */
 int
 smb_vop_frlock(vnode_t *vp, cred_t *cr, int flag, flock64_t *bf)
 {
-	int cmd = nbl_need_check(vp) ? F_SETLK_NBMAND : F_SETLK;
 	flk_callback_t flk_cb;
+	int cmd = F_SETLK_NBMAND;
+
+	if (smb_allow_advisory_locks != 0 && !nbl_need_check(vp)) {
+		/*
+		 * The file system does not support nbmand, and
+		 * smb_allow_advisory_locks is enabled. (danger!)
+		 */
+		cmd = F_SETLK;
+	}
 
 	flk_init_callback(&flk_cb, smb_lock_frlock_callback, NULL);
 
diff --git a/usr/src/uts/common/smbsrv/smb_kproto.h b/usr/src/uts/common/smbsrv/smb_kproto.h
index 2dd8787aeb..ac91ab17c1 100644
--- a/usr/src/uts/common/smbsrv/smb_kproto.h
+++ b/usr/src/uts/common/smbsrv/smb_kproto.h
@@ -117,6 +117,7 @@ extern	int smb_opipe_threshold;
 extern	int smb_ssetup_timeout;
 extern	int smb_tcon_timeout;
 extern	int smb_opipe_timeout;
+extern	int smb_allow_advisory_locks;
 extern const uint32_t smb_vop_dosattr_settable;
 
 /* Thread priorities - see smb_init.c */