11010 Obsolete SMB server work-around for ZFS read-only

Reviewed by: Matt Barden <matt.barden@nexenta.com>
Reviewed by: Evan Layton <evan.layton@nexenta.com>
Approved by: Garrett D'Amore <garrett@damore.org>

6 files changed, 42 insertions, 148 deletions

diff --git a/usr/src/cmd/mdb/common/modules/smbsrv/smbsrv.c b/usr/src/cmd/mdb/common/modules/smbsrv/smbsrv.c
index 9e1402a5dc..d1ec161350 100644
--- a/usr/src/cmd/mdb/common/modules/smbsrv/smbsrv.c
+++ b/usr/src/cmd/mdb/common/modules/smbsrv/smbsrv.c
@@ -21,7 +21,7 @@
 
 /*
  * Copyright (c) 2007, 2010, Oracle and/or its affiliates. All rights reserved.
- * Copyright 2016 Nexenta Systems, Inc. All rights reserved.
+ * Copyright 2017 Nexenta Systems, Inc. All rights reserved.
  */
 
 #include <mdb/mdb_modapi.h>
@@ -1721,9 +1721,7 @@ typedef struct mdb_smb_ofile {
 
 static const mdb_bitmask_t
 ofile_flag_bits[] = {
-	{ "RO",
-	    SMB_OFLAGS_READONLY,
-	    SMB_OFLAGS_READONLY },
+	{ "RO", 1, 1 }, /* old SMB_OFLAGS_READONLY */
 	{ "EXEC",
 	    SMB_OFLAGS_EXECONLY,
 	    SMB_OFLAGS_EXECONLY },
diff --git a/usr/src/uts/common/fs/smbsrv/smb_common_open.c b/usr/src/uts/common/fs/smbsrv/smb_common_open.c
index 64f375f11f..331cf2c729 100644
--- a/usr/src/uts/common/fs/smbsrv/smb_common_open.c
+++ b/usr/src/uts/common/fs/smbsrv/smb_common_open.c
@@ -244,14 +244,10 @@ smb_common_open(smb_request_t *sr)
  * 1. The creator of a readonly file can write to/modify the size of the file
  * using the original create fid, even though the file will appear as readonly
  * to all other fids and via a CIFS getattr call.
- * The readonly bit therefore cannot be set in the filesystem until the file
- * is closed (smb_ofile_close). It is accounted for via ofile and node flags.
  *
  * 2. A setinfo operation (using either an open fid or a path) to set/unset
  * readonly will be successful regardless of whether a creator of a readonly
- * file has an open fid (and has the special privilege mentioned in #1,
- * above).  I.e., the creator of a readonly fid holding that fid will no longer
- * have a special privilege.
+ * file has an open fid.
  *
  * 3. The DOS readonly bit affects only data and some metadata.
  * The following metadata can be changed regardless of the readonly bit:
@@ -636,10 +632,10 @@ smb_open_subr(smb_request_t *sr)
 		case FILE_OVERWRITE_IF:
 		case FILE_OVERWRITE:
 			op->dattr |= FILE_ATTRIBUTE_ARCHIVE;
-			/* Don't apply readonly bit until smb_ofile_close */
+			/* Don't apply readonly until smb_set_open_attributes */
 			if (op->dattr & FILE_ATTRIBUTE_READONLY) {
-				op->created_readonly = B_TRUE;
 				op->dattr &= ~FILE_ATTRIBUTE_READONLY;
+				op->created_readonly = B_TRUE;
 			}
 
 			/*
@@ -728,16 +724,26 @@ create:
 		 */
 		smb_node_wrlock(dnode);
 
-		/* Don't apply readonly bit until smb_ofile_close */
+		/*
+		 * Create always sets the DOS attributes, type, and mode
+		 * in the if/else below (different for file vs directory).
+		 * Don't set the readonly bit until smb_set_open_attributes
+		 * or that would prevent this open.  Note that op->dattr
+		 * needs to be what smb_set_open_attributes will use,
+		 * except for the readonly bit.
+		 */
+		bzero(&new_attr, sizeof (new_attr));
+		new_attr.sa_mask = SMB_AT_DOSATTR | SMB_AT_TYPE | SMB_AT_MODE;
 		if (op->dattr & FILE_ATTRIBUTE_READONLY) {
 			op->dattr &= ~FILE_ATTRIBUTE_READONLY;
 			op->created_readonly = B_TRUE;
 		}
 
-		bzero(&new_attr, sizeof (new_attr));
+		/*
+		 * SMB create can specify the create time.
+		 */
 		if ((op->crtime.tv_sec != 0) &&
 		    (op->crtime.tv_sec != UINT_MAX)) {
-
 			new_attr.sa_mask |= SMB_AT_CRTIME;
 			new_attr.sa_crtime = op->crtime;
 		}
@@ -746,11 +752,12 @@ create:
 			op->dattr |= FILE_ATTRIBUTE_ARCHIVE;
 			new_attr.sa_dosattr = op->dattr;
 			new_attr.sa_vattr.va_type = VREG;
-			new_attr.sa_vattr.va_mode = is_stream ? S_IRUSR :
-			    S_IRUSR | S_IRGRP | S_IROTH |
-			    S_IWUSR | S_IWGRP | S_IWOTH;
-			new_attr.sa_mask |=
-			    SMB_AT_DOSATTR | SMB_AT_TYPE | SMB_AT_MODE;
+			if (is_stream)
+				new_attr.sa_vattr.va_mode = S_IRUSR | S_IWUSR;
+			else
+				new_attr.sa_vattr.va_mode =
+				    S_IRUSR | S_IRGRP | S_IROTH |
+				    S_IWUSR | S_IWGRP | S_IWOTH;
 
 			/*
 			 * We set alloc_size = op->dsize later,
@@ -795,8 +802,6 @@ create:
 			new_attr.sa_dosattr = op->dattr;
 			new_attr.sa_vattr.va_type = VDIR;
 			new_attr.sa_vattr.va_mode = 0777;
-			new_attr.sa_mask |=
-			    SMB_AT_DOSATTR | SMB_AT_TYPE | SMB_AT_MODE;
 
 			rc = smb_fsop_mkdir(sr, sr->user_cr, dnode,
 			    op->fqi.fq_last_comp, &new_attr, &op->fqi.fq_fnode);
@@ -849,8 +854,8 @@ create:
 
 	/*
 	 * This MUST be done after ofile creation, so that explicitly
-	 * set timestamps can be remembered on the ofile, and the
-	 * readonly flag will be stored "pending" on the node.
+	 * set timestamps can be remembered on the ofile, and setting
+	 * the readonly flag won't affect access via this open.
 	 */
 	if (status == NT_STATUS_SUCCESS) {
 		if ((rc = smb_set_open_attributes(sr, of)) != 0) {
@@ -1007,8 +1012,6 @@ smb_open_overwrite(smb_arg_open_t *op)
  * - If we created_readonly, we now store the real DOS attributes
  *   (including the readonly bit) so subsequent opens will see it.
  *
- * Both are stored "pending" rather than in the file system.
- *
  * Returns: errno
  */
 static int
@@ -1047,7 +1050,7 @@ smb_set_open_attributes(smb_request_t *sr, smb_ofile_t *of)
 	 * However, keep track of the fact that we modified
 	 * the file via this handle, so we can do the evil,
 	 * gratuitious mtime update on close that Windows
-	 * clients appear to expect.
+	 * clients expect.
 	 */
 	if (op->action_taken == SMB_OACT_TRUNCATED)
 		of->f_written = B_TRUE;
diff --git a/usr/src/uts/common/fs/smbsrv/smb_fsops.c b/usr/src/uts/common/fs/smbsrv/smb_fsops.c
index e94e7e7d99..819547773d 100644
--- a/usr/src/uts/common/fs/smbsrv/smb_fsops.c
+++ b/usr/src/uts/common/fs/smbsrv/smb_fsops.c
@@ -1243,25 +1243,6 @@ smb_fsop_setattr(
 		return (EACCES);
 
 	/*
-	 * The file system cannot detect pending READDONLY
-	 * (i.e. if the file has been opened readonly but
-	 * not yet closed) so we need to test READONLY here.
-	 *
-	 * Note that file handle that were opened before the
-	 * READONLY flag was set in the node (or the FS) are
-	 * immune to that change, and remain writable.
-	 */
-	if (sr && (set_attr->sa_mask & SMB_AT_SIZE)) {
-		if (sr->fid_ofile) {
-			if (SMB_OFILE_IS_READONLY(sr->fid_ofile))
-				return (EACCES);
-		} else {
-			if (SMB_PATHFILE_IS_READONLY(sr, snode))
-				return (EACCES);
-		}
-	}
-
-	/*
 	 * SMB checks access on open and retains an access granted
 	 * mask for use while the file is open.  ACL changes should
 	 * not affect access to an open file.
@@ -1351,23 +1332,6 @@ smb_fsop_set_data_length(
 		return (EACCES);
 
 	/*
-	 * The file system cannot detect pending READDONLY
-	 * (i.e. if the file has been opened readonly but
-	 * not yet closed) so we need to test READONLY here.
-	 *
-	 * Note that file handle that were opened before the
-	 * READONLY flag was set in the node (or the FS) are
-	 * immune to that change, and remain writable.
-	 */
-	if (sr->fid_ofile) {
-		if (SMB_OFILE_IS_READONLY(sr->fid_ofile))
-			return (EACCES);
-	} else {
-		/* This requires an open file. */
-		return (EACCES);
-	}
-
-	/*
 	 * SMB checks access on open and retains an access granted
 	 * mask for use while the file is open.  ACL changes should
 	 * not affect access to an open file.
@@ -1502,8 +1466,7 @@ smb_fsop_write(
 	if (SMB_TREE_IS_READONLY(sr))
 		return (EROFS);
 
-	if (SMB_OFILE_IS_READONLY(of) ||
-	    SMB_TREE_HAS_ACCESS(sr, ACE_WRITE_DATA | ACE_APPEND_DATA) == 0)
+	if (SMB_TREE_HAS_ACCESS(sr, ACE_WRITE_DATA | ACE_APPEND_DATA) == 0)
 		return (EACCES);
 
 	rc = smb_ofile_access(of, cr, FILE_WRITE_DATA);
diff --git a/usr/src/uts/common/fs/smbsrv/smb_node.c b/usr/src/uts/common/fs/smbsrv/smb_node.c
index 73152e7d33..e19d4e668b 100644
--- a/usr/src/uts/common/fs/smbsrv/smb_node.c
+++ b/usr/src/uts/common/fs/smbsrv/smb_node.c
@@ -1165,7 +1165,6 @@ smb_node_alloc(
 	node->n_refcnt = 1;
 	node->n_hash_bucket = bucket;
 	node->n_hashkey = hashkey;
-	node->n_pending_dosattr = 0;
 	node->n_open_count = 0;
 	node->n_allocsz = 0;
 	node->n_dnode = NULL;
@@ -1387,9 +1386,9 @@ smb_node_is_system(smb_node_t *node)
  * smb_node_file_is_readonly
  *
  * Checks if the file (which node represents) is marked readonly
- * in the filesystem. No account is taken of any pending readonly
- * in the node, which must be handled by the callers.
- * (See SMB_OFILE_IS_READONLY and SMB_PATHFILE_IS_READONLY)
+ * in the filesystem.  Note that there may be handles open with
+ * modify rights, and those continue to allow access even after
+ * the DOS read-only flag has been set in the file system.
  */
 boolean_t
 smb_node_file_is_readonly(smb_node_t *node)
@@ -1399,9 +1398,6 @@ smb_node_file_is_readonly(smb_node_t *node)
 	if (node == NULL)
 		return (B_FALSE);	/* pipes */
 
-	if (node->n_pending_dosattr & FILE_ATTRIBUTE_READONLY)
-		return (B_TRUE);
-
 	bzero(&attr, sizeof (smb_attr_t));
 	attr.sa_mask = SMB_AT_DOSATTR;
 	(void) smb_fsop_getattr(NULL, zone_kcred(), node, &attr);
@@ -1573,40 +1569,18 @@ smb_node_setattr(smb_request_t *sr, smb_node_t *node,
 		 */
 	}
 
-	/*
-	 * After this point, tmp_attr is what we will actually
-	 * store in the file system _now_, which may differ
-	 * from the callers attr and f_pending_attr w.r.t.
-	 * the DOS readonly flag etc.
-	 */
-	bcopy(attr, &tmp_attr, sizeof (tmp_attr));
-	if (attr->sa_mask & (SMB_AT_DOSATTR | SMB_AT_ALLOCSZ)) {
+	if ((attr->sa_mask & SMB_AT_ALLOCSZ) != 0) {
 		mutex_enter(&node->n_mutex);
-		if ((attr->sa_mask & SMB_AT_DOSATTR) != 0) {
-			tmp_attr.sa_dosattr &= smb_vop_dosattr_settable;
-			if (((tmp_attr.sa_dosattr &
-			    FILE_ATTRIBUTE_READONLY) != 0) &&
-			    (node->n_open_count != 0)) {
-				/* Delay setting readonly */
-				node->n_pending_dosattr =
-				    tmp_attr.sa_dosattr;
-				tmp_attr.sa_dosattr &=
-				    ~FILE_ATTRIBUTE_READONLY;
-			} else {
-				node->n_pending_dosattr = 0;
-			}
-		}
 		/*
 		 * Simulate n_allocsz persistence only while
 		 * there are opens.  See smb_node_getattr
 		 */
-		if ((attr->sa_mask & SMB_AT_ALLOCSZ) != 0 &&
-		    node->n_open_count != 0)
+		if (node->n_open_count != 0)
 			node->n_allocsz = attr->sa_allocsz;
 		mutex_exit(&node->n_mutex);
 	}
 
-	rc = smb_fsop_setattr(sr, cr, node, &tmp_attr);
+	rc = smb_fsop_setattr(sr, cr, node, attr);
 	if (rc != 0)
 		return (rc);
 
@@ -1655,21 +1629,7 @@ smb_node_getattr(smb_request_t *sr, smb_node_t *node, cred_t *cr,
 
 	mutex_enter(&node->n_mutex);
 
-	/*
-	 * When there are open handles, and one of them has
-	 * set the DOS readonly flag (in n_pending_dosattr),
-	 * it will not have been stored in the file system.
-	 * In this case use n_pending_dosattr. Note that
-	 * n_pending_dosattr has only the settable bits,
-	 * (setattr masks it with smb_vop_dosattr_settable)
-	 * so we need to keep any non-settable bits we got
-	 * from the file-system above.
-	 */
 	if (attr->sa_mask & SMB_AT_DOSATTR) {
-		if (node->n_pending_dosattr) {
-			attr->sa_dosattr &= ~smb_vop_dosattr_settable;
-			attr->sa_dosattr |= node->n_pending_dosattr;
-		}
 		if (attr->sa_dosattr == 0) {
 			attr->sa_dosattr = (isdir) ?
 			    FILE_ATTRIBUTE_DIRECTORY:
diff --git a/usr/src/uts/common/fs/smbsrv/smb_ofile.c b/usr/src/uts/common/fs/smbsrv/smb_ofile.c
index 64c0682932..445ef9f040 100644
--- a/usr/src/uts/common/fs/smbsrv/smb_ofile.c
+++ b/usr/src/uts/common/fs/smbsrv/smb_ofile.c
@@ -22,7 +22,7 @@
  * Copyright (c) 2007, 2010, Oracle and/or its affiliates. All rights reserved.
  * Copyright 2016 Syneto S.R.L. All rights reserved.
  * Copyright (c) 2016 by Delphix. All rights reserved.
- * Copyright 2019 Nexenta Systems, Inc.  All rights reserved.
+ * Copyright 2017 Nexenta Systems, Inc.  All rights reserved.
  */
 
 /*
@@ -278,18 +278,6 @@ smb_ofile_open(
 			}
 		}
 
-		if (tree->t_flags & SMB_TREE_READONLY)
-			of->f_flags |= SMB_OFLAGS_READONLY;
-
-		/*
-		 * Note that if we created_readonly, that
-		 * will _not_ yet show in attr.sa_dosattr
-		 * so creating a readonly file gives the
-		 * caller a writable handle as it should.
-		 */
-		if (attr.sa_dosattr & FILE_ATTRIBUTE_READONLY)
-			of->f_flags |= SMB_OFLAGS_READONLY;
-
 		smb_node_inc_open_ofiles(node);
 		smb_node_add_ofile(node, of);
 		smb_node_ref(node);
@@ -416,22 +404,18 @@ smb_ofile_close(smb_ofile_t *of, int32_t mtime_sec)
 		}
 		if (smb_node_dec_open_ofiles(of->f_node) == 0) {
 			/*
-			 * Last close. The f_pending_attr has
-			 * only times (atime,ctime,mtime) so
-			 * we can borrow it to commit the
-			 * n_pending_dosattr from the node.
+			 * Last close.  If we're not deleting
+			 * the file, apply any pending attrs.
+			 * Leave allocsz zero when no open files,
+			 * just to avoid confusion, because it's
+			 * only updated when there are opens.
 			 */
-			pa->sa_dosattr =
-			    of->f_node->n_pending_dosattr;
-			if (pa->sa_dosattr != 0)
-				pa->sa_mask |= SMB_AT_DOSATTR;
-			/* Let's leave this zero when not in use. */
-			of->f_node->n_allocsz = 0;
 			mutex_enter(&of->f_node->n_mutex);
 			if (of->f_node->flags & NODE_FLAGS_DELETE_ON_CLOSE) {
 				smb_node_delete_on_close(of->f_node);
 				pa->sa_mask = 0;
 			}
+			of->f_node->n_allocsz = 0;
 			mutex_exit(&of->f_node->n_mutex);
 		}
 		if (pa->sa_mask != 0) {
@@ -441,9 +425,6 @@ smb_ofile_close(smb_ofile_t *of, int32_t mtime_sec)
 			 * we pass NULL as the ofile to setattr
 			 * so it will write to the file system
 			 * and not keep anything on the ofile.
-			 * This clears n_pending_dosattr if
-			 * there are no opens, otherwise the
-			 * dosattr will be pending again.
 			 */
 			(void) smb_node_setattr(NULL, of->f_node,
 			    of->f_cr, NULL, pa);
diff --git a/usr/src/uts/common/smbsrv/smb_ktypes.h b/usr/src/uts/common/smbsrv/smb_ktypes.h
index 5f38203b16..6195dbd9fd 100644
--- a/usr/src/uts/common/smbsrv/smb_ktypes.h
+++ b/usr/src/uts/common/smbsrv/smb_ktypes.h
@@ -667,7 +667,6 @@ typedef struct smb_node {
 	/* If entering both, go in order n_lock_list, n_wlock_list */
 	smb_llist_t		n_lock_list;	/* active locks */
 	smb_llist_t		n_wlock_list;	/* waiting locks */
-	uint32_t		n_pending_dosattr;
 	volatile int		flags;
 	u_offset_t		n_allocsz;
 	uint32_t		n_fcn_count;
@@ -1123,16 +1122,6 @@ typedef struct smb_tree {
 	smb_tree_has_feature((sr)->tid_tree, SMB_TREE_TRAVERSE_MOUNTS))
 
 /*
- * SMB_OFILE_IS_READONLY reflects whether an ofile is readonly or not.
- * The macro takes into account read-only settings in any of:
- * the tree, the node (pending) and the file-system object.
- * all of this is evaluated in smb_ofile_open() and after that
- * we can just test the f_flags & SMB_OFLAGS_READONLY
- */
-#define	SMB_OFILE_IS_READONLY(of)	\
-	((of)->f_flags & SMB_OFLAGS_READONLY)
-
-/*
  * SMB_PATHFILE_IS_READONLY indicates whether or not a file is
  * readonly when the caller has a path rather than an ofile.
  */
@@ -1265,7 +1254,7 @@ typedef struct smb_opipe {
  *   will be set for the file node upon close.
  */
 
-#define	SMB_OFLAGS_READONLY		0x0001
+/*	SMB_OFLAGS_READONLY		0x0001 (obsolete) */
 #define	SMB_OFLAGS_EXECONLY		0x0002
 #define	SMB_OFLAGS_SET_DELETE_ON_CLOSE	0x0004
 #define	SMB_OFLAGS_LLF_POS_VALID	0x0008