6490312 Passing invalid entries through sharenfs is no longer trapped

6490719 sharemgr and zfs does not get along well

3 files changed, 159 insertions, 32 deletions

diff --git a/usr/src/cmd/dfs.cmds/sharemgr/commands.c b/usr/src/cmd/dfs.cmds/sharemgr/commands.c
index ce70a92386..2998b16068 100644
--- a/usr/src/cmd/dfs.cmds/sharemgr/commands.c
+++ b/usr/src/cmd/dfs.cmds/sharemgr/commands.c
@@ -3721,39 +3721,43 @@ sa_legacy_share(int flags, int argc, char *argv[])
 			    }
 			}
 		    } else {
+			char *type;
 			/*
 			 * may want to change persist state, but the
-			 * important thing is to change options unless
-			 * this is ZFS where we just want to do the
-			 * enable since everything is current.
+			 * important thing is to change options. We
+			 * need to change them regardless of the
+			 * source.
 			 */
-			if (!sa_zfs_is_shared(sharepath)) {
-			    char *type;
-			    remove_all_options(share, protocol);
-			    type = sa_get_share_attr(share, "type");
-			    if (type != NULL &&
-				strcmp(type, "transient") != 0) {
-				curtype = SA_SHARE_PERMANENT;
-			    }
-			    if (type != NULL)
-				sa_free_attr_string(type);
-			    if (curtype != persist) {
-				(void) sa_set_share_attr(share, "type",
+			if (sa_zfs_is_shared(sharepath)) {
+			    zfs = 1;
+			}
+			remove_all_options(share, protocol);
+			type = sa_get_share_attr(share, "type");
+			if (type != NULL &&
+			    strcmp(type, "transient") != 0) {
+			    curtype = SA_SHARE_PERMANENT;
+			}
+			if (type != NULL)
+			    sa_free_attr_string(type);
+			if (curtype != persist) {
+			    (void) sa_set_share_attr(share, "type",
 					persist == SA_SHARE_PERMANENT ?
 						"persist" : "transient");
-			    }
-			} else {
-			    zfs++;
 			}
 		    }
+		    /* have a group to hold this share path */
+		    if (ret == SA_OK && options != NULL &&
+			strlen(options) > 0) {
+			ret = sa_parse_legacy_options(share,
+							options,
+							protocol);
+		    }
 		    if (!zfs) {
-			/* have a group to hold this share path */
-			if (ret == SA_OK && options != NULL &&
-				strlen(options) > 0) {
-			    ret = sa_parse_legacy_options(share,
-							    options,
-							    protocol);
-			}
+			/*
+			 * zfs shares never have resource or
+			 * description and we can't store the values
+			 * so don't try.
+			 */
 			if (ret == SA_OK && description != NULL)
 			    ret = sa_set_share_description(share, description);
 			if (ret == SA_OK && resource != NULL)
diff --git a/usr/src/lib/libshare/common/libshare_zfs.c b/usr/src/lib/libshare/common/libshare_zfs.c
index 0a8f327edd..26bf9a8f9a 100644
--- a/usr/src/lib/libshare/common/libshare_zfs.c
+++ b/usr/src/lib/libshare/common/libshare_zfs.c
@@ -152,8 +152,8 @@ sa_zfs_is_shared(char *path)
  * group. If the group doesn't exist, create it first, making sure it
  * is marked as a ZFS group.
  *
- * Not that all ZFS shares are in a subgroup of the top level group
- * "zfs".
+ * Note that all ZFS shares are in a subgroup of the top level group
+ * called "zfs".
  */
 
 static sa_group_t
@@ -172,6 +172,8 @@ find_or_create_group(char *groupname, char *proto, int *err)
 	group = sa_get_group(groupname);
 	if (group == NULL) {
 	    group = sa_create_group(groupname, &ret);
+
+	    /* make sure this is flagged as a ZFS group */
 	    if (group != NULL)
 		ret = sa_set_group_attr(group, "zfs", "true");
 	}
@@ -198,6 +200,64 @@ find_or_create_group(char *groupname, char *proto, int *err)
 }
 
 /*
+ * find_or_create_zfs_subgroup(groupname, optstring, *err)
+ *
+ * ZFS shares will be in a subgroup of the "zfs" master group.  This
+ * function looks to see if the groupname exists and returns it if it
+ * does or else creates a new one with the specified name and returns
+ * that.  The "zfs" group will exist before we get here, but we make
+ * sure just in case.
+ *
+ * err must be a valid pointer.
+ */
+
+static sa_group_t
+find_or_create_zfs_subgroup(char *groupname, char *optstring, int *err)
+{
+	sa_group_t group = NULL;
+	sa_group_t zfs;
+	char *name;
+	char *options;
+
+	/* start with the top-level "zfs" group */
+	zfs = sa_get_group("zfs");
+	*err = SA_OK;
+	if (zfs != NULL) {
+	    for (group = sa_get_sub_group(zfs); group != NULL;
+		group = sa_get_next_group(group)) {
+		name = sa_get_group_attr(group, "name");
+		if (name != NULL && strcmp(name, groupname) == 0) {
+		    /* have the group so break out of here */
+		    sa_free_attr_string(name);
+		    break;
+		}
+		if (name != NULL)
+		    sa_free_attr_string(name);
+	    }
+
+	    if (group == NULL) {
+		/* need to create the sub-group since it doesn't exist */
+		group = _sa_create_zfs_group(zfs, groupname);
+		if (group != NULL) {
+		    set_node_attr(group, "zfs", "true");
+		}
+		if (strcmp(optstring, "on") == 0)
+		    optstring = "rw";
+		if (group != NULL) {
+		    options = strdup(optstring);
+		    if (options != NULL) {
+			*err = sa_parse_legacy_options(group, options, "nfs");
+			free(options);
+		    } else {
+			*err = SA_NO_MEMORY;
+		    }
+		}
+	    }
+	}
+	return (group);
+}
+
+/*
  * sa_get_zfs_shares(groupname)
  *
  * Walk the mnttab for all zfs mounts and determine which are
@@ -223,6 +283,7 @@ sa_get_zfs_shares(char *groupname)
 	zfs_source_t source;
 	char sourcestr[ZFS_MAXPROPLEN];
 	libzfs_handle_t *libhandle;
+	char *options;
 
 	/*
 	 * if we can't access libzfs, don't bother doing anything.
@@ -269,10 +330,46 @@ sa_get_zfs_shares(char *groupname)
 			}
 			if (err == SA_OK) {
 			    if (source & ZFS_SRC_INHERITED) {
-				share = _sa_add_share(group, cur->mountp,
+				int doshopt = 0;
+				/*
+				 * Need to find the "real" parent
+				 * sub-group. It may not be mounted,
+				 * but it was identified in the
+				 * "sourcestr" variable. The real
+				 * parent not mounted can occur if
+				 * "canmount=off and sharenfs=on".
+				 */
+				group = find_or_create_zfs_subgroup(sourcestr,
+								    shareopts,
+								    &doshopt);
+				if (group != NULL) {
+				    share = _sa_add_share(group, cur->mountp,
 							SA_SHARE_TRANSIENT,
 							&err);
+					/*
+					 * some options may only be on
+					 * shares. If the opt string
+					 * contains one of those, we
+					 * put it just on the share.
+					 */
+				    if (share != NULL &&
+					doshopt == SA_PROP_SHARE_ONLY) {
+					options = strdup(shareopts);
+					if (options != NULL) {
+					    err = sa_parse_legacy_options(share,
+								options, "nfs");
+					    free(options);
+					}
+				    }
+				} else {
+				    err = SA_NO_MEMORY;
+				}
 			    } else {
+				/*
+				 * this is a sub-group that actually
+				 * contains the sharenfs property as
+				 * opposed to a share that inherited.
+				 */
 				group = _sa_create_zfs_group(zfsgroup,
 								cur->resource);
 				set_node_attr(group, "zfs", "true");
@@ -280,7 +377,6 @@ sa_get_zfs_shares(char *groupname)
 							SA_SHARE_TRANSIENT,
 							&err);
 				if (err == SA_OK) {
-				    char *options;
 				    if (strcmp(shareopts, "on") != 0) {
 					options = strdup(shareopts);
 					if (options != NULL) {
@@ -289,6 +385,26 @@ sa_get_zfs_shares(char *groupname)
 									"nfs");
 					    free(options);
 					}
+					if (err == SA_PROP_SHARE_ONLY) {
+					/*
+					 * Same as above, some
+					 * properties may only be on
+					 * shares, but due to the ZFS
+					 * sub-groups being
+					 * artificial, we sometimes
+					 * get this and have to deal
+					 * with it. We do it by
+					 * attempting to put it on the
+					 * share.
+					 */
+					    options = strdup(shareopts);
+					    if (options != NULL)
+						err = sa_parse_legacy_options(
+									share,
+									options,
+									"nfs");
+					    free(options);
+					}
 					/* unmark the share's changed state */
 					set_node_attr(share, "changed", NULL);
 				    }
diff --git a/usr/src/lib/libshare/common/libsharecore.c b/usr/src/lib/libshare/common/libsharecore.c
index 4e48c9fede..000b3bd3c2 100644
--- a/usr/src/lib/libshare/common/libsharecore.c
+++ b/usr/src/lib/libshare/common/libsharecore.c
@@ -1180,9 +1180,16 @@ parse_sharetab(void)
 			share = _sa_add_share(group, tmplist->path,
 						SA_SHARE_TRANSIENT, &err);
 		    } else {
-			(void) printf(gettext("Group for temporary share"
-						"not found: %s\n"),
-					tmplist->path);
+			/*
+			 * While this case shouldn't occur very often,
+			 * it does occur out of a "zfs set
+			 * sharenfs=off" when the dataset is also set
+			 * to canmount=off. A warning will then cause
+			 * the zfs command to abort. Since we add it
+			 * to the default list, everything works
+			 * properly anyway and the library doesn't
+			 * need to give a warning.
+			 */
 			share = _sa_add_share(lgroup, tmplist->path,
 						SA_SHARE_TRANSIENT, &err);
 		    }