From 570d9af5ceed18e600bbf040e48752f923718a01 Mon Sep 17 00:00:00 2001

From: Mark Reynolds <mreynolds@redhat.com>

Date: Tue, 4 Aug 2015 12:19:31 -0400

Subject: [PATCH 81/84] Ticket 47931 - memberOf & retrocl deadlocks

Bug Description:  When concurrently updating multiple backends the

                  memberOf and retrocl plugins can deadlock on each

                  other.  This is caused by the required retrocl lock,

                  and the db lock on the changenumber index in the

                  retrocl db.

Fix Description:  Added scoping to the retrocl that allows subtrees/suffixes

                  to be included or excluded.  Also moved the existing

                  memberOf scoping outside of its global lock.

                  Also improved the memberOf config copying to be consistent

                  and more efficient.  Improved the memberOf scoping attributes

                  to be multivalued.  And, properly valdiated new config

                  settings in the preop valdiation function, instead of the

                  "apply config" function.

https://fedorahosted.org/389/ticket/47931

Valgrind: passed

Reviewed by: nhosoi(Thanks!)

(cherry picked from commit fd959ac864d6d86d24928bc2c6f097d1a6031ecd)

(cherry picked from commit d8108476d3bedbcc03f6c61bfb3d50e921faaf42)

(cherry picked from commit 9ba3240a177c156e365f22c721432321bb0a679e)

---

 ldap/servers/plugins/memberof/memberof.c        | 247 ++++++++++++++---------

 ldap/servers/plugins/memberof/memberof.h        |   8 +-

 ldap/servers/plugins/memberof/memberof_config.c | 253 +++++++++++++++++-------

 ldap/servers/plugins/retrocl/retrocl.c          | 183 +++++++++++++++--

 ldap/servers/plugins/retrocl/retrocl.h          |   4 +

 ldap/servers/plugins/retrocl/retrocl_po.c       |  41 +++-

 6 files changed, 536 insertions(+), 200 deletions(-)

diff --git a/ldap/servers/plugins/memberof/memberof.c b/ldap/servers/plugins/memberof/memberof.c

index 14bad98..1840e34 100644

--- a/ldap/servers/plugins/memberof/memberof.c

+++ b/ldap/servers/plugins/memberof/memberof.c

@@ -148,7 +148,7 @@ static int memberof_compare(MemberOfConfig *config, const void *a, const void *b

 static int memberof_qsort_compare(const void *a, const void *b);

 static void memberof_load_array(Slapi_Value **array, Slapi_Attr *attr);

 static int memberof_del_dn_from_groups(Slapi_PBlock *pb, MemberOfConfig *config, Slapi_DN *sdn);

-static int memberof_call_foreach_dn(Slapi_PBlock *pb, Slapi_DN *sdn,

+static int memberof_call_foreach_dn(Slapi_PBlock *pb, Slapi_DN *sdn, MemberOfConfig *config,

 	char **types, plugin_search_entry_callback callback,  void *callback_data);

 static int memberof_is_direct_member(MemberOfConfig *config, Slapi_Value *groupdn,

 	Slapi_Value *memberdn);

@@ -176,7 +176,7 @@ static const char *fetch_attr(Slapi_Entry *e, const char *attrname,

 static void memberof_fixup_task_thread(void *arg);

 static int memberof_fix_memberof(MemberOfConfig *config, char *dn, char *filter_str);

 static int memberof_fix_memberof_callback(Slapi_Entry *e, void *callback_data);

-

+static int memberof_entry_in_scope(MemberOfConfig *config, Slapi_DN *sdn);

 /*** implementation ***/

@@ -521,7 +521,8 @@ memberof_get_plugin_area()

 int memberof_postop_del(Slapi_PBlock *pb)

 {

 	int ret = SLAPI_PLUGIN_SUCCESS;

-	MemberOfConfig configCopy = {0, 0, 0, 0};

+	MemberOfConfig *mainConfig = NULL;

+	MemberOfConfig configCopy = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0};

 	Slapi_DN *sdn;

 	void *caller_id = NULL;

@@ -541,12 +542,13 @@ int memberof_postop_del(Slapi_PBlock *pb)

 		struct slapi_entry *e = NULL;

 		slapi_pblock_get( pb, SLAPI_ENTRY_PRE_OP, &e );

-

-		/* We need to get the config lock first.  Trying to get the

-		 * config lock after we already hold the op lock can cause

-		 * a deadlock. */

 		memberof_rlock_config();

-		/* copy config so it doesn't change out from under us */

+		mainConfig = memberof_get_config();

+		if(!memberof_entry_in_scope(mainConfig, slapi_entry_get_sdn(e))){

+			/* The entry is not in scope, bail...*/

+			memberof_unlock_config();

+			goto bail;

+		}

 		memberof_copy_config(&configCopy, memberof_get_config());

 		memberof_unlock_config();

@@ -561,7 +563,6 @@ int memberof_postop_del(Slapi_PBlock *pb)

 			                "memberof_postop_del: error deleting dn (%s) from group. Error (%d)\n",

 			                slapi_sdn_get_dn(sdn),ret);

 			memberof_unlock();

-			memberof_free_config(&configCopy);

 			goto bail;

 		}

@@ -586,10 +587,10 @@ int memberof_postop_del(Slapi_PBlock *pb)

 			}

 		}

 		memberof_unlock();

