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

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