Blame SOURCES/0080-Ticket-49436-double-free-in-COS-in-some-conditions.patch

b69e47
From fc9a206c294fb5ea2401a9365f01ef2565799478 Mon Sep 17 00:00:00 2001
b69e47
From: William Brown <firstyear@redhat.com>
b69e47
Date: Thu, 2 Nov 2017 13:32:41 +1000
b69e47
Subject: [PATCH] Ticket 49436 - double free in COS in some conditions
b69e47
b69e47
Bug Description:  virtualattrs and COS have some serious memory
b69e47
ownership issues. What was happening is that COS with multiple
b69e47
attributes using the same sp_handle would cause a structure
b69e47
to be registered twice. During shutdown we would then trigger
b69e47
a double free in the process.
b69e47
b69e47
Fix Description:  Change the behaviour of sp_handles to use a
b69e47
handle *per* attribute we register to guarantee the assocation
b69e47
between them.
b69e47
b69e47
https://pagure.io/389-ds-base/issue/49436
b69e47
b69e47
Author: wibrown
b69e47
b69e47
Review by: mreynolds, vashirov (Thanks!)
b69e47
b69e47
(cherry pick from commit ee4428a3f5d2d8e37a7107c7dce9d622fc17d41c)
b69e47
---
b69e47
 dirsrvtests/tests/suites/cos/indirect_cos_test.py |  43 +-
b69e47
 ldap/servers/plugins/cos/cos_cache.c              | 723 +++++++++++-----------
b69e47
 ldap/servers/plugins/roles/roles_cache.c          |  50 +-
b69e47
 ldap/servers/slapd/vattr.c                        |  34 +-
b69e47
 4 files changed, 406 insertions(+), 444 deletions(-)
b69e47
b69e47
diff --git a/dirsrvtests/tests/suites/cos/indirect_cos_test.py b/dirsrvtests/tests/suites/cos/indirect_cos_test.py
b69e47
index 1aac6b8ed..452edcdf8 100644
b69e47
--- a/dirsrvtests/tests/suites/cos/indirect_cos_test.py
b69e47
+++ b/dirsrvtests/tests/suites/cos/indirect_cos_test.py
b69e47
@@ -7,6 +7,7 @@ import subprocess
b69e47
 
b69e47
 from lib389 import Entry
b69e47
 from lib389.idm.user import UserAccounts
b69e47
+from lib389.idm.domain import Domain
b69e47
 from lib389.topologies import topology_st as topo
b69e47
 from lib389._constants import (DEFAULT_SUFFIX, DN_DM, PASSWORD, HOST_STANDALONE,
b69e47
                                SERVERID_STANDALONE, PORT_STANDALONE)
b69e47
@@ -48,14 +49,8 @@ def check_user(inst):
b69e47
 def setup_subtree_policy(topo):
b69e47
     """Set up subtree password policy
b69e47
     """
b69e47
-    try:
b69e47
-        topo.standalone.modify_s("cn=config", [(ldap.MOD_REPLACE,
b69e47
-                                                'nsslapd-pwpolicy-local',
b69e47
-                                                'on')])
b69e47
-    except ldap.LDAPError as e:
b69e47
-        log.error('Failed to set fine-grained policy: error {}'.format(
b69e47
-            e.message['desc']))
b69e47
-        raise e
b69e47
+
b69e47
+    topo.standalone.config.set('nsslapd-pwpolicy-local', 'on')
b69e47
 
b69e47
     log.info('Create password policy for subtree {}'.format(OU_PEOPLE))
b69e47
     try:
b69e47
@@ -68,15 +63,9 @@ def setup_subtree_policy(topo):
b69e47
             OU_PEOPLE, e.message['desc']))
b69e47
         raise e
b69e47
 
b69e47
-    log.info('Add pwdpolicysubentry attribute to {}'.format(OU_PEOPLE))
b69e47
-    try:
b69e47
-        topo.standalone.modify_s(DEFAULT_SUFFIX, [(ldap.MOD_REPLACE,
b69e47
-                                                   'pwdpolicysubentry',
b69e47
-                                                   PW_POLICY_CONT_PEOPLE2)])
b69e47
-    except ldap.LDAPError as e:
b69e47
-        log.error('Failed to pwdpolicysubentry pw policy '
b69e47
-                  'policy for {}: error {}'.format(OU_PEOPLE, e.message['desc']))
b69e47
-        raise e
b69e47
+    domain = Domain(topo.standalone, DEFAULT_SUFFIX)
b69e47
+    domain.replace('pwdpolicysubentry', PW_POLICY_CONT_PEOPLE2)
b69e47
+
b69e47
     time.sleep(1)
b69e47
 
b69e47
 
b69e47
@@ -116,12 +105,9 @@ def setup(topo, request):
b69e47
     """
b69e47
     log.info('Add custom schema...')
b69e47
     try:
b69e47
-        ATTR_1 = ("( 1.3.6.1.4.1.409.389.2.189 NAME 'x-department' " +
b69e47
-                  "SYNTAX 1.3.6.1.4.1.1466.115.121.1.15 X-ORIGIN 'user defined' )")
b69e47
-        ATTR_2 = ("( 1.3.6.1.4.1.409.389.2.187 NAME 'x-en-ou' " +
b69e47
-                  "SYNTAX 1.3.6.1.4.1.1466.115.121.1.15 X-ORIGIN 'user defined' )")
b69e47
-        OC = ("( xPerson-oid NAME 'xPerson' DESC '' SUP person STRUCTURAL MAY " +
b69e47
-              "( x-department $ x-en-ou ) X-ORIGIN 'user defined' )")
b69e47
+        ATTR_1 = (b"( 1.3.6.1.4.1.409.389.2.189 NAME 'x-department' SYNTAX 1.3.6.1.4.1.1466.115.121.1.15 X-ORIGIN 'user defined' )")
b69e47
+        ATTR_2 = (b"( 1.3.6.1.4.1.409.389.2.187 NAME 'x-en-ou' SYNTAX 1.3.6.1.4.1.1466.115.121.1.15 X-ORIGIN 'user defined' )")
b69e47
+        OC = (b"( xPerson-oid NAME 'xPerson' DESC '' SUP person STRUCTURAL MAY ( x-department $ x-en-ou ) X-ORIGIN 'user defined' )")
b69e47
         topo.standalone.modify_s("cn=schema", [(ldap.MOD_ADD, 'attributeTypes', ATTR_1),
b69e47
                                                (ldap.MOD_ADD, 'attributeTypes', ATTR_2),
b69e47
                                                (ldap.MOD_ADD, 'objectClasses', OC)])
b69e47
@@ -142,14 +128,9 @@ def setup(topo, request):
b69e47
         'homeDirectory': '/home/test_user',
b69e47
         'seeAlso': 'cn=cosTemplate,dc=example,dc=com'
b69e47
     }
b69e47
-    users.create(properties=user_properties)
b69e47
-    try:
b69e47
-        topo.standalone.modify_s(TEST_USER_DN, [(ldap.MOD_ADD,
b69e47
-                                                 'objectclass',
b69e47
-                                                 'xPerson')])
b69e47
-    except ldap.LDAPError as e:
b69e47
-        log.fatal('Failed to add objectclass to user')
b69e47
-        raise e
b69e47
+    user = users.create(properties=user_properties)
b69e47
+
b69e47
+    user.add('objectClass', 'xPerson')
b69e47
 
b69e47
     # Setup COS
b69e47
     log.info("Setup indirect COS...")
b69e47
diff --git a/ldap/servers/plugins/cos/cos_cache.c b/ldap/servers/plugins/cos/cos_cache.c
b69e47
index 87d48908c..0e93183d2 100644
b69e47
--- a/ldap/servers/plugins/cos/cos_cache.c
b69e47
+++ b/ldap/servers/plugins/cos/cos_cache.c
b69e47
@@ -108,9 +108,6 @@ void * cos_get_plugin_identity(void);
b69e47
 #define COSTYPE_INDIRECT 3
b69e47
 #define COS_DEF_ERROR_NO_TEMPLATES -2
b69e47
 
b69e47
-/* the global plugin handle */
b69e47
-static volatile vattr_sp_handle *vattr_handle = NULL;
b69e47
-
b69e47
 /* both variables are protected by change_lock */
b69e47
 static int cos_cache_notify_flag = 0;
b69e47
 static PRBool cos_cache_at_work = PR_FALSE;
b69e47
@@ -289,75 +286,61 @@ static Slapi_CondVar *start_cond = NULL;
b69e47
 */
b69e47
 int cos_cache_init(void)
b69e47
 {
b69e47
-	int ret = 0;
b69e47
-
b69e47
-	slapi_log_err(SLAPI_LOG_TRACE, COS_PLUGIN_SUBSYSTEM, "--> cos_cache_init\n");
b69e47
-
b69e47
-	slapi_vattrcache_cache_none();
b69e47
-	cache_lock = slapi_new_mutex();
b69e47
-	change_lock = slapi_new_mutex();
b69e47
-	stop_lock = slapi_new_mutex();
b69e47
-	something_changed = slapi_new_condvar(change_lock);
b69e47
-	keeprunning =1;
b69e47
-	start_lock = slapi_new_mutex();
b69e47
-	start_cond = slapi_new_condvar(start_lock);
b69e47
-	started = 0;
b69e47
-
b69e47
-	if (stop_lock == NULL ||
b69e47
-	    change_lock == NULL ||
b69e47
-	    cache_lock == NULL ||
b69e47
-	    stop_lock == NULL ||
b69e47
-	    start_lock == NULL ||
b69e47
-	    start_cond == NULL ||
b69e47
-	    something_changed == NULL)
b69e47
-	{
b69e47
-		slapi_log_err(SLAPI_LOG_ERR, COS_PLUGIN_SUBSYSTEM,
b69e47
-			   "cos_cache_init - Cannot create mutexes\n" );
b69e47
-				ret = -1;
b69e47
-		goto out;
b69e47
-	}
b69e47
-
b69e47
-	/* grab the views interface */
b69e47
-	if(slapi_apib_get_interface(Views_v1_0_GUID, &views_api))
b69e47
-	{
b69e47
-		/* lets be tolerant if views is disabled */
b69e47
-		views_api = 0;
b69e47
-	}
b69e47
+    int ret = 0;
b69e47
 
b69e47
-	if (slapi_vattrspi_register((vattr_sp_handle **)&vattr_handle, 
b69e47
-	                            cos_cache_vattr_get, 
b69e47
-	                            cos_cache_vattr_compare, 
b69e47
-	                            cos_cache_vattr_types) != 0)
b69e47
-	{
b69e47
-		slapi_log_err(SLAPI_LOG_ERR, COS_PLUGIN_SUBSYSTEM,
b69e47
-		   "cos_cache_init - Cannot register as service provider\n" );
b69e47
-			ret = -1;
b69e47
-		goto out;
b69e47
-	}
b69e47
+    slapi_log_err(SLAPI_LOG_TRACE, COS_PLUGIN_SUBSYSTEM, "--> cos_cache_init\n");
b69e47
+
b69e47
+    slapi_vattrcache_cache_none();
b69e47
+    cache_lock = slapi_new_mutex();
b69e47
+    change_lock = slapi_new_mutex();
b69e47
+    stop_lock = slapi_new_mutex();
b69e47
+    something_changed = slapi_new_condvar(change_lock);
b69e47
+    keeprunning = 1;
b69e47
+    start_lock = slapi_new_mutex();
b69e47
+    start_cond = slapi_new_condvar(start_lock);
b69e47
+    started = 0;
b69e47
+
b69e47
+    if (stop_lock == NULL ||
b69e47
+        change_lock == NULL ||
b69e47
+        cache_lock == NULL ||
b69e47
+        stop_lock == NULL ||
b69e47
+        start_lock == NULL ||
b69e47
+        start_cond == NULL ||
b69e47
+        something_changed == NULL) {
b69e47
+        slapi_log_err(SLAPI_LOG_ERR, COS_PLUGIN_SUBSYSTEM,
b69e47
+                      "cos_cache_init - Cannot create mutexes\n");
b69e47
+        ret = -1;
b69e47
+        goto out;
b69e47
+    }
b69e47
 
b69e47
-	if ( PR_CreateThread (PR_USER_THREAD, 
b69e47
-				cos_cache_wait_on_change, 
b69e47
-				NULL,
b69e47
-				PR_PRIORITY_NORMAL, 
b69e47
-				PR_GLOBAL_THREAD, 
b69e47
-				PR_UNJOINABLE_THREAD, 
b69e47
-				SLAPD_DEFAULT_THREAD_STACKSIZE) == NULL )
b69e47
-	{
b69e47
-		slapi_log_err(SLAPI_LOG_ERR, COS_PLUGIN_SUBSYSTEM,
b69e47
-			   "cos_cache_init - PR_CreateThread failed\n" );
b69e47
-		ret = -1;
b69e47
-		goto out;
b69e47
-	}
b69e47
+    /* grab the views interface */
b69e47
+    if (slapi_apib_get_interface(Views_v1_0_GUID, &views_api)) {
b69e47
+        /* lets be tolerant if views is disabled */
b69e47
+        views_api = 0;
b69e47
+    }
b69e47
 
b69e47
-		/* wait for that thread to get started */
b69e47
-		if (ret == 0) {
b69e47
-			slapi_lock_mutex(start_lock);
b69e47
-			while (!started) {
b69e47
-				while (slapi_wait_condvar(start_cond, NULL) == 0);
b69e47
-			}
b69e47
-			slapi_unlock_mutex(start_lock);
b69e47
-		}
b69e47
+    if (PR_CreateThread(PR_USER_THREAD,
b69e47
+                        cos_cache_wait_on_change,
b69e47
+                        NULL,
b69e47
+                        PR_PRIORITY_NORMAL,
b69e47
+                        PR_GLOBAL_THREAD,
b69e47
+                        PR_UNJOINABLE_THREAD,
b69e47
+                        SLAPD_DEFAULT_THREAD_STACKSIZE) == NULL) {
b69e47
+        slapi_log_err(SLAPI_LOG_ERR, COS_PLUGIN_SUBSYSTEM,
b69e47
+                      "cos_cache_init - PR_CreateThread failed\n");
b69e47
+        ret = -1;
b69e47
+        goto out;
b69e47
+    }
b69e47
 
b69e47
+    /* wait for that thread to get started */
b69e47
+    if (ret == 0) {
b69e47
+        slapi_lock_mutex(start_lock);
b69e47
+        while (!started) {
b69e47
+            while (slapi_wait_condvar(start_cond, NULL) == 0)
b69e47
+                ;
b69e47
+        }
b69e47
+        slapi_unlock_mutex(start_lock);
b69e47
+    }
b69e47
 
b69e47
 out:
b69e47
 	slapi_log_err(SLAPI_LOG_TRACE, COS_PLUGIN_SUBSYSTEM, "<-- cos_cache_init\n");
b69e47
@@ -752,321 +735,311 @@ struct dn_defs_info {
b69e47
 static int 
b69e47
 cos_dn_defs_cb (Slapi_Entry* e, void *callback_data)
b69e47
 {
b69e47
-	struct dn_defs_info *info;
b69e47
-	cosAttrValue **pSneakyVal = 0;
b69e47
-	cosAttrValue *pObjectclass = 0;
b69e47
-	cosAttrValue *pCosTargetTree = 0;
b69e47
-	cosAttrValue *pCosTemplateDn = 0;
b69e47
-	cosAttrValue *pCosSpecifier = 0;
b69e47
-	cosAttrValue *pCosAttribute = 0;
b69e47
-	cosAttrValue *pCosOverrides = 0;
b69e47
-	cosAttrValue *pCosOperational = 0;
b69e47
-	cosAttrValue *pCosOpDefault = 0;
b69e47
-	cosAttrValue *pCosMerge = 0;
b69e47
-	cosAttrValue *pDn = 0;
b69e47
-	struct berval **dnVals;
b69e47
-	int cosType = 0;
b69e47
-	int valIndex = 0;
b69e47
-	Slapi_Attr *dnAttr;
b69e47
-	char *attrType = 0;
b69e47
-	char *norm_dn = NULL;
b69e47
-	info=(struct dn_defs_info *)callback_data;
b69e47
-	
b69e47
-	cos_cache_add_attrval(&pDn, slapi_entry_get_dn(e));
b69e47
-	if(slapi_entry_first_attr(e, &dnAttr)) {
b69e47
-		goto bail;
b69e47
-	}
b69e47
+    struct dn_defs_info *info;
b69e47
+    cosAttrValue **pSneakyVal = 0;
b69e47
+    cosAttrValue *pObjectclass = 0;
b69e47
+    cosAttrValue *pCosTargetTree = 0;
b69e47
+    cosAttrValue *pCosTemplateDn = 0;
b69e47
+    cosAttrValue *pCosSpecifier = 0;
b69e47
+    cosAttrValue *pCosAttribute = 0;
b69e47
+    cosAttrValue *pCosOverrides = 0;
b69e47
+    cosAttrValue *pCosOperational = 0;
b69e47
+    cosAttrValue *pCosOpDefault = 0;
b69e47
+    cosAttrValue *pCosMerge = 0;
b69e47
+    cosAttrValue *pDn = 0;
b69e47
+    struct berval **dnVals;
b69e47
+    int cosType = 0;
b69e47
+    int valIndex = 0;
b69e47
+    Slapi_Attr *dnAttr;
b69e47
+    char *attrType = 0;
b69e47
+    char *norm_dn = NULL;
b69e47
+    info = (struct dn_defs_info *)callback_data;
b69e47
+
b69e47
+    cos_cache_add_attrval(&pDn, slapi_entry_get_dn(e));
b69e47
+    if (slapi_entry_first_attr(e, &dnAttr)) {
b69e47
+        goto bail;
b69e47
+    }
b69e47
 
b69e47
-	do {
b69e47
-		attrType = 0;		
b69e47
-		/* we need to fill in the details of the definition now */
b69e47
-		slapi_attr_get_type(dnAttr, &attrType);		
b69e47
-		if(!attrType) {
b69e47
-			continue;
b69e47
-		}
b69e47
-		pSneakyVal = 0;
b69e47
-		if(!slapi_utf8casecmp((unsigned char*)attrType, (unsigned char*)"objectclass"))
b69e47
-			pSneakyVal = &pObjectclass;
b69e47
-		else if(!slapi_utf8casecmp((unsigned char*)attrType, (unsigned char*)"cosTargetTree")){
b69e47
-			if(pCosTargetTree){
b69e47
-				norm_dn = slapi_create_dn_string("%s", pCosTargetTree->val);
b69e47
-				if(norm_dn){
b69e47
-					slapi_ch_free_string(&pCosTargetTree->val);
b69e47
-					pCosTargetTree->val = norm_dn;
b69e47
-				}
b69e47
-			}
b69e47
-			pSneakyVal = &pCosTargetTree;
b69e47
-		} else if(!slapi_utf8casecmp((unsigned char*)attrType, (unsigned char*)"cosTemplateDn"))
b69e47
-			pSneakyVal = &pCosTemplateDn;
b69e47
-		else if(!slapi_utf8casecmp((unsigned char*)attrType, (unsigned char*)"cosSpecifier"))
b69e47
-			pSneakyVal = &pCosSpecifier;
b69e47
-		else if(!slapi_utf8casecmp((unsigned char*)attrType, (unsigned char*)"cosAttribute"))
b69e47
-			pSneakyVal = &pCosAttribute;
b69e47
-		else if(!slapi_utf8casecmp((unsigned char*)attrType, (unsigned char*)"cosIndirectSpecifier"))
b69e47
-			pSneakyVal = &pCosSpecifier;			
b69e47
-		if(!pSneakyVal) {
b69e47
-			continue;
b69e47
-		}
b69e47
-		/* It's a type we're interested in */
b69e47
-		if(slapi_attr_get_bervals_copy(dnAttr, &dnVals)) {
b69e47
-			continue;
b69e47
-		}
b69e47
-		valIndex = 0;
b69e47
-		if(!dnVals) {
b69e47
-			continue;
b69e47
-		}
b69e47
-		for (valIndex = 0; dnVals[valIndex]; valIndex++)
b69e47
-		{
b69e47
-			if(!dnVals[valIndex]->bv_val) {
b69e47
-				continue;
b69e47
-			}
b69e47
-			/*
b69e47
-			parse any overide or default values
b69e47
-			and deal with them
b69e47
-			*/
b69e47
-			if(pSneakyVal == &pCosAttribute)
b69e47
-			{
b69e47
-				int qualifier_hit = 0;
b69e47
-				int op_qualifier_hit = 0;
b69e47
-				int merge_schemes_qualifier_hit = 0;
b69e47
-				int override_qualifier_hit =0;
b69e47
-				int default_qualifier_hit = 0;
b69e47
-				int operational_default_qualifier_hit = 0;
b69e47
-				do
b69e47
-				{
b69e47
-					qualifier_hit = 0;
b69e47
+    do {
b69e47
+        attrType = 0;
b69e47
+        /* we need to fill in the details of the definition now */
b69e47
+        slapi_attr_get_type(dnAttr, &attrType);
b69e47
+        if (!attrType) {
b69e47
+            continue;
b69e47
+        }
b69e47
+        pSneakyVal = 0;
b69e47
+        if (!slapi_utf8casecmp((unsigned char *)attrType, (unsigned char *)"objectclass"))
b69e47
+            pSneakyVal = &pObjectclass;
b69e47
+        else if (!slapi_utf8casecmp((unsigned char *)attrType, (unsigned char *)"cosTargetTree")) {
b69e47
+            if (pCosTargetTree) {
b69e47
+                norm_dn = slapi_create_dn_string("%s", pCosTargetTree->val);
b69e47
+                if (norm_dn) {
b69e47
+                    slapi_ch_free_string(&pCosTargetTree->val);
b69e47
+                    pCosTargetTree->val = norm_dn;
b69e47
+                }
b69e47
+            }
b69e47
+            pSneakyVal = &pCosTargetTree;
b69e47
+        } else if (!slapi_utf8casecmp((unsigned char *)attrType, (unsigned char *)"cosTemplateDn"))
b69e47
+            pSneakyVal = &pCosTemplateDn;
b69e47
+        else if (!slapi_utf8casecmp((unsigned char *)attrType, (unsigned char *)"cosSpecifier"))
b69e47
+            pSneakyVal = &pCosSpecifier;
b69e47
+        else if (!slapi_utf8casecmp((unsigned char *)attrType, (unsigned char *)"cosAttribute"))
b69e47
+            pSneakyVal = &pCosAttribute;
b69e47
+        else if (!slapi_utf8casecmp((unsigned char *)attrType, (unsigned char *)"cosIndirectSpecifier"))
b69e47
+            pSneakyVal = &pCosSpecifier;
b69e47
+        if (!pSneakyVal) {
b69e47
+            continue;
b69e47
+        }
b69e47
+        /* It's a type we're interested in */
b69e47
+        if (slapi_attr_get_bervals_copy(dnAttr, &dnVals)) {
b69e47
+            continue;
b69e47
+        }
b69e47
+        valIndex = 0;
b69e47
+        if (!dnVals) {
b69e47
+            continue;
b69e47
+        }
b69e47
+        for (valIndex = 0; dnVals[valIndex]; valIndex++) {
b69e47
+            if (!dnVals[valIndex]->bv_val) {
b69e47
+                continue;
b69e47
+            }
b69e47
+            /*
b69e47
+            parse any overide or default values
b69e47
+            and deal with them
b69e47
+            */
b69e47
+            if (pSneakyVal == &pCosAttribute) {
b69e47
+                int qualifier_hit = 0;
b69e47
+                int op_qualifier_hit = 0;
b69e47
+                int merge_schemes_qualifier_hit = 0;
b69e47
+                int override_qualifier_hit = 0;
b69e47
+                int default_qualifier_hit = 0;
b69e47
+                int operational_default_qualifier_hit = 0;
b69e47
+                do {
b69e47
+                    qualifier_hit = 0;
b69e47
+
b69e47
+                    if (cos_cache_backwards_stricmp_and_clip(dnVals[valIndex]->bv_val, " operational")) {
b69e47
+                        /* matched */
b69e47
+                        op_qualifier_hit = 1;
b69e47
+                        qualifier_hit = 1;
b69e47
+                    }
b69e47
+
b69e47
+                    if (cos_cache_backwards_stricmp_and_clip(dnVals[valIndex]->bv_val, " merge-schemes")) {
b69e47
+                        /* matched */
b69e47
+                        merge_schemes_qualifier_hit = 1;
b69e47
+                        qualifier_hit = 1;
b69e47
+                    }
b69e47
+
b69e47
+                    if (cos_cache_backwards_stricmp_and_clip(dnVals[valIndex]->bv_val, " override")) {
b69e47
+                        /* matched */
b69e47
+                        override_qualifier_hit = 1;
b69e47
+                        qualifier_hit = 1;
b69e47
+                    }
b69e47
+
b69e47
+                    if (cos_cache_backwards_stricmp_and_clip(dnVals[valIndex]->bv_val, " default")) {
b69e47
+                        default_qualifier_hit = 1;
b69e47
+                        qualifier_hit = 1;
b69e47
+                    }
b69e47
+
b69e47
+                    if (cos_cache_backwards_stricmp_and_clip(dnVals[valIndex]->bv_val, " operational-default")) {
b69e47
+                        operational_default_qualifier_hit = 1;
b69e47
+                        qualifier_hit = 1;
b69e47
+                    }
b69e47
+                } while (qualifier_hit == 1);
b69e47
+
b69e47
+                /*
b69e47
+                * At this point, dnVals[valIndex]->bv_val
b69e47
+                * is the value of cosAttribute, stripped of
b69e47
+                * any qualifiers, so add this pure attribute type to
b69e47
+                * the appropriate lists.
b69e47
+                */
b69e47
+
b69e47
+                if (op_qualifier_hit) {
b69e47
+                    cos_cache_add_attrval(&pCosOperational,
b69e47
+                                          dnVals[valIndex]->bv_val);
b69e47
+                }
b69e47
+                if (merge_schemes_qualifier_hit) {
b69e47
+                    cos_cache_add_attrval(&pCosMerge, dnVals[valIndex]->bv_val);
b69e47
+                }
b69e47
+                if (override_qualifier_hit) {
b69e47
+                    cos_cache_add_attrval(&pCosOverrides,
b69e47
+                                          dnVals[valIndex]->bv_val);
b69e47
+                }
b69e47
+                if (default_qualifier_hit) {
b69e47
+                    /* attr is added below in pSneakyVal, in any case */
b69e47
+                }
b69e47
+
b69e47
+                if (operational_default_qualifier_hit) {
b69e47
+                    cos_cache_add_attrval(&pCosOpDefault,
b69e47
+                                          dnVals[valIndex]->bv_val);
b69e47
+                }
b69e47
+
b69e47
+                /*
b69e47
+                 * Each SP_handle is associated to one and only one vattr.
b69e47
+                 * We could consider making this a single function rather
b69e47
+                 * than the double-call.
b69e47
+                 */
b69e47
+
b69e47
+                vattr_sp_handle *vattr_handle = NULL;
b69e47
+
b69e47
+                if (slapi_vattrspi_register((vattr_sp_handle **)&vattr_handle,
b69e47
+                                            cos_cache_vattr_get,
b69e47
+                                            cos_cache_vattr_compare,
b69e47
+                                            cos_cache_vattr_types) != 0) {
b69e47
+                    slapi_log_err(SLAPI_LOG_ERR, COS_PLUGIN_SUBSYSTEM, "cos_cache_init - Cannot register as service provider for %s\n", dnVals[valIndex]->bv_val);
b69e47
+                } else {
b69e47
+                    slapi_vattrspi_regattr((vattr_sp_handle *)vattr_handle, dnVals[valIndex]->bv_val, NULL, NULL);
b69e47
+                }
b69e47
+
b69e47
+            } /* if(attrType is cosAttribute) */
b69e47
+
b69e47
+            /*
b69e47
+             * Add the attributetype to the appropriate
b69e47
+             * list.
b69e47
+             */
b69e47
+            cos_cache_add_attrval(pSneakyVal, dnVals[valIndex]->bv_val);
b69e47
+
b69e47
+        } /* for (valIndex = 0; dnVals[valIndex]; valIndex++) */
b69e47
+
b69e47
+        ber_bvecfree(dnVals);
b69e47
+        dnVals = NULL;
b69e47
+    } while (!slapi_entry_next_attr(e, dnAttr, &dnAttr));
b69e47
+
b69e47
+    if (pCosAttribute && (!pCosTargetTree || !pCosTemplateDn)) {
b69e47
+        /* get the parent of the definition */
b69e47
+        char *orig = slapi_dn_parent(pDn->val);
b69e47
+        char *parent = NULL;
b69e47
+        if (orig) {
b69e47
+            parent = slapi_create_dn_string("%s", orig);
b69e47
+            if (!parent) {
b69e47
+                parent = orig;
b69e47
+                slapi_log_err(SLAPI_LOG_ERR, COS_PLUGIN_SUBSYSTEM,
b69e47
+                              "cos_dn_defs_cb - "
b69e47
+                              "Failed to normalize parent dn %s. "
b69e47
+                              "Adding the pre normalized dn.\n",
b69e47
+                              parent);
b69e47
+            }
b69e47
+            if (!pCosTargetTree) {
b69e47
+                cos_cache_add_attrval(&pCosTargetTree, parent);
b69e47
+            }
b69e47
+            if (!pCosTemplateDn) {
b69e47
+                cos_cache_add_attrval(&pCosTemplateDn, parent);
b69e47
+            }
b69e47
+            if (parent != orig) {
b69e47
+                slapi_ch_free_string(&parent);
b69e47
+            }
b69e47
+            slapi_ch_free_string(&orig);
b69e47
+        } else {
b69e47
+            slapi_log_err(SLAPI_LOG_ERR, COS_PLUGIN_SUBSYSTEM,
b69e47
+                          "cos_dn_defs_cb - "
b69e47
+                          "Failed to get parent dn of cos definition %s.\n",
b69e47
+                          pDn->val);
b69e47
+            if (!pCosTemplateDn) {
b69e47
+                if (!pCosTargetTree) {
b69e47
+                    slapi_log_err(SLAPI_LOG_ERR, COS_PLUGIN_SUBSYSTEM, "cos_dn_defs_cb - cosTargetTree and cosTemplateDn are not set.\n");
b69e47
+                } else {
b69e47
+                    slapi_log_err(SLAPI_LOG_ERR, COS_PLUGIN_SUBSYSTEM, "cos_dn_defs_cb - cosTemplateDn is not set.\n");
b69e47
+                }
b69e47
+            } else if (!pCosTargetTree) {
b69e47
+                slapi_log_err(SLAPI_LOG_ERR, COS_PLUGIN_SUBSYSTEM, "cos_dn_defs_cb - cosTargetTree is not set.\n");
b69e47
+            }
b69e47
+        }
b69e47
+    }
b69e47
 
b69e47
-					if(cos_cache_backwards_stricmp_and_clip(dnVals[valIndex]->bv_val, " operational"))
b69e47
-					{
b69e47
-						/* matched */
b69e47
-						op_qualifier_hit = 1;
b69e47
-						qualifier_hit = 1;
b69e47
-					}
b69e47
-					
b69e47
-					if(cos_cache_backwards_stricmp_and_clip(dnVals[valIndex]->bv_val, " merge-schemes"))
b69e47
-					{
b69e47
-						/* matched */
b69e47
-						merge_schemes_qualifier_hit = 1;
b69e47
-						qualifier_hit = 1;
b69e47
-					}
b69e47
+    /*
b69e47
+    determine the type of class of service scheme
b69e47
+    */
b69e47
 
b69e47
-					if(cos_cache_backwards_stricmp_and_clip(dnVals[valIndex]->bv_val, " override"))
b69e47
-					{
b69e47
-						/* matched */
b69e47
-						override_qualifier_hit = 1;
b69e47
-						qualifier_hit = 1;
b69e47
-					}
b69e47
-					
b69e47
-					if(cos_cache_backwards_stricmp_and_clip(dnVals[valIndex]->bv_val, " default")) {
b69e47
-						default_qualifier_hit = 1;
b69e47
-						qualifier_hit = 1;
b69e47
-					}
b69e47
+    if (pObjectclass) {
b69e47
+        if (cos_cache_attrval_exists(pObjectclass, "cosDefinition")) {
b69e47
+            cosType = COSTYPE_CLASSIC;
b69e47
+        } else if (cos_cache_attrval_exists(pObjectclass, "cosClassicDefinition")) {
b69e47
+            cosType = COSTYPE_CLASSIC;
b69e47
 
b69e47
-					if(cos_cache_backwards_stricmp_and_clip(dnVals[valIndex]->bv_val, " operational-default")) {
b69e47
-						operational_default_qualifier_hit = 1;
b69e47
-						qualifier_hit = 1;
b69e47
-					}
b69e47
-				}
b69e47
-				while(qualifier_hit == 1);
b69e47
+        } else if (cos_cache_attrval_exists(pObjectclass, "cosPointerDefinition")) {
b69e47
+            cosType = COSTYPE_POINTER;
b69e47
 
b69e47
-				/*
b69e47
-				* At this point, dnVals[valIndex]->bv_val
b69e47
-				* is the value of cosAttribute, stripped of
b69e47
-				* any qualifiers, so add this pure attribute type to
b69e47
-				* the appropriate lists.
b69e47
-				*/
b69e47
-		
b69e47
-				if ( op_qualifier_hit ) {
b69e47
-					cos_cache_add_attrval(&pCosOperational,
b69e47
-					                      dnVals[valIndex]->bv_val);
b69e47
-				}
b69e47
-				if ( merge_schemes_qualifier_hit ) {
b69e47
-					cos_cache_add_attrval(&pCosMerge, dnVals[valIndex]->bv_val);
b69e47
-				}
b69e47
-				if ( override_qualifier_hit ) {
b69e47
-					cos_cache_add_attrval(&pCosOverrides,
b69e47
-					                      dnVals[valIndex]->bv_val);
b69e47
-				}
b69e47
-				if ( default_qualifier_hit ) {
b69e47
-					/* attr is added below in pSneakyVal, in any case */
b69e47
-				}
b69e47
+        } else if (cos_cache_attrval_exists(pObjectclass, "cosIndirectDefinition")) {
b69e47
+            cosType = COSTYPE_INDIRECT;
b69e47
 
b69e47
-				if ( operational_default_qualifier_hit ) {
b69e47
-					cos_cache_add_attrval(&pCosOpDefault,
b69e47
-					                      dnVals[valIndex]->bv_val);
b69e47
-				}
b69e47
+        } else
b69e47
+            cosType = COSTYPE_BADTYPE;
b69e47
+    }
b69e47
 
b69e47
-				slapi_vattrspi_regattr((vattr_sp_handle *)vattr_handle,
b69e47
-				                        dnVals[valIndex]->bv_val, NULL, NULL);
b69e47
-			} /* if(attrType is cosAttribute) */
b69e47
+    /*
b69e47
+    we should now have a full definition,
b69e47
+    do some sanity checks because we don't
b69e47
+    want bogus entries in the cache
b69e47
+    then ship it
b69e47
+    */
b69e47
+
b69e47
+    /* these must exist */
b69e47
+    if (pDn && pObjectclass &&
b69e47
+        ((cosType == COSTYPE_CLASSIC &&
b69e47
+          pCosTemplateDn &&
b69e47
+          pCosSpecifier &&
b69e47
+          pCosAttribute) ||
b69e47
+         (cosType == COSTYPE_POINTER &&
b69e47
+          pCosTemplateDn &&
b69e47
+          pCosAttribute) ||
b69e47
+         (cosType == COSTYPE_INDIRECT &&
b69e47
+          pCosSpecifier &&
b69e47
+          pCosAttribute))) {
b69e47
+        int rc = 0;
b69e47
+        /*
b69e47
+    we'll leave the referential integrity stuff
b69e47
+    up to the referint plug-in and assume all
b69e47
+    is good - if it's not then we just have a
b69e47
+    useless definition and we'll nag copiously later.
b69e47
+        */
b69e47
+        char *pTmpDn = slapi_ch_strdup(pDn->val); /* because dn gets hosed on error */
b69e47
+
b69e47
+        if (!(rc = cos_cache_add_defn(info->pDefs, &pDn, cosType,
b69e47
+                                      &pCosTargetTree, &pCosTemplateDn,
b69e47
+                                      &pCosSpecifier, &pCosAttribute,
b69e47
+                                      &pCosOverrides, &pCosOperational,
b69e47
+                                      &pCosMerge, &pCosOpDefault))) {
b69e47
+            info->ret = 0; /* we have succeeded to add the defn*/
b69e47
+        } else {
b69e47
+            /*
b69e47
+             * Failed but we will continue the search for other defs
b69e47
+             * Don't reset info->ret....it keeps track of any success
b69e47
+            */
b69e47
+            if (rc == COS_DEF_ERROR_NO_TEMPLATES) {
b69e47
+                slapi_log_err(SLAPI_LOG_ERR, COS_PLUGIN_SUBSYSTEM, "cos_dn_defs_cb - Skipping CoS Definition %s"
b69e47
+                                                                   "--no CoS Templates found, which should be added before the CoS Definition.\n",
b69e47
+                              pTmpDn);
b69e47
+            } else {
b69e47
+                slapi_log_err(SLAPI_LOG_ERR, COS_PLUGIN_SUBSYSTEM, "cos_dn_defs_cb - Skipping CoS Definition %s\n"
b69e47
+                                                                   "--error(%d)\n",
b69e47
+                              pTmpDn, rc);
b69e47
+            }
b69e47
+        }
b69e47
 
b69e47
-			/*
b69e47
-			 * Add the attributetype to the appropriate
b69e47
-			 * list.
b69e47
-			 */
b69e47
-			cos_cache_add_attrval(pSneakyVal, dnVals[valIndex]->bv_val);
b69e47
-			
b69e47
-		}/* for (valIndex = 0; dnVals[valIndex]; valIndex++) */
b69e47
-		
b69e47
-		ber_bvecfree( dnVals );
b69e47
-		dnVals = NULL;
b69e47
-	} while(!slapi_entry_next_attr(e, dnAttr, &dnAttr));
b69e47
-
b69e47
-	if (pCosAttribute && (!pCosTargetTree || !pCosTemplateDn)) {
b69e47
-		/* get the parent of the definition */
b69e47
-		char *orig = slapi_dn_parent(pDn->val);
b69e47
-		char *parent = NULL;
b69e47
-		if (orig) {
b69e47
-			parent = slapi_create_dn_string("%s", orig);
b69e47
-			if (!parent) {
b69e47
-				parent = orig;
b69e47
-				slapi_log_err(SLAPI_LOG_ERR, COS_PLUGIN_SUBSYSTEM, 
b69e47
-				              "cos_dn_defs_cb - "
b69e47
-				              "Failed to normalize parent dn %s. "
b69e47
-				              "Adding the pre normalized dn.\n", 
b69e47
-				              parent);
b69e47
-			}
b69e47
-			if (!pCosTargetTree) {
b69e47
-				cos_cache_add_attrval(&pCosTargetTree, parent);
b69e47
-			}
b69e47
-			if (!pCosTemplateDn) {
b69e47
-				cos_cache_add_attrval(&pCosTemplateDn, parent);
b69e47
-			}
b69e47
-			if (parent != orig) {
b69e47
-				slapi_ch_free_string(&parent);
b69e47
-			}
b69e47
-			slapi_ch_free_string(&orig);
b69e47
-		} else {
b69e47
-			slapi_log_err(SLAPI_LOG_ERR, COS_PLUGIN_SUBSYSTEM,
b69e47
-			              "cos_dn_defs_cb - "
b69e47
-			              "Failed to get parent dn of cos definition %s.\n",
b69e47
-			              pDn->val);
b69e47
-			if (!pCosTemplateDn) {
b69e47
-				if (!pCosTargetTree) {
b69e47
-					slapi_log_err(SLAPI_LOG_ERR, COS_PLUGIN_SUBSYSTEM, "cos_dn_defs_cb - cosTargetTree and cosTemplateDn are not set.\n");
b69e47
-				} else {
b69e47
-					slapi_log_err(SLAPI_LOG_ERR, COS_PLUGIN_SUBSYSTEM, "cos_dn_defs_cb - cosTemplateDn is not set.\n");
b69e47
-				}
b69e47
-			} else if (!pCosTargetTree) {
b69e47
-				slapi_log_err(SLAPI_LOG_ERR, COS_PLUGIN_SUBSYSTEM, "cos_dn_defs_cb - cosTargetTree is not set.\n");
b69e47
-			}
b69e47
-		}
b69e47
-	}
b69e47
-	
b69e47
-	/*
b69e47
-	determine the type of class of service scheme 
b69e47
-	*/
b69e47
-	
b69e47
-	if(pObjectclass)
b69e47
-	{
b69e47
-		if(cos_cache_attrval_exists(pObjectclass, "cosDefinition"))
b69e47
-		{
b69e47
-			cosType = COSTYPE_CLASSIC;
b69e47
-		}
b69e47
-		else if(cos_cache_attrval_exists(pObjectclass, "cosClassicDefinition"))
b69e47
-		{
b69e47
-			cosType = COSTYPE_CLASSIC;
b69e47
-			
b69e47
-		}
b69e47
-		else if(cos_cache_attrval_exists(pObjectclass, "cosPointerDefinition"))
b69e47
-		{
b69e47
-			cosType = COSTYPE_POINTER;
b69e47
-			
b69e47
-		}
b69e47
-		else if(cos_cache_attrval_exists(pObjectclass, "cosIndirectDefinition"))
b69e47
-		{
b69e47
-			cosType = COSTYPE_INDIRECT;
b69e47
-			
b69e47
-		}
b69e47
-		else
b69e47
-			cosType = COSTYPE_BADTYPE;
b69e47
-	}
b69e47
-	
b69e47
-	/*	
b69e47
-	we should now have a full definition, 
b69e47
-	do some sanity checks because we don't
b69e47
-	want bogus entries in the cache 
b69e47
-	then ship it
b69e47
-	*/
b69e47
-	
b69e47
-	/* these must exist */
b69e47
-	if(pDn && pObjectclass && 
b69e47
-		(
b69e47
-		(cosType == COSTYPE_CLASSIC &&
b69e47
-		pCosTemplateDn && 
b69e47
-		pCosSpecifier &&   
b69e47
-		pCosAttribute ) 
b69e47
-		||
b69e47
-		(cosType == COSTYPE_POINTER &&
b69e47
-		pCosTemplateDn && 
b69e47
-		pCosAttribute ) 
b69e47
-		||
b69e47
-		(cosType == COSTYPE_INDIRECT &&
b69e47
-		pCosSpecifier &&   
b69e47
-		pCosAttribute ) 
b69e47
-		)
b69e47
-		)
b69e47
-	{
b69e47
-		int rc = 0;
b69e47
-	/*
b69e47
-	we'll leave the referential integrity stuff
b69e47
-	up to the referint plug-in and assume all
b69e47
-	is good - if it's not then we just have a
b69e47
-	useless definition and we'll nag copiously later.
b69e47
-		*/
b69e47
-		char *pTmpDn = slapi_ch_strdup(pDn->val); /* because dn gets hosed on error */
b69e47
-		
b69e47
-		if(!(rc = cos_cache_add_defn(info->pDefs, &pDn, cosType,
b69e47
-								&pCosTargetTree, &pCosTemplateDn, 
b69e47
-								&pCosSpecifier, &pCosAttribute,
b69e47
-								&pCosOverrides, &pCosOperational,
b69e47
-								&pCosMerge, &pCosOpDefault))) {
b69e47
-			info->ret = 0;  /* we have succeeded to add the defn*/
b69e47
-		} else {
b69e47
-			/*
b69e47
-			 * Failed but we will continue the search for other defs
b69e47
-			 * Don't reset info->ret....it keeps track of any success
b69e47
-			*/
b69e47
-			if ( rc == COS_DEF_ERROR_NO_TEMPLATES) {
b69e47
-				slapi_log_err(SLAPI_LOG_ERR, COS_PLUGIN_SUBSYSTEM, "cos_dn_defs_cb - Skipping CoS Definition %s"
b69e47
-					"--no CoS Templates found, which should be added before the CoS Definition.\n",
b69e47
-					pTmpDn);
b69e47
-			} else {
b69e47
-				slapi_log_err(SLAPI_LOG_ERR, COS_PLUGIN_SUBSYSTEM, "cos_dn_defs_cb - Skipping CoS Definition %s\n"
b69e47
-					"--error(%d)\n",
b69e47
-					pTmpDn, rc);
b69e47
-			}
b69e47
-		}
b69e47
-		
b69e47
-		slapi_ch_free_string(&pTmpDn);
b69e47
-	}
b69e47
-	else
b69e47
-	{
b69e47
-	/* 
b69e47
-	this definition is brain dead - bail
b69e47
-	if we have a dn use it to report, if not then *really* bad
b69e47
-	things are going on
b69e47
-		*/
b69e47
-		if(pDn)
b69e47
-		{
b69e47
-			slapi_log_err(SLAPI_LOG_ERR, COS_PLUGIN_SUBSYSTEM, "cos_dn_defs_cb - "
b69e47
-				"Incomplete cos definition detected in %s, discarding from cache.\n",pDn->val);
b69e47
-		}
b69e47
-		else
b69e47
-			slapi_log_err(SLAPI_LOG_ERR, COS_PLUGIN_SUBSYSTEM, "cos_dn_defs_cb - "
b69e47
-				"Incomplete cos definition detected, no DN to report, discarding from cache.\n");
b69e47
-		
b69e47
-		if(pCosTargetTree)
b69e47
-			cos_cache_del_attrval_list(&pCosTargetTree);
b69e47
-		if(pCosTemplateDn)
b69e47
-			cos_cache_del_attrval_list(&pCosTemplateDn);
b69e47
-		if(pCosSpecifier)
b69e47
-			cos_cache_del_attrval_list(&pCosSpecifier);
b69e47
-		if(pCosAttribute)
b69e47
-			cos_cache_del_attrval_list(&pCosAttribute);
b69e47
-		if(pDn)
b69e47
-			cos_cache_del_attrval_list(&pDn);
b69e47
-	}
b69e47
+        slapi_ch_free_string(&pTmpDn);
b69e47
+    } else {
b69e47
+        /*
b69e47
+    this definition is brain dead - bail
b69e47
+    if we have a dn use it to report, if not then *really* bad
b69e47
+    things are going on
b69e47
+        */
b69e47
+        if (pDn) {
b69e47
+            slapi_log_err(SLAPI_LOG_ERR, COS_PLUGIN_SUBSYSTEM, "cos_dn_defs_cb - "
b69e47
+                                                               "Incomplete cos definition detected in %s, discarding from cache.\n",
b69e47
+                          pDn->val);
b69e47
+        } else
b69e47
+            slapi_log_err(SLAPI_LOG_ERR, COS_PLUGIN_SUBSYSTEM, "cos_dn_defs_cb - "
b69e47
+                                                               "Incomplete cos definition detected, no DN to report, discarding from cache.\n");
b69e47
+
b69e47
+        if (pCosTargetTree)
b69e47
+            cos_cache_del_attrval_list(&pCosTargetTree);
b69e47
+        if (pCosTemplateDn)
b69e47
+            cos_cache_del_attrval_list(&pCosTemplateDn);
b69e47
+        if (pCosSpecifier)
b69e47
+            cos_cache_del_attrval_list(&pCosSpecifier);
b69e47
+        if (pCosAttribute)
b69e47
+            cos_cache_del_attrval_list(&pCosAttribute);
b69e47
+        if (pDn)
b69e47
+            cos_cache_del_attrval_list(&pDn);
b69e47
+    }
b69e47
 bail:
b69e47
 	/* we don't keep the objectclasses, so lets free them */
b69e47
 	if(pObjectclass) {
b69e47
diff --git a/ldap/servers/plugins/roles/roles_cache.c b/ldap/servers/plugins/roles/roles_cache.c
b69e47
index 3697eaa97..3e1724963 100644
b69e47
--- a/ldap/servers/plugins/roles/roles_cache.c
b69e47
+++ b/ldap/servers/plugins/roles/roles_cache.c
b69e47
@@ -48,9 +48,6 @@ static char *allUserAttributes[] = {
b69e47
 /* views scoping */
b69e47
 static void **views_api;
b69e47
 
b69e47
-/* Service provider handler */
b69e47
-static vattr_sp_handle *vattr_handle = NULL;
b69e47
-
b69e47
 /* List of nested roles */
b69e47
 typedef struct _role_object_nested {
b69e47
 	Slapi_DN *dn;	/* value of attribute nsroledn in a nested role definition */
b69e47
@@ -224,13 +221,16 @@ int roles_cache_init()
b69e47
 
b69e47
 	/* Register a callback on backends creation|modification|deletion, 
b69e47
       so that we update the corresponding cache */
b69e47
-	slapi_register_backend_state_change(NULL, roles_cache_trigger_update_suffix);
b69e47
-   
b69e47
-	if ( slapi_vattrspi_register((vattr_sp_handle **)&vattr_handle, 
b69e47
-									roles_sp_get_value, 
b69e47
-									roles_sp_compare_value, 
b69e47
-									roles_sp_list_types) )
b69e47
-	{
b69e47
+    slapi_register_backend_state_change(NULL, roles_cache_trigger_update_suffix);
b69e47
+
b69e47
+    /* Service provider handler - only used once! and freed by vattr! */
b69e47
+    vattr_sp_handle *vattr_handle = NULL;
b69e47
+
b69e47
+
b69e47
+    if (slapi_vattrspi_register((vattr_sp_handle **)&vattr_handle,
b69e47
+                                roles_sp_get_value,
b69e47
+                                roles_sp_compare_value,
b69e47
+                                roles_sp_list_types)) {
b69e47
         slapi_log_err(SLAPI_LOG_ERR, ROLES_PLUGIN_SUBSYSTEM,
b69e47
                "roles_cache_init - slapi_vattrspi_register failed\n");
b69e47
 
b69e47
@@ -648,22 +648,20 @@ void roles_cache_stop()
b69e47
 
b69e47
     slapi_log_err(SLAPI_LOG_PLUGIN, ROLES_PLUGIN_SUBSYSTEM, "--> roles_cache_stop\n");
b69e47
 
b69e47
-	/* Go through all the roles list and trigger the associated structure */
b69e47
-	slapi_rwlock_wrlock(global_lock);
b69e47
-	current_role = roles_list;
b69e47
-	while ( current_role )
b69e47
-	{
b69e47
-		slapi_lock_mutex(current_role->change_lock);
b69e47
-		current_role->keeprunning = 0;	
b69e47
-		next_role = current_role->next;
b69e47
-		slapi_notify_condvar(current_role->something_changed, 1 );
b69e47
-		slapi_unlock_mutex(current_role->change_lock);
b69e47
-
b69e47
-		current_role = next_role;
b69e47
-	}
b69e47
-	slapi_rwlock_unlock(global_lock);
b69e47
-	slapi_ch_free((void **)&vattr_handle);
b69e47
-	roles_list = NULL;
b69e47
+    /* Go through all the roles list and trigger the associated structure */
b69e47
+    slapi_rwlock_wrlock(global_lock);
b69e47
+    current_role = roles_list;
b69e47
+    while (current_role) {
b69e47
+        slapi_lock_mutex(current_role->change_lock);
b69e47
+        current_role->keeprunning = 0;
b69e47
+        next_role = current_role->next;
b69e47
+        slapi_notify_condvar(current_role->something_changed, 1);
b69e47
+        slapi_unlock_mutex(current_role->change_lock);
b69e47
+
b69e47
+        current_role = next_role;
b69e47
+    }
b69e47
+    slapi_rwlock_unlock(global_lock);
b69e47
+    roles_list = NULL;
b69e47
 
b69e47
     slapi_log_err(SLAPI_LOG_PLUGIN, ROLES_PLUGIN_SUBSYSTEM, "<-- roles_cache_stop\n");
b69e47
 }
b69e47
diff --git a/ldap/servers/slapd/vattr.c b/ldap/servers/slapd/vattr.c
b69e47
index ef4d7f279..84e01cd62 100644
b69e47
--- a/ldap/servers/slapd/vattr.c
b69e47
+++ b/ldap/servers/slapd/vattr.c
b69e47
@@ -1843,8 +1843,15 @@ static int vattr_map_create(void)
b69e47
 	return 0;
b69e47
 }
b69e47
 
b69e47
-void vattr_map_entry_free(vattr_map_entry *vae) {
b69e47
-    slapi_ch_free((void **)&(vae->sp_list));
b69e47
+void
b69e47
+vattr_map_entry_free(vattr_map_entry *vae)
b69e47
+{
b69e47
+    vattr_sp_handle *list_entry = vae->sp_list;
b69e47
+    while (list_entry != NULL) {
b69e47
+        vattr_sp_handle *next_entry = list_entry->next;
b69e47
+        slapi_ch_free((void **)&list_entry);
b69e47
+        list_entry = next_entry;
b69e47
+    }
b69e47
     slapi_ch_free_string(&(vae->type_name));
b69e47
     slapi_ch_free((void **)&vae);
b69e47
 }
b69e47
@@ -2134,16 +2141,9 @@ int slapi_vattr_schema_check_type(Slapi_Entry *e, char *type)
b69e47
 
b69e47
 vattr_map_entry *vattr_map_entry_new(char *type_name, vattr_sp_handle *sph, void* hint)
b69e47
 {
b69e47
-	vattr_map_entry *result = NULL;
b69e47
-	vattr_sp_handle *sp_copy = NULL;
b69e47
-
b69e47
-	sp_copy = (vattr_sp_handle*)slapi_ch_calloc(1, sizeof (vattr_sp_handle));
b69e47
-	sp_copy->sp = sph->sp;
b69e47
-	sp_copy->hint = hint;
b69e47
-
b69e47
-	result = (vattr_map_entry*)slapi_ch_calloc(1, sizeof (vattr_map_entry));
b69e47
-	result->type_name = slapi_ch_strdup(type_name);
b69e47
-	result->sp_list = sp_copy;
b69e47
+    vattr_map_entry *result = (vattr_map_entry *)slapi_ch_calloc(1, sizeof(vattr_map_entry));
b69e47
+    result->type_name = slapi_ch_strdup(type_name);
b69e47
+    result->sp_list = sph;
b69e47
 
b69e47
 	/* go get schema */
b69e47
 	result->objectclasses = vattr_map_entry_build_schema(type_name);
b69e47
@@ -2259,6 +2259,16 @@ we'd need to hold a lock on the read path, which we don't want to do.
b69e47
 So any SP which relinquishes its need to handle a type needs to continue
b69e47
 to handle the calls on it, but return nothing */
b69e47
 /* DBDB need to sort out memory ownership here, it's not quite right */
b69e47
+/*
b69e47
+ * This function was inconsistent. We would allocated and "kind of",
b69e47
+ * copy the sp_handle here for the vattr_map_entry_new path. But we
b69e47
+ * would "take ownership" for the existing entry and the list addition
b69e47
+ * path. Instead now, EVERY sp_handle we take, we take ownership of
b69e47
+ * and the CALLER must allocate a new one each time.
b69e47
+ *
b69e47
+ * Better idea, is that regattr should just take the fn pointers
b69e47
+ * and callers never *see* the sp_handle structure at all.
b69e47
+ */
b69e47
 
b69e47
 int vattr_map_sp_insert(char *type_to_add, vattr_sp_handle *sp, void *hint)
b69e47
 {
b69e47
-- 
b69e47
2.13.6
b69e47