+bail:

 		memberof_free_config(&configCopy);

 	}

-bail:

 	if(ret){

 		slapi_pblock_set(pb, SLAPI_RESULT_CODE, &ret;;

 		ret = SLAPI_PLUGIN_FAILURE;

@@ -623,7 +624,7 @@ memberof_del_dn_from_groups(Slapi_PBlock *pb, MemberOfConfig *config, Slapi_DN *

 		groupattrs[0] = config->groupattrs[i];

-		rc = memberof_call_foreach_dn(pb, sdn, groupattrs,

+		rc = memberof_call_foreach_dn(pb, sdn, config, groupattrs,

 		                              memberof_del_dn_type_callback, &data);

 	}

@@ -673,6 +674,20 @@ memberof_del_dn_type_callback(Slapi_Entry *e, void *callback_data)

 	return rc;

 }

+/* Check if the the entry include scope is a child of the sdn */

+static Slapi_DN*

+memberof_scope_is_child_of_dn(MemberOfConfig *config, Slapi_DN *sdn)

+{

+	int i = 0;

+

+	while(config->entryScopes && config->entryScopes[i]){

+		if(slapi_sdn_issuffix(config->entryScopes[i], sdn)){

+			return config->entryScopes[i];

+		}

+		i++;

+	}

+	return NULL;

+}

 /*

  * Does a callback search of "type=dn" under the db suffix that "dn" is in,

  * unless all_backends is set, then we look at all the backends.  If "dn"

@@ -681,7 +696,7 @@ memberof_del_dn_type_callback(Slapi_Entry *e, void *callback_data)

  */

 int

 memberof_call_foreach_dn(Slapi_PBlock *pb, Slapi_DN *sdn,

-	char **types, plugin_search_entry_callback callback, void *callback_data)

+	MemberOfConfig *config, char **types, plugin_search_entry_callback callback, void *callback_data)

 {

 	Slapi_PBlock *search_pb = NULL;

 	Slapi_DN *base_sdn = NULL;

@@ -689,9 +704,7 @@ memberof_call_foreach_dn(Slapi_PBlock *pb, Slapi_DN *sdn,

 	char *escaped_filter_val;

 	char *filter_str = NULL;

 	char *cookie = NULL;

-	int all_backends = memberof_config_get_all_backends();

-	Slapi_DN *entry_scope = memberof_config_get_entry_scope();

-        Slapi_DN *entry_scope_exclude_subtree = memberof_config_get_entry_scope_exclude_subtree();

+	int all_backends = config->allBackends;

 	int types_name_len = 0;

 	int num_types = 0;

 	int dn_len = slapi_sdn_get_ndn_len(sdn);

@@ -699,11 +712,7 @@ memberof_call_foreach_dn(Slapi_PBlock *pb, Slapi_DN *sdn,

 	int rc = 0;

 	int i = 0;

-	if (entry_scope && !slapi_sdn_issuffix(sdn, entry_scope)) {

-		return (rc);

-	}

-        

-        if (entry_scope_exclude_subtree && slapi_sdn_issuffix(sdn, entry_scope_exclude_subtree)) {

+	if (!memberof_entry_in_scope(config, sdn)) {

 		return (rc);

 	}

@@ -760,6 +769,8 @@ memberof_call_foreach_dn(Slapi_PBlock *pb, Slapi_DN *sdn,

 	search_pb = slapi_pblock_new();

 	be = slapi_get_first_backend(&cookie);

 	while(be){

+		Slapi_DN *scope_sdn = NULL;

+

 		if(!all_backends){

 			be = slapi_be_select(sdn);

 			if(be == NULL){

@@ -775,13 +786,14 @@ memberof_call_foreach_dn(Slapi_PBlock *pb, Slapi_DN *sdn,

 				continue;

 			}

 		}

-		if (entry_scope) {

-			if (slapi_sdn_issuffix(base_sdn, entry_scope)) {

+

+		if (config->entryScopes || config->entryScopeExcludeSubtrees) {

+			if (memberof_entry_in_scope(config, base_sdn)) {

 				/* do nothing, entry scope is spanning 

 				 * multiple suffixes, start at suffix */

-			} else if (slapi_sdn_issuffix(entry_scope, base_sdn)) {

+			} else if ((scope_sdn = memberof_scope_is_child_of_dn(config, base_sdn))) {

 				/* scope is below suffix, set search base */

-				base_sdn = entry_scope;

+				base_sdn = scope_sdn;

 			} else if(!all_backends){

 				break;

 			} else {

@@ -799,7 +811,6 @@ memberof_call_foreach_dn(Slapi_PBlock *pb, Slapi_DN *sdn,

 			break;

 		}

-

 		if(!all_backends){

 			break;

 		}

@@ -824,10 +835,7 @@ int memberof_postop_modrdn(Slapi_PBlock *pb)

 {

 	int ret = SLAPI_PLUGIN_SUCCESS;

 	void *caller_id = NULL;

-	Slapi_DN *entry_scope = NULL;

-        Slapi_DN *entry_scope_exclude_subtree = memberof_config_get_entry_scope_exclude_subtree();

-	entry_scope = memberof_config_get_entry_scope();

 	slapi_log_error( SLAPI_LOG_TRACE, MEMBEROF_PLUGIN_SUBSYSTEM,

 		     "--> memberof_postop_modrdn\n" );

@@ -842,7 +850,7 @@ int memberof_postop_modrdn(Slapi_PBlock *pb)

 	if(memberof_oktodo(pb))

 	{

 		MemberOfConfig *mainConfig = 0;

-		MemberOfConfig configCopy = {0, 0, 0, 0};

+		MemberOfConfig configCopy = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0};

 		struct slapi_entry *pre_e = NULL;

 		struct slapi_entry *post_e = NULL;

 		Slapi_DN *pre_sdn = 0;

@@ -850,7 +858,6 @@ int memberof_postop_modrdn(Slapi_PBlock *pb)

 		slapi_pblock_get( pb, SLAPI_ENTRY_PRE_OP, &pre_e );

 		slapi_pblock_get( pb, SLAPI_ENTRY_POST_OP, &post_e );

-		

 		if(pre_e && post_e)

 		{

 			pre_sdn = slapi_entry_get_sdn(pre_e);

@@ -863,11 +870,19 @@ int memberof_postop_modrdn(Slapi_PBlock *pb)

 		memberof_copy_config(&configCopy, mainConfig);

 		memberof_unlock_config();

+		/* Need to check both the pre/post entries */

+		if((pre_sdn && !memberof_entry_in_scope(&configCopy, pre_sdn)) &&

+		   (post_sdn && !memberof_entry_in_scope(&configCopy, post_sdn)))

+		{

+			/* The entry is not in scope */

+			goto bail;

+		}

+

 		memberof_lock();

 		/*  update any downstream members */

 		if(pre_sdn && post_sdn && configCopy.group_filter &&

-			0 == slapi_filter_test_simple(post_e, configCopy.group_filter))

+		   0 == slapi_filter_test_simple(post_e, configCopy.group_filter))

 		{

 			int i = 0;

 			Slapi_Attr *attr = 0;

@@ -879,7 +894,7 @@ int memberof_postop_modrdn(Slapi_PBlock *pb)

 				if(0 == slapi_entry_attr_find(post_e, configCopy.groupattrs[i], &attr))

 				{

 					if((ret = memberof_moddn_attr_list(pb, &configCopy, pre_sdn,

-					                            post_sdn, attr) != 0))

+					                                   post_sdn, attr) != 0))

 					{

 						slapi_log_error( SLAPI_LOG_FATAL, MEMBEROF_PLUGIN_SUBSYSTEM,

 							"memberof_postop_modrdn - update failed for (%s), error (%d)\n",

@@ -894,49 +909,49 @@ int memberof_postop_modrdn(Slapi_PBlock *pb)

 		 * of other group entries.  We need to update any member

 		 * attributes to refer to the new name. */

 		if (ret == LDAP_SUCCESS && pre_sdn && post_sdn) {

-			if ((entry_scope && !slapi_sdn_issuffix(post_sdn, entry_scope)) || 

-                            (entry_scope_exclude_subtree && slapi_sdn_issuffix(post_sdn, entry_scope_exclude_subtree))) {

+			if (!memberof_entry_in_scope(&configCopy, post_sdn)){

 				if((ret = memberof_del_dn_from_groups(pb, &configCopy, pre_sdn))){

 					slapi_log_error( SLAPI_LOG_FATAL, MEMBEROF_PLUGIN_SUBSYSTEM,

 						"memberof_postop_modrdn - delete dn failed for (%s), error (%d)\n",

 						slapi_sdn_get_dn(pre_sdn), ret);

 				}

 				if(ret == LDAP_SUCCESS && pre_e && configCopy.group_filter &&

-						0 == slapi_filter_test_simple(pre_e, configCopy.group_filter)) {

+				   0 == slapi_filter_test_simple(pre_e, configCopy.group_filter))

+				{

 					/* is the entry of interest as a group? */

-						int i = 0;

-						Slapi_Attr *attr = 0;

-

-						/* Loop through to find each grouping attribute separately. */

-						for (i = 0; configCopy.groupattrs[i] && ret == LDAP_SUCCESS; i++) {

-							if (0 == slapi_entry_attr_find(pre_e, configCopy.groupattrs[i], &attr)) {

-								if((ret = memberof_del_attr_list(pb, &configCopy, pre_sdn, attr))){

-									slapi_log_error( SLAPI_LOG_FATAL, MEMBEROF_PLUGIN_SUBSYSTEM,

-									"memberof_postop_modrdn: error deleting attr list - dn (%s). Error (%d)\n",

-									slapi_sdn_get_dn(pre_sdn),ret);

-								}

+					int i = 0;

+					Slapi_Attr *attr = 0;

+					/* Loop through to find each grouping attribute separately. */

+					for (i = 0; configCopy.groupattrs[i] && ret == LDAP_SUCCESS; i++) {

+						if (0 == slapi_entry_attr_find(pre_e, configCopy.groupattrs[i], &attr)) {

+							if((ret = memberof_del_attr_list(pb, &configCopy, pre_sdn, attr))){

+								slapi_log_error( SLAPI_LOG_FATAL, MEMBEROF_PLUGIN_SUBSYSTEM,

+								"memberof_postop_modrdn: error deleting attr list - dn (%s). Error (%d)\n",

+								slapi_sdn_get_dn(pre_sdn),ret);

 							}

+

 						}

-					} 

+					}

+				}

 				if(ret == LDAP_SUCCESS) {

-						memberof_del_dn_data del_data = {0, configCopy.memberof_attr};

-						if((ret = memberof_del_dn_type_callback(post_e, &del_data))){

-							slapi_log_error( SLAPI_LOG_FATAL, MEMBEROF_PLUGIN_SUBSYSTEM,

-								"memberof_postop_modrdn - delete dn callback failed for (%s), error (%d)\n",

-								slapi_entry_get_dn(post_e), ret);

-						}

+					memberof_del_dn_data del_data = {0, configCopy.memberof_attr};

+					if((ret = memberof_del_dn_type_callback(post_e, &del_data))){

+						slapi_log_error( SLAPI_LOG_FATAL, MEMBEROF_PLUGIN_SUBSYSTEM,

+							"memberof_postop_modrdn - delete dn callback failed for (%s), error (%d)\n",

+							slapi_entry_get_dn(post_e), ret);

 					}

+				}

 			} else {

 				if((ret = memberof_replace_dn_from_groups(pb, &configCopy, pre_sdn, post_sdn))){

 					slapi_log_error( SLAPI_LOG_FATAL, MEMBEROF_PLUGIN_SUBSYSTEM,

-						"memberof_postop_modrdn - replace dne failed for (%s), error (%d)\n",

+						"memberof_postop_modrdn - replace dn failed for (%s), error (%d)\n",

 						slapi_sdn_get_dn(pre_sdn), ret);

 				}

 			}

 		}

-

 		memberof_unlock();

+bail:

 		memberof_free_config(&configCopy);

 	}

@@ -978,7 +993,7 @@ memberof_replace_dn_from_groups(Slapi_PBlock *pb, MemberOfConfig *config,

 		groupattrs[0] = config->groupattrs[i];

-		if((ret = memberof_call_foreach_dn(pb, pre_sdn, groupattrs,

+		if((ret = memberof_call_foreach_dn(pb, pre_sdn, config, groupattrs,

 		                                   memberof_replace_dn_type_callback,

 		                                   &data)))

 		{

@@ -1096,12 +1111,11 @@ int memberof_postop_modify(Slapi_PBlock *pb)

 		goto done;

 	}

-

-	if(memberof_oktodo(pb) && (sdn = memberof_getsdn(pb)))

+	if(memberof_oktodo(pb))

 	{

 		int config_copied = 0;

 		MemberOfConfig *mainConfig = 0;

-		MemberOfConfig configCopy = {0, 0, 0, 0};

+		MemberOfConfig configCopy = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0};

 		/* get the mod set */

 		slapi_pblock_get(pb, SLAPI_MODIFY_MODS, &mods;;

@@ -1120,19 +1134,22 @@ int memberof_postop_modify(Slapi_PBlock *pb)

 			 * only copy the config the first time it's needed so

 			 * it remains the same for all mods in the operation,

 			 * despite any config changes that may be made. */

-			if (!config_copied)

-			{

+			if (!config_copied){

 				memberof_rlock_config();

 				mainConfig = memberof_get_config();

 				if (memberof_is_grouping_attr(type, mainConfig))

 				{

 					interested = 1;

+					if (!memberof_entry_in_scope(mainConfig, sdn)){

+						/* Entry is not in scope */

+						memberof_unlock_config();

+						goto bail;

+					}

 					/* copy config so it doesn't change out from under us */

 					memberof_copy_config(&configCopy, mainConfig);

 					config_copied = 1;

 				}

-

 				memberof_unlock_config();

 			} else {

 				if (memberof_is_grouping_attr(type, &configCopy))

@@ -1229,8 +1246,7 @@ int memberof_postop_modify(Slapi_PBlock *pb)

 		}

 bail:

-		if (config_copied)

-		{

+		if (config_copied){

 			memberof_free_config(&configCopy);

 		}

@@ -1276,22 +1292,25 @@ int memberof_postop_add(Slapi_PBlock *pb)

 	if(memberof_oktodo(pb) && (sdn = memberof_getsdn(pb)))

 	{

-		MemberOfConfig *mainConfig = 0;

-		MemberOfConfig configCopy = {0, 0, 0, 0};

 		struct slapi_entry *e = NULL;

-

+		MemberOfConfig configCopy = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0};

+		MemberOfConfig *mainConfig;

 		slapi_pblock_get( pb, SLAPI_ENTRY_POST_OP, &e );

-		

 		/* is the entry of interest? */

 		memberof_rlock_config();

 		mainConfig = memberof_get_config();

 		if(e && mainConfig && mainConfig->group_filter &&

 		   0 == slapi_filter_test_simple(e, mainConfig->group_filter))

+

 		{

 			interested = 1;

-			/* copy config so it doesn't change out from under us */

-			memberof_copy_config(&configCopy, mainConfig);

+			if(!memberof_entry_in_scope(mainConfig, slapi_entry_get_sdn(e))){

+				/* Entry is not in scope */

+				memberof_unlock_config();

+				goto bail;

+			}

+			memberof_copy_config(&configCopy, memberof_get_config());

 		}

 		memberof_unlock_config();

@@ -1316,11 +1335,11 @@ int memberof_postop_add(Slapi_PBlock *pb)

 			}

 			memberof_unlock();

-

 			memberof_free_config(&configCopy);

 		}

 	}

+bail:

 	if(ret){

 		slapi_pblock_set(pb, SLAPI_RESULT_CODE, &ret;;

 		ret = SLAPI_PLUGIN_FAILURE;

@@ -1358,26 +1377,61 @@ int memberof_oktodo(Slapi_PBlock *pb)

 	}

 	if(slapi_pblock_get(pb, SLAPI_PLUGIN_OPRETURN, &oprc) != 0) 

-        {

+	{

 		slapi_log_error( SLAPI_LOG_FATAL, MEMBEROF_PLUGIN_SUBSYSTEM,

 			"memberof_postop_oktodo: could not get parameters\n" );

 		ret = -1;

 	}

-        /* this plugin should only execute if the operation succeeded

-	*/

-        if(oprc != 0)

+	/* this plugin should only execute if the operation succeeded */

+	if(oprc != 0)

 	{

 		ret = 0;

 	}

-	

+

+bail:

 	slapi_log_error( SLAPI_LOG_TRACE, MEMBEROF_PLUGIN_SUBSYSTEM,

 		     "<-- memberof_postop_oktodo\n" );

-bail:

 	return ret;

 }

+/*

+ * Return 1 if the entry is in the scope.

+ * For MODRDN the caller should check both the preop

+ * and postop entries.  If we are moving out of, or

+ * into scope, we should process it.

+ */

+static int

+memberof_entry_in_scope(MemberOfConfig *config, Slapi_DN *sdn)

+{

+    if (config->entryScopeExcludeSubtrees){

+        int i = 0;

+

+        /* check the excludes */

+        while(config->entryScopeExcludeSubtrees[i]){

+            if (slapi_sdn_issuffix(sdn, config->entryScopeExcludeSubtrees[i])){

+                return 0;

+            }

+            i++;

+        }

+    }

+    if (config->entryScopes){

+        int i = 0;

+

+        /* check the excludes */

+        while(config->entryScopes[i]){

+            if (slapi_sdn_issuffix(sdn, config->entryScopes[i])){

+                return 1;

+            }

+            i++;

+        }

+        return 0;

+    }

+

+    return 1;

+}

+

 static Slapi_DN *

 memberof_getsdn(Slapi_PBlock *pb)

 {

@@ -2045,7 +2099,7 @@ memberof_get_groups_r(MemberOfConfig *config, Slapi_DN *member_sdn,

 {

 	/* Search for any grouping attributes that point to memberdn.

 	 * For each match, add it to the list, recurse and do same search */

-	return memberof_call_foreach_dn(NULL, member_sdn, config->groupattrs,

+	return memberof_call_foreach_dn(NULL, member_sdn, config, config->groupattrs,

 		memberof_get_groups_callback, data);

 }

@@ -2057,12 +2111,12 @@ int memberof_get_groups_callback(Slapi_Entry *e, void *callback_data)

 {

 	Slapi_DN *group_sdn = slapi_entry_get_sdn(e);

 	char *group_ndn = slapi_entry_get_ndn(e);

-        char *group_dn = slapi_entry_get_dn(e);

+	char *group_dn = slapi_entry_get_dn(e);

 	Slapi_Value *group_ndn_val = 0;

-        Slapi_Value *group_dn_val = 0;

+	Slapi_Value *group_dn_val = 0;

 	Slapi_ValueSet *groupvals = *((memberof_get_groups_data*)callback_data)->groupvals;

-        Slapi_ValueSet *group_norm_vals = *((memberof_get_groups_data*)callback_data)->group_norm_vals;

-        Slapi_DN *entry_scope_exclude_subtree = memberof_config_get_entry_scope_exclude_subtree();

+	Slapi_ValueSet *group_norm_vals = *((memberof_get_groups_data*)callback_data)->group_norm_vals;

+	MemberOfConfig *config = ((memberof_get_groups_data*)callback_data)->config;

 	int rc = 0;

 	if(slapi_is_shutting_down()){

@@ -2116,18 +2170,19 @@ int memberof_get_groups_callback(Slapi_Entry *e, void *callback_data)

 		goto bail;

 	}

-        /* if the group does not belong to an excluded subtree, adds it to the valueset */

-        if (!(entry_scope_exclude_subtree && slapi_sdn_issuffix(group_sdn, entry_scope_exclude_subtree))) {

-                /* Push group_dn_val into the valueset.  This memory is now owned

-                 * by the valueset. */

-                group_dn_val = slapi_value_new_string(group_dn);

-                slapi_valueset_add_value_ext(groupvals, group_dn_val, SLAPI_VALUE_FLAG_PASSIN);

-                slapi_valueset_add_value_ext(group_norm_vals, group_ndn_val, SLAPI_VALUE_FLAG_PASSIN);

-        }

-        

-	/* now recurse to find parent groups of e */

-	memberof_get_groups_r(((memberof_get_groups_data*)callback_data)->config,

-		group_sdn, callback_data);

+	/* if the group does not belong to an excluded subtree, adds it to the valueset */

+	if (memberof_entry_in_scope(config, group_sdn)) {

+			/* Push group_dn_val into the valueset.  This memory is now owned

+			 * by the valueset. */

+			group_dn_val = slapi_value_new_string(group_dn);

+			slapi_valueset_add_value_ext(groupvals, group_dn_val, SLAPI_VALUE_FLAG_PASSIN);

+			slapi_valueset_add_value_ext(group_norm_vals, group_ndn_val, SLAPI_VALUE_FLAG_PASSIN);

+	}

+	if(!config->skip_nested || config->fixup_task){

+		/* now recurse to find parent groups of e */

+		memberof_get_groups_r(((memberof_get_groups_data*)callback_data)->config,

+			group_sdn, callback_data);

+	}

 bail:

 	return rc;

@@ -2218,8 +2273,8 @@ memberof_test_membership(Slapi_PBlock *pb, MemberOfConfig *config,

 {

 	char *attrs[2] = {config->memberof_attr, 0};

-	return memberof_call_foreach_dn(pb, group_sdn, attrs, 

-		memberof_test_membership_callback , config);

+	return memberof_call_foreach_dn(pb, group_sdn, config, attrs,

+		memberof_test_membership_callback, config);

 }

 /*

diff --git a/ldap/servers/plugins/memberof/memberof.h b/ldap/servers/plugins/memberof/memberof.h

index 59029d7..4516387 100644

--- a/ldap/servers/plugins/memberof/memberof.h

+++ b/ldap/servers/plugins/memberof/memberof.h

@@ -80,8 +80,10 @@ typedef struct memberofconfig {

 	char **groupattrs;

 	char *memberof_attr;

 	int allBackends;

-	Slapi_DN *entryScope;

-	Slapi_DN *entryScopeExcludeSubtree;

+	Slapi_DN **entryScopes;

+	int entryScopeCount;

+	Slapi_DN **entryScopeExcludeSubtrees;

+	int entryExcludeScopeCount;

 	Slapi_Filter *group_filter;

 	Slapi_Attr **group_slapiattrs;

 } MemberOfConfig;

@@ -100,8 +102,6 @@ void memberof_rlock_config();

 void memberof_wlock_config();

 void memberof_unlock_config();

 int memberof_config_get_all_backends();

-Slapi_DN * memberof_config_get_entry_scope();

-Slapi_DN * memberof_config_get_entry_scope_exclude_subtree();

 void memberof_set_config_area(Slapi_DN *sdn);

 Slapi_DN * memberof_get_config_area();

 void memberof_set_plugin_area(Slapi_DN *sdn);

diff --git a/ldap/servers/plugins/memberof/memberof_config.c b/ldap/servers/plugins/memberof/memberof_config.c

index eb83bd0..5fc1314 100644

--- a/ldap/servers/plugins/memberof/memberof_config.c

+++ b/ldap/servers/plugins/memberof/memberof_config.c

@@ -77,7 +77,7 @@ static int memberof_search (Slapi_PBlock *pb, Slapi_Entry* entryBefore, Slapi_En

 /* This is the main configuration which is updated from dse.ldif.  The

  * config will be copied when it is used by the plug-in to prevent it

  * being changed out from under a running memberOf operation. */

-static MemberOfConfig theConfig = {NULL, NULL,0, NULL, NULL, NULL, NULL};

+static MemberOfConfig theConfig = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0};

 static Slapi_RWLock *memberof_config_lock = 0;

 static int inited = 0;

@@ -89,6 +89,19 @@ static int dont_allow_that(Slapi_PBlock *pb, Slapi_Entry* entryBefore, Slapi_Ent

 	return SLAPI_DSE_CALLBACK_ERROR;

 }

+static void

+memberof_free_scope(Slapi_DN **scopes, int *count)

+{

+	int i = 0;

+

+	while(scopes && scopes[i]){

+		slapi_sdn_free(&scopes[i]);

+		i++;

+	}

+	slapi_ch_free((void**)&scopes);

+	*count = 0;

+}

+

 /*

  * memberof_config()

  *

@@ -184,16 +197,22 @@ memberof_release_config()

  *

  * Validate the pending changes in the e entry.

  */

-static int 

+int

 memberof_validate_config (Slapi_PBlock *pb, Slapi_Entry* entryBefore, Slapi_Entry* e, 

 	int *returncode, char *returntext, void *arg)

 {

 	Slapi_Attr *memberof_attr = NULL;

 	Slapi_Attr *group_attr = NULL;

 	Slapi_DN *config_sdn = NULL;

+	Slapi_DN **include_dn = NULL;

+	Slapi_DN **exclude_dn = NULL;

 	char *syntaxoid = NULL;

 	char *config_dn = NULL;

+	char *skip_nested = NULL;

+	char **entry_scopes = NULL;

+	char **entry_exclude_scopes = NULL;

 	int not_dn_syntax = 0;

+	int num_vals = 0;

 	*returncode = LDAP_UNWILLING_TO_PERFORM; /* be pessimistic */

@@ -301,8 +320,112 @@ memberof_validate_config (Slapi_PBlock *pb, Slapi_Entry* entryBefore, Slapi_Entr

 			*returncode = LDAP_UNWILLING_TO_PERFORM;

 		}

 	}

+	/*

+	 * Check the entry scopes

+	 */

+	entry_scopes = slapi_entry_attr_get_charray_ext(e, MEMBEROF_ENTRY_SCOPE_ATTR, &num_vals);

+	if(entry_scopes){

+		int i = 0;

+

+		/* Validate the syntax before we create our DN array */

+		for (i = 0;i < num_vals; i++){

+			if(slapi_dn_syntax_check(pb, entry_scopes[i], 1)){

+				/* invalid dn syntax */

+				PR_snprintf(returntext, SLAPI_DSE_RETURNTEXT_SIZE,

+					"%s: Invalid DN (%s) for include suffix.",

+					MEMBEROF_PLUGIN_SUBSYSTEM, entry_scopes[i]);

+				slapi_ch_array_free(entry_scopes);

+				theConfig.entryScopeCount = 0;

+				*returncode = LDAP_UNWILLING_TO_PERFORM;

+				goto done;

+			}

+		}

+		/* Now create our SDN array for conflict checking */

+		include_dn = (Slapi_DN **)slapi_ch_calloc(sizeof(Slapi_DN *), num_vals+1);

+		for (i = 0;i < num_vals; i++){

+			include_dn[i] = slapi_sdn_new_dn_passin(entry_scopes[i]);

+		}

+	}

+	/*

+	 * Check and process the entry exclude scopes

+	 */

+	entry_exclude_scopes =

+		slapi_entry_attr_get_charray_ext(e, MEMBEROF_ENTRY_SCOPE_EXCLUDE_SUBTREE, &num_vals);

+	if(entry_exclude_scopes){

+		int i = 0;

+

+		/* Validate the syntax before we create our DN array */

+		for (i = 0;i < num_vals; i++){

+			if(slapi_dn_syntax_check(pb, entry_exclude_scopes[i], 1)){

+				/* invalid dn syntax */

+				PR_snprintf(returntext, SLAPI_DSE_RETURNTEXT_SIZE,

+					"%s: Invalid DN (%s) for exclude suffix.",

+					MEMBEROF_PLUGIN_SUBSYSTEM, entry_scopes[i]);

+				slapi_ch_array_free(entry_exclude_scopes);

+				*returncode = LDAP_UNWILLING_TO_PERFORM;

+				goto done;

+			}

+		}

+		/* Now create our SDN array for conflict checking */

+		exclude_dn = (Slapi_DN **)slapi_ch_calloc(sizeof(Slapi_DN *),num_vals+1);

+		for (i = 0;i < num_vals; i++){

+			exclude_dn[i] = slapi_sdn_new_dn_passin(entry_exclude_scopes[i]);