6493853 sharemgr: inconsistent messages if parent is shared with UFS and ZFS

6499968 sharemgr: timing window can cause problems during system start if dfstab was modified
6502752 sharemgr: nfs_get_root_principal: bad host name
6510387 sharemgr: add-share prints error
6511920 sharemgr: set with -S krb5 optionset sets the group to nfs:sys
6513576 sharemgr: under some error conditions can't remove a share

2 files changed, 248 insertions, 120 deletions

diff --git a/usr/src/lib/libshare/common/libshare.c b/usr/src/lib/libshare/common/libshare.c
index 9b54b30e1b..e221c61cb7 100644
--- a/usr/src/lib/libshare/common/libshare.c
+++ b/usr/src/lib/libshare/common/libshare.c
@@ -34,6 +34,7 @@
 #include <ctype.h>
 #include <sys/types.h>
 #include <sys/stat.h>
+#include <fcntl.h>
 #include <unistd.h>
 #include <libxml/parser.h>
 #include <libxml/tree.h>
@@ -51,6 +52,8 @@
 #define	TSTAMP(tm)	(uint64_t)(((uint64_t)tm.tv_sec << 32) | \
 					(tm.tv_nsec & 0xffffffff))
 
+#define	DFS_LOCK_FILE	"/etc/dfs/fstypes"
+
 /*
  * internal data structures
  */
@@ -74,6 +77,8 @@ extern void update_legacy_config(void);
 extern int issubdir(char *, char *);
 extern void sa_zfs_init(void);
 extern void sa_zfs_fini(void);
+extern void sablocksigs(sigset_t *);
+extern void saunblocksigs(sigset_t *);
 
 static int sa_initialized = 0;
 
@@ -167,46 +172,6 @@ sa_errorstr(int err)
 }
 
 /*
- * get_legacy_timestamp(root, path)
- *	gets the timestamp of the last time sharemgr updated the legacy
- *	files. This is used to determine if someone has modified them by
- *	hand.
- */
-static uint64_t
-get_legacy_timestamp(xmlNodePtr root, char *path)
-{
-	uint64_t tval = 0;
-	xmlNodePtr node;
-	xmlChar *lpath = NULL;
-	xmlChar *timestamp = NULL;
-
-	for (node = root->xmlChildrenNode; node != NULL;
-		node = node->next) {
-	    if (xmlStrcmp(node->name, (xmlChar *)"legacy") == 0) {
-		/* a possible legacy node for this path */
-		lpath = xmlGetProp(node, (xmlChar *)"path");
-		if (lpath != NULL && xmlStrcmp(lpath, (xmlChar *)path) == 0) {
-		    /* now have the node, extract the data */
-		    timestamp = xmlGetProp(node, (xmlChar *)"timestamp");
-		    if (timestamp != NULL) {
-			tval = strtoull((char *)timestamp, NULL, 0);
-			break;
-		    }
-		}
-		if (lpath != NULL) {
-		    xmlFree(lpath);
-		    lpath = NULL;
-		}
-	    }
-	}
-	if (lpath != NULL)
-	    xmlFree(lpath);
-	if (timestamp != NULL)
-	    xmlFree(timestamp);
-	return (tval);
-}
-
-/*
  * set_legacy_timestamp(root, path, timevalue)
  *
  * add the current timestamp value to the configuration for use in
@@ -282,30 +247,25 @@ is_shared(sa_share_t share)
 }
 
 /*
- * checksubdir(newpath, strictness)
+ * checksubdirgroup(group, newpath, strictness)
  *
- * checksubdir determines if the specified path (newpath) is a
- * subdirectory of another share. It calls issubdir() from the old
- * share implementation to do the complicated work. The strictness
- * parameter determines how strict a check to make against the
- * path. The strictness values mean:
+ * check all the specified newpath against all the paths in the
+ * group. This is a helper function for checksubdir to make it easier
+ * to also check ZFS subgroups.
+ * The strictness values mean:
  * SA_CHECK_NORMAL == only check newpath against shares that are active
  * SA_CHECK_STRICT == check newpath against both active shares and those
  *		      stored in the repository
  */
 static int
-checksubdir(char *newpath, int strictness)
+checksubdirgroup(sa_group_t group, char *newpath, int strictness)
 {
-	sa_group_t group;
 	sa_share_t share;
-	int issub;
-	char *path = NULL;
+	char *path;
+	int issub = SA_OK;
 
-	for (issub = 0, group = sa_get_group(NULL);
-		group != NULL && !issub;
-		group = sa_get_next_group(group)) {
-	    for (share = sa_get_share(group, NULL); share != NULL;
-		    share = sa_get_next_share(share)) {
+	for (share = sa_get_share(group, NULL); share != NULL;
+	    share = sa_get_next_share(share)) {
 		/*
 		 * The original behavior of share never checked
 		 * against the permanent configuration
@@ -314,27 +274,63 @@ checksubdir(char *newpath, int strictness)
 		 * could be considered incorrect.  We may tighten this
 		 * up in the future.
 		 */
-		if (strictness == SA_CHECK_NORMAL && !is_shared(share))
-		    continue;
+	    if (strictness == SA_CHECK_NORMAL && !is_shared(share))
+		continue;
 
-		path = sa_get_share_attr(share, "path");
+	    path = sa_get_share_attr(share, "path");
 		/*
 		 * If path is NULL, then a share is in the process of
 		 * construction or someone has modified the property
-		 * group inappropriately. It should be ignored.
+		 * group inappropriately. It should be
+		 * ignored. issubdir() comes from the original share
+		 * implementation and does the difficult part of
+		 * checking subdirectories.
 		 */
-		if (path == NULL)
-		    continue;
-		if (newpath != NULL &&
-		    (strcmp(path, newpath) == 0 || issubdir(newpath, path) ||
-			issubdir(path, newpath))) {
-		    sa_free_attr_string(path);
-		    path = NULL;
-		    issub = SA_INVALID_PATH;
-		    break;
-		}
+	    if (path == NULL)
+		continue;
+	    if (newpath != NULL &&
+		(strcmp(path, newpath) == 0 || issubdir(newpath, path) ||
+		issubdir(path, newpath))) {
 		sa_free_attr_string(path);
 		path = NULL;
+		issub = SA_INVALID_PATH;
+		break;
+	    }
+	    sa_free_attr_string(path);
+	    path = NULL;
+	}
+	return (issub);
+}
+
+/*
+ * checksubdir(newpath, strictness)
+ *
+ * checksubdir determines if the specified path (newpath) is a
+ * subdirectory of another share. It calls checksubdirgroup() to do
+ * the complicated work. The strictness parameter determines how
+ * strict a check to make against the path. The strictness values
+ * mean: SA_CHECK_NORMAL == only check newpath against shares that are
+ * active SA_CHECK_STRICT == check newpath against both active shares
+ * and those * stored in the repository
+ */
+static int
+checksubdir(char *newpath, int strictness)
+{
+	sa_group_t group;
+	int issub;
+	char *path = NULL;
+
+	for (issub = 0, group = sa_get_group(NULL);
+		group != NULL && !issub;
+		group = sa_get_next_group(group)) {
+	    if (sa_group_is_zfs(group)) {
+		sa_group_t subgroup;
+		for (subgroup = sa_get_sub_group(group);
+		    subgroup != NULL && !issub;
+		    subgroup = sa_get_next_group(subgroup))
+		    issub = checksubdirgroup(subgroup, newpath, strictness);
+	    } else {
+		issub = checksubdirgroup(group, newpath, strictness);
 	    }
 	}
 	if (path != NULL)
@@ -378,7 +374,7 @@ validpath(char *path, int strictness)
 		fstype = sa_fstype(path);
 		if (fstype != NULL && strcmp(fstype, "zfs") == 0) {
 		    if (sa_zfs_is_shared(path))
-			error = SA_DUPLICATE_NAME;
+			error = SA_INVALID_NAME;
 		}
 		if (fstype != NULL)
 		    sa_free_fstype(fstype);
@@ -541,6 +537,10 @@ sa_init(int init_service)
 	struct stat st;
 	int legacy = 0;
 	uint64_t tval = 0;
+	int lockfd;
+	sigset_t old;
+	int updatelegacy = B_FALSE;
+	scf_simple_prop_t *prop;
 
 	if (!sa_initialized) {
 	    /* get protocol specific structures */
@@ -557,10 +557,62 @@ sa_init(int init_service)
 		 */
 		scf_handle = sa_scf_init();
 		if (scf_handle != NULL) {
+			/*
+			 * Need to lock the extraction of the
+			 * configuration if the dfstab file has
+			 * changed. Lock everything now and release if
+			 * not needed.  Use a file that isn't being
+			 * manipulated by other parts of the system in
+			 * order to not interfere with locking. Using
+			 * dfstab doesn't work.
+			 */
+		    sablocksigs(&old);
+		    lockfd = open(DFS_LOCK_FILE, O_RDWR);
+		    if (lockfd >= 0) {
+			extern int errno;
+			errno = 0;
+			(void) lockf(lockfd, F_LOCK, 0);
+			/*
+			 * Check whether we are going to need to merge
+			 * any dfstab changes. This is done by
+			 * comparing the value of legacy-timestamp
+			 * with the current st_ctim of the file. If
+			 * they are different, an update is needed and
+			 * the file must remain locked until the merge
+			 * is done in order to prevent multiple
+			 * startups from changing the SMF repository
+			 * at the same time.  The first to get the
+			 * lock will make any changes before the
+			 * others can read the repository.
+			 */
+			prop = scf_simple_prop_get(scf_handle->handle,
+						(const char *)
+						    SA_SVC_FMRI_BASE ":default",
+						"operation",
+						"legacy-timestamp");
+			if (prop != NULL) {
+			    char *i64;
+			    i64 = scf_simple_prop_next_astring(prop);
+			    if (i64 != NULL) {
+				tval = strtoull(i64, NULL, 0);
+			    }
+			    if (stat(SA_LEGACY_DFSTAB, &st) >= 0 &&
+				tval != TSTAMP(st.st_ctim)) {
+				updatelegacy = B_TRUE;
+			    }
+			} else {
+			    /* We haven't set the timestamp before so do it. */
+			    updatelegacy = B_TRUE;
+			}
+		    }
+		    if (updatelegacy == B_FALSE) {
+			/* Don't need the lock anymore */
+			(void) lockf(lockfd, F_ULOCK, 0);
+			(void) close(lockfd);
+		    }
 		    (void) sa_get_config(scf_handle, &sa_config_tree,
 				    &sa_config_doc);
-		    tval = get_legacy_timestamp(sa_config_tree,
-						SA_LEGACY_DFSTAB);
+		    saunblocksigs(&old);
 		    if (tval == 0) {
 			/* first time so make sure default is setup */
 			sa_group_t defgrp;
@@ -573,13 +625,17 @@ sa_init(int init_service)
 				opt = sa_create_optionset(defgrp, "nfs");
 			}
 		    }
-		    if (stat(SA_LEGACY_DFSTAB, &st) >= 0 &&
-			tval != TSTAMP(st.st_ctim)) {
+		    if (updatelegacy == B_TRUE) {
+			sablocksigs(&old);
 			getlegacyconfig(SA_LEGACY_DFSTAB, &sa_config_tree);
 			if (stat(SA_LEGACY_DFSTAB, &st) >= 0)
 			    set_legacy_timestamp(sa_config_tree,
 						SA_LEGACY_DFSTAB,
 						TSTAMP(st.st_ctim));
+			saunblocksigs(&old);
+			/* Safe to unlock now to allow others to run */
+			(void) lockf(lockfd, F_ULOCK, 0);
+			(void) close(lockfd);
 		    }
 		    legacy |= sa_get_zfs_shares("zfs");
 		    legacy |= gettransients(&sa_config_tree);
diff --git a/usr/src/lib/libshare/common/libsharecore.c b/usr/src/lib/libshare/common/libsharecore.c
index a2ed24afca..4b5fdc46c0 100644
--- a/usr/src/lib/libshare/common/libsharecore.c
+++ b/usr/src/lib/libshare/common/libsharecore.c
@@ -96,6 +96,39 @@ extern int set_node_share(void *, char *, char *);
 extern void set_node_attr(void *, char *, char *);
 
 /*
+ * sablocksigs(*sigs)
+ *
+ * block important signals for a critical region. Arg is a pointer to
+ * a sigset_t that is used later for the unblock.
+ */
+void
+sablocksigs(sigset_t *sigs)
+{
+	sigset_t new;
+
+	if (sigs != NULL) {
+	    (void) sigprocmask(SIG_BLOCK, NULL, &new);
+	    (void) sigaddset(&new, SIGHUP);
+	    (void) sigaddset(&new, SIGINT);
+	    (void) sigaddset(&new, SIGQUIT);
+	    (void) sigaddset(&new, SIGTSTP);
+	    (void) sigprocmask(SIG_SETMASK, &new, sigs);
+	}
+}
+
+/*
+ * saunblocksigs(*sigs)
+ *
+ * unblock previously blocked signals from the sigs arg.
+ */
+void
+saunblocksigs(sigset_t *sigs)
+{
+	if (sigs != NULL)
+	    (void) sigprocmask(SIG_SETMASK, sigs, NULL);
+}
+
+/*
  * alloc_sharelist()
  *
  * allocator function to return an zfs_sharelist_t
@@ -259,9 +292,10 @@ getdfstab(FILE *dfs)
 			    item->resource = strdup(resource);
 		    }
 		}
+		if (item != NULL && item->fstype == NULL) {
+		    item->fstype = strdup("nfs"); /* this is the default */
+		}
 	    }
-	    if (item != NULL && item->fstype == NULL)
-		item->fstype = strdup("nfs"); /* this is the default */
 	}
 	first = fix_notice(first);
 	return (first);
@@ -500,17 +534,12 @@ sa_comment_line(char *line, char *err)
 {
 	FILE *dfstab;
 	xfs_sharelist_t *list;
-	sigset_t old, new;
+	sigset_t old;
 
 	dfstab = open_dfstab(SA_LEGACY_DFSTAB);
 	if (dfstab != NULL) {
 		(void) setvbuf(dfstab, NULL, _IOLBF, BUFSIZ * 8);
-		(void) sigprocmask(SIG_BLOCK, NULL, &new);
-		(void) sigaddset(&new, SIGHUP);
-		(void) sigaddset(&new, SIGINT);
-		(void) sigaddset(&new, SIGQUIT);
-		(void) sigaddset(&new, SIGTSTP);
-		(void) sigprocmask(SIG_SETMASK, &new, &old);
+		sablocksigs(&old);
 		(void) lockf(fileno(dfstab), F_LOCK, 0);
 		list = getdfstab(dfstab);
 		rewind(dfstab);
@@ -524,7 +553,7 @@ sa_comment_line(char *line, char *err)
 		(void) fsync(fileno(dfstab));
 		(void) lockf(fileno(dfstab), F_ULOCK, 0);
 		(void) fclose(dfstab);
-		(void) sigprocmask(SIG_SETMASK, &old, NULL);
+		saunblocksigs(&old);
 		if (list != NULL)
 		    dfs_free_list(list);
 	}
@@ -546,17 +575,12 @@ sa_delete_legacy(sa_share_t share)
 	char *path;
 	sa_optionset_t optionset;
 	sa_group_t parent;
-	sigset_t old, new;
+	sigset_t old;
 
 	dfstab = open_dfstab(SA_LEGACY_DFSTAB);
 	if (dfstab != NULL) {
 		(void) setvbuf(dfstab, NULL, _IOLBF, BUFSIZ * 8);
-		(void) sigprocmask(SIG_BLOCK, NULL, &new);
-		(void) sigaddset(&new, SIGHUP);
-		(void) sigaddset(&new, SIGINT);
-		(void) sigaddset(&new, SIGQUIT);
-		(void) sigaddset(&new, SIGTSTP);
-		(void) sigprocmask(SIG_SETMASK, &new, &old);
+		sablocksigs(&old);
 		path = sa_get_share_attr(share, "path");
 		parent = sa_get_parent_group(share);
 		if (parent != NULL) {
@@ -591,7 +615,7 @@ sa_delete_legacy(sa_share_t share)
 		    (void) lockf(fileno(dfstab), F_ULOCK, 0);
 		}
 		(void) fsync(fileno(dfstab));
-		(void) sigprocmask(SIG_SETMASK, &old, NULL);
+		saunblocksigs(&old);
 		(void) fclose(dfstab);
 		sa_free_attr_string(path);
 	} else {
@@ -624,7 +648,7 @@ sa_update_legacy(sa_share_t share, char *proto)
 	int ret = SA_OK;
 	xfs_sharelist_t *list;
 	char *path;
-	sigset_t old, new;
+	sigset_t old;
 	char *persist;
 
 	ret = sa_proto_update_legacy(proto, share);
@@ -640,12 +664,7 @@ sa_update_legacy(sa_share_t share, char *proto)
 	    dfstab = open_dfstab(SA_LEGACY_DFSTAB);
 	    if (dfstab != NULL) {
 		(void) setvbuf(dfstab, NULL, _IOLBF, BUFSIZ * 8);
-		(void) sigprocmask(SIG_BLOCK, NULL, &new);
-		(void) sigaddset(&new, SIGHUP);
-		(void) sigaddset(&new, SIGINT);
-		(void) sigaddset(&new, SIGQUIT);
-		(void) sigaddset(&new, SIGTSTP);
-		(void) sigprocmask(SIG_SETMASK, &new, &old);
+		sablocksigs(&old);
 		path = sa_get_share_attr(share, "path");
 		(void) lockf(fileno(dfstab), F_LOCK, 0);
 		list = getdfstab(dfstab);
@@ -657,7 +676,7 @@ sa_update_legacy(sa_share_t share, char *proto)
 		(void) fflush(dfstab);
 		(void) lockf(fileno(dfstab), F_ULOCK, 0);
 		(void) fsync(fileno(dfstab));
-		(void) sigprocmask(SIG_SETMASK, &old, NULL);
+		saunblocksigs(&old);
 		(void) fclose(dfstab);
 		sa_free_attr_string(path);
 		if (list != NULL)
@@ -1058,6 +1077,7 @@ parse_dfstab(char *dfstab, xmlNodePtr root)
 							    list->options,
 							    list->fstype);
 			}
+			sa_format_free(oldprops);
 		    }
 		}
 	    } else {
@@ -1168,6 +1188,7 @@ get_share_list(int *errp)
 
 	if ((fp = fopen(SHARETAB, "r")) != NULL) {
 		struct share	*sharetab_entry;
+		(void) lockf(fileno(fp), F_LOCK, 0);
 
 		while (getshare(fp, &sharetab_entry) > 0) {
 		    newp = alloc_sharelist();
@@ -1203,6 +1224,7 @@ get_share_list(int *errp)
 		    if (newp->description == NULL)
 			goto err;
 		}
+		(void) lockf(fileno(fp), F_ULOCK, 0);
 		(void) fclose(fp);
 	} else {
 	    *errp = errno;
@@ -1802,6 +1824,50 @@ emptyshare(struct share *sh)
 }
 
 /*
+ * checkshare(struct share *)
+ *
+ * If the share to write to sharetab is not present, need to add.  If
+ * the share is present, replace if options are different else we want
+ * to keep it.
+ * Return values:
+ *	1 - keep
+ *	2 - replace
+ * The CHK_NEW value isn't currently returned.
+ */
+#define	CHK_NEW		0
+#define	CHK_KEEP	1
+#define	CHK_REPLACE	2
+static int
+checkshare(struct share *sh)
+{
+	xfs_sharelist_t *list, *head;
+	int err;
+	int ret = CHK_NEW;
+
+	head = list = get_share_list(&err);
+	while (list != NULL && ret == CHK_NEW) {
+	    if (strcmp(sh->sh_path, list->path) == 0) {
+		/* Have the same path so check if replace or keep */
+		if (strcmp(sh->sh_opts, list->options) == 0)
+		    ret = CHK_KEEP;
+		else
+		    ret = CHK_REPLACE;
+	    }
+	    list = list->next;
+	}
+	if (head != NULL) {
+	    dfs_free_list(head);
+	}
+	/*
+	 * Just in case it was added by another process after our
+	 * scan, we always replace even if we think it is new.
+	 */
+	if (ret == CHK_NEW)
+	    ret = CHK_REPLACE;
+	return (ret);
+}
+
+/*
  * sa_update_sharetab(share, proto)
  *
  * Update the sharetab file with info from the specified share.
@@ -1816,7 +1882,8 @@ sa_update_sharetab(sa_share_t share, char *proto)
 	char *path;
 	int logging = 0;
 	FILE *sharetab;
-	sigset_t old, new;
+	sigset_t old;
+	int action;
 
 	path = sa_get_share_attr(share, "path");
 	if (path != NULL) {
@@ -1827,22 +1894,32 @@ sa_update_sharetab(sa_share_t share, char *proto)
 	    }
 	    if (sharetab != NULL) {
 		(void) setvbuf(sharetab, NULL, _IOLBF, BUFSIZ * 8);
-		(void) sigprocmask(SIG_BLOCK, NULL, &new);
-		(void) sigaddset(&new, SIGHUP);
-		(void) sigaddset(&new, SIGINT);
-		(void) sigaddset(&new, SIGQUIT);
-		(void) sigaddset(&new, SIGTSTP);
-		(void) sigprocmask(SIG_SETMASK, &new, &old);
-		(void) lockf(fileno(sharetab), F_LOCK, 0);
-		(void) remshare(sharetab, path, &logging);
-		/* fill in share structure and write it out */
+		sablocksigs(&old);
+		/*
+		 * Fill in share structure and write it out if the
+		 * share isn't already shared with the same options.
+		 */
 		(void) fillshare(share, proto, &shtab);
-		(void) putshare(sharetab, &shtab);
+		/*
+		 * If share is new or changed, remove the old,
+		 * otherwise keep it in place since it hasn't changed.
+		 */
+		action = checkshare(&shtab);
+		(void) lockf(fileno(sharetab), F_LOCK, 0);
+		switch (action) {
+		case CHK_REPLACE:
+		    (void) remshare(sharetab, path, &logging);
+		    (void) putshare(sharetab, &shtab);
+		    break;
+		case CHK_KEEP:
+		    /* Don't do anything */
+		    break;
+		}
 		emptyshare(&shtab);
 		(void) fflush(sharetab);
 		(void) lockf(fileno(sharetab), F_ULOCK, 0);
 		(void) fsync(fileno(sharetab));
-		(void) sigprocmask(SIG_SETMASK, &old, NULL);
+		saunblocksigs(&old);
 		(void) fclose(sharetab);
 	    } else {
 		if (errno == EACCES || errno == EPERM) {
@@ -1868,7 +1945,7 @@ sa_delete_sharetab(char *path, char *proto)
 	int ret = SA_OK;
 	int logging = 0;
 	FILE *sharetab;
-	sigset_t old, new;
+	sigset_t old;
 #ifdef lint
 	proto = proto;
 #endif
@@ -1880,18 +1957,13 @@ sa_delete_sharetab(char *path, char *proto)
 	    }
 	    if (sharetab != NULL) {
 		/* should block keyboard level signals around the lock */
-		(void) sigprocmask(SIG_BLOCK, NULL, &new);
-		(void) sigaddset(&new, SIGHUP);
-		(void) sigaddset(&new, SIGINT);
-		(void) sigaddset(&new, SIGQUIT);
-		(void) sigaddset(&new, SIGTSTP);
-		(void) sigprocmask(SIG_SETMASK, &new, &old);
+		sablocksigs(&old);
 		(void) lockf(fileno(sharetab), F_LOCK, 0);
 		ret = remshare(sharetab, path, &logging);
 		(void) fflush(sharetab);
 		(void) lockf(fileno(sharetab), F_ULOCK, 0);
 		(void) fsync(fileno(sharetab));
-		(void) sigprocmask(SIG_SETMASK, &old, NULL);
+		saunblocksigs(&old);
 		(void) fclose(sharetab);
 	    } else {
 		if (errno == EACCES || errno == EPERM) {