6604618 sharemgr can validate options against NULL optionset, have bogus result

8 files changed, 62 insertions, 36 deletions

diff --git a/usr/src/lib/libshare/common/libshare.c b/usr/src/lib/libshare/common/libshare.c
index 1dcb43f903..3ba127b807 100644
--- a/usr/src/lib/libshare/common/libshare.c
+++ b/usr/src/lib/libshare/common/libshare.c
@@ -69,7 +69,6 @@ extern struct sa_proto_plugin *sap_proto_list;
 /* current SMF/SVC repository handle */
 extern void getlegacyconfig(sa_handle_t, char *, xmlNodePtr *);
 extern int gettransients(sa_handle_impl_t, xmlNodePtr *);
-extern int sa_valid_property(void *, char *, sa_property_t);
 extern char *sa_fstype(char *);
 extern int sa_is_share(void *);
 extern int sa_is_resource(void *);
@@ -3201,11 +3200,15 @@ sa_add_property(void *object, sa_property_t property)
 	sa_group_t parent;
 	sa_group_t group;
 	char *proto;
+	sa_handle_t handle;
 
-	proto = sa_get_optionset_attr(object, "type");
 	if (property != NULL) {
-		if ((ret = sa_valid_property(object, proto, property)) ==
-		    SA_OK) {
+		handle = sa_find_group_handle((sa_group_t)object);
+		if (handle == NULL)
+			return (SA_CONFIG_ERR);
+		proto = sa_get_optionset_attr(object, "type");
+		if ((ret = sa_valid_property(handle, object, proto,
+		    property)) == SA_OK) {
 			property = (sa_property_t)xmlAddChild(
 			    (xmlNodePtr)object, (xmlNodePtr)property);
 		} else {
@@ -3213,10 +3216,10 @@ sa_add_property(void *object, sa_property_t property)
 				sa_free_attr_string(proto);
 			return (ret);
 		}
+		if (proto != NULL)
+			sa_free_attr_string(proto);
 	}
 
-	if (proto != NULL)
-		sa_free_attr_string(proto);
 
 	parent = sa_get_parent_group(object);
 	if (!sa_is_persistent(parent))
diff --git a/usr/src/lib/libshare/common/libshare.h b/usr/src/lib/libshare/common/libshare.h
index f70b902af9..a560b7731f 100644
--- a/usr/src/lib/libshare/common/libshare.h
+++ b/usr/src/lib/libshare/common/libshare.h
@@ -222,7 +222,7 @@ extern int sa_add_property(void *, sa_property_t);
 extern int sa_update_property(sa_property_t, char *);
 extern int sa_remove_property(sa_property_t);
 extern int sa_commit_properties(sa_optionset_t, int);
-extern int sa_valid_property(void *, char *, sa_property_t);
+extern int sa_valid_property(sa_handle_t, void *, char *, sa_property_t);
 extern int sa_is_persistent(void *);
 
 /* security control */
@@ -251,7 +251,8 @@ extern char *sa_get_protocol_status(char *);
 extern void sa_format_free(char *);
 extern sa_protocol_properties_t sa_create_protocol_properties(char *);
 extern int sa_add_protocol_property(sa_protocol_properties_t, sa_property_t);
-extern int sa_proto_valid_prop(char *, sa_property_t, sa_optionset_t);
+extern int sa_proto_valid_prop(sa_handle_t, char *, sa_property_t,
+    sa_optionset_t);
 extern int sa_proto_valid_space(char *, char *);
 extern char *sa_proto_space_alias(char *, char *);
 extern int sa_proto_get_transients(sa_handle_t, char *);
diff --git a/usr/src/lib/libshare/common/libshare_impl.h b/usr/src/lib/libshare/common/libshare_impl.h
index 96ddf90f85..256deb1e04 100644
--- a/usr/src/lib/libshare/common/libshare_impl.h
+++ b/usr/src/lib/libshare/common/libshare_impl.h
@@ -61,7 +61,8 @@ struct sa_plugin_ops {
 	void	(*sa_fini)();
 	int	(*sa_share)(sa_share_t);	/* start sharing */
 	int	(*sa_unshare)(sa_share_t, char *);	/* stop sharing */
-	int	(*sa_valid_prop)(sa_property_t, sa_optionset_t); /* validate */
+	int	(*sa_valid_prop)(sa_handle_t, sa_property_t,
+	    sa_optionset_t); /* validate */
 	int	(*sa_valid_space)(char *);	/* is name valid optionspace? */
 	int	(*sa_security_prop)(char *);	/* property is security */
 	int	(*sa_legacy_opts)(sa_group_t, char *); /* parse legacy opts */
@@ -115,7 +116,8 @@ typedef struct sa_handle_impl {
 
 extern int sa_proto_share(char *, sa_share_t);
 extern int sa_proto_unshare(sa_share_t, char *, char *);
-extern int sa_proto_valid_prop(char *, sa_property_t, sa_optionset_t);
+extern int sa_proto_valid_prop(sa_handle_t, char *, sa_property_t,
+    sa_optionset_t);
 extern int sa_proto_security_prop(char *, char *);
 extern int sa_proto_legacy_opts(char *, sa_group_t, char *);
 extern int sa_proto_share_resource(char *, sa_resource_t);
diff --git a/usr/src/lib/libshare/common/libsharecore.c b/usr/src/lib/libshare/common/libsharecore.c
index 3489f1344f..5c92ce4c7c 100644
--- a/usr/src/lib/libshare/common/libsharecore.c
+++ b/usr/src/lib/libshare/common/libsharecore.c
@@ -1626,19 +1626,20 @@ update_legacy_config(sa_handle_t handle)
 }
 
 /*
- * sa_valid_property(object, proto, property)
+ * sa_valid_property(handle, object, proto, property)
  *
  * check to see if the specified property is valid relative to the
  * specified protocol. The protocol plugin is called to do the work.
  */
 
 int
-sa_valid_property(void *object, char *proto, sa_property_t property)
+sa_valid_property(sa_handle_t handle, void *object, char *proto,
+    sa_property_t property)
 {
 	int ret = SA_OK;
 
 	if (proto != NULL && property != NULL) {
-		ret = sa_proto_valid_prop(proto, property, object);
+		ret = sa_proto_valid_prop(handle, proto, property, object);
 	}
 
 	return (ret);
diff --git a/usr/src/lib/libshare/common/plugin.c b/usr/src/lib/libshare/common/plugin.c
index fea981cce4..c2082b8a1a 100644
--- a/usr/src/lib/libshare/common/plugin.c
+++ b/usr/src/lib/libshare/common/plugin.c
@@ -305,19 +305,20 @@ sa_proto_unshare_resource(char *proto, sa_resource_t resource)
 }
 
 /*
- * sa_proto_valid_prop(proto, prop, opt)
+ * sa_proto_valid_prop(handle, proto, prop, opt)
  *
  * Check to see if the specified prop is valid for this protocol.
  */
 
 int
-sa_proto_valid_prop(char *proto, sa_property_t prop, sa_optionset_t opt)
+sa_proto_valid_prop(sa_handle_t handle, char *proto, sa_property_t prop,
+    sa_optionset_t opt)
 {
 	struct sa_plugin_ops *ops = find_protocol(proto);
 	int ret = 0;
 
 	if (ops != NULL && ops->sa_valid_prop != NULL)
-		ret = ops->sa_valid_prop(prop, opt);
+		ret = ops->sa_valid_prop(handle, prop, opt);
 	return (ret);
 }
 
diff --git a/usr/src/lib/libshare/nfs/libshare_nfs.c b/usr/src/lib/libshare/nfs/libshare_nfs.c
index bafaf08f10..8ddc74e1a0 100644
--- a/usr/src/lib/libshare/nfs/libshare_nfs.c
+++ b/usr/src/lib/libshare/nfs/libshare_nfs.c
@@ -64,7 +64,7 @@ static int nfs_init();
 static void nfs_fini();
 static int nfs_enable_share(sa_share_t);
 static int nfs_disable_share(sa_share_t, char *);
-static int nfs_validate_property(sa_property_t, sa_optionset_t);
+static int nfs_validate_property(sa_handle_t, sa_property_t, sa_optionset_t);
 static int nfs_validate_security_mode(char *);
 static int nfs_is_security_opt(char *);
 static int nfs_parse_legacy_options(sa_group_t, char *);
@@ -1592,26 +1592,26 @@ check_public(sa_group_t group, sa_share_t skipshare)
 }
 
 /*
- * public_exists(share)
+ * public_exists(handle, share)
  *
  * check to see if public option is set on any other share than the
  * one specified. Need to check zfs sub-groups as well as the top
  * level groups.
  */
 static int
-public_exists(sa_share_t skipshare)
+public_exists(sa_handle_t handle, sa_share_t skipshare)
 {
-	sa_group_t group;
-	sa_handle_t handle;
-
-	group = sa_get_parent_group(skipshare);
-	if (group == NULL)
-		return (SA_NO_SUCH_GROUP);
+	sa_group_t group = NULL;
 
-	handle = sa_find_group_handle(group);
 	if (handle == NULL)
 		return (SA_SYSTEM_ERR);
 
+	if (skipshare != NULL) {
+		group = sa_get_parent_group(skipshare);
+		if (group == NULL)
+			return (SA_NO_SUCH_GROUP);
+	}
+
 	for (group = sa_get_group(handle, NULL); group != NULL;
 	    group = sa_get_next_group(group)) {
 		/* Walk any ZFS subgroups as well as all standard groups */
@@ -1652,6 +1652,7 @@ nfs_enable_share(sa_share_t share)
 	int err = SA_OK;
 	int i;
 	int iszfs;
+	sa_handle_t handle;
 
 	/* Don't drop core if the NFS module isn't loaded. */
 	(void) signal(SIGSYS, SIG_IGN);
@@ -1700,7 +1701,8 @@ nfs_enable_share(sa_share_t share)
 	 * no other share has it set. If it is already used, fail.
 	 */
 
-	if (export.ex_flags & EX_PUBLIC && public_exists(share)) {
+	handle = sa_find_group_handle((sa_group_t)share);
+	if (export.ex_flags & EX_PUBLIC && public_exists(handle, share)) {
 		(void) printf(dgettext(TEXT_DOMAIN,
 		    "NFS: Cannot share more than one file "
 		    "system with 'public' property\n"));
@@ -1986,13 +1988,14 @@ check_rorw(char *v1, char *v2)
 }
 
 /*
- * nfs_validate_property(property, parent)
+ * nfs_validate_property(handle, property, parent)
  *
  * Check that the property has a legitimate value for its type.
  */
 
 static int
-nfs_validate_property(sa_property_t property, sa_optionset_t parent)
+nfs_validate_property(sa_handle_t handle, sa_property_t property,
+    sa_optionset_t parent)
 {
 	int ret = SA_OK;
 	char *propname;
@@ -2011,10 +2014,22 @@ nfs_validate_property(sa_property_t property, sa_optionset_t parent)
 
 	if (ret == SA_OK) {
 		parent_group = sa_get_parent_group((sa_share_t)parent);
-		if (optdefs[optindex].share && !sa_is_share(parent_group))
+		if (optdefs[optindex].share && parent_group != NULL &&
+		    !sa_is_share(parent_group))
 			ret = SA_PROP_SHARE_ONLY;
 	}
 	if (ret == SA_OK) {
+		if (optdefs[optindex].index == OPT_PUBLIC) {
+			/*
+			 * Public is special in that only one instance can
+			 * be in the repository at the same time.
+			 */
+			if (public_exists(handle, parent_group)) {
+				if (propname != NULL)
+					sa_free_attr_string(propname);
+				return (SA_VALUE_CONFLICT);
+			}
+		}
 		value = sa_get_property_attr(property, "value");
 		if (value != NULL) {
 			/* first basic type checking */
@@ -2137,7 +2152,7 @@ nfs_validate_property(sa_property_t property, sa_optionset_t parent)
 
 			if (ret == SA_OK && optdefs[optindex].check != NULL) {
 				/* do the property specific check */
-				ret = optdefs[optindex].check(property);
+				ret = optdefs[optindex].check(handle, property);
 			}
 		}
 	}
diff --git a/usr/src/lib/libshare/nfs/libshare_nfs.h b/usr/src/lib/libshare/nfs/libshare_nfs.h
index 6e748ea897..0da882f435 100644
--- a/usr/src/lib/libshare/nfs/libshare_nfs.h
+++ b/usr/src/lib/libshare/nfs/libshare_nfs.h
@@ -20,7 +20,7 @@
  */
 
 /*
- * Copyright 2006 Sun Microsystems, Inc.  All rights reserved.
+ * Copyright 2008 Sun Microsystems, Inc.  All rights reserved.
  * Use is subject to license terms.
  */
 
@@ -83,7 +83,7 @@ struct option_defs {
 	int index;
 	int type;
 	int share;	/* share only option */
-	int (*check)(char *);
+	int (*check)(sa_handle_t, char *);
 };
 
 /*
diff --git a/usr/src/lib/libshare/smb/libshare_smb.c b/usr/src/lib/libshare/smb/libshare_smb.c
index 8bdea1652a..fac051c1ea 100644
--- a/usr/src/lib/libshare/smb/libshare_smb.c
+++ b/usr/src/lib/libshare/smb/libshare_smb.c
@@ -62,7 +62,7 @@ static int smb_share_changed(sa_share_t);
 static int smb_resource_changed(sa_resource_t);
 static int smb_rename_resource(sa_handle_t, sa_resource_t, char *);
 static int smb_disable_share(sa_share_t share, char *);
-static int smb_validate_property(sa_property_t, sa_optionset_t);
+static int smb_validate_property(sa_handle_t, sa_property_t, sa_optionset_t);
 static int smb_set_proto_prop(sa_property_t);
 static sa_protocol_properties_t smb_get_proto_set(void);
 static char *smb_get_status(void);
@@ -699,13 +699,16 @@ done:
 }
 
 /*
- * smb_validate_property(property, parent)
+ * smb_validate_property(handle, property, parent)
  *
  * Check that the property has a legitimate value for its type.
+ * Handle isn't currently used but may need to be in the future.
  */
 
+/*ARGSUSED*/
 static int
-smb_validate_property(sa_property_t property, sa_optionset_t parent)
+smb_validate_property(sa_handle_t handle, sa_property_t property,
+    sa_optionset_t parent)
 {
 	int ret = SA_OK;
 	char *propname;