diff --git a/SOURCES/0073-Ticket-49180-backport-1.3.6-errors-log-filled-with-a.patch b/SOURCES/0073-Ticket-49180-backport-1.3.6-errors-log-filled-with-a.patch new file mode 100644 index 0000000..ae197fb --- /dev/null +++ b/SOURCES/0073-Ticket-49180-backport-1.3.6-errors-log-filled-with-a.patch @@ -0,0 +1,50 @@ +From 1787e9ffda09f9ec8518ceaede5cf1ef014c5d17 Mon Sep 17 00:00:00 2001 +From: Ludwig Krispenz +Date: Wed, 27 Sep 2017 10:58:36 +0200 +Subject: [PATCH] Ticket: 49180 - backport 1.3.6 errors log filled with + attrlist_replace - attr_replace + + Bug: If a RUV contains the same URL with different replica IDs the created referrals contain duplicates + + Fix: check duplicate referrals + + Reviewed by: Mark, thanks +--- + ldap/servers/plugins/replication/repl5_ruv.c | 13 ++++++++++++- + 1 file changed, 12 insertions(+), 1 deletion(-) + +diff --git a/ldap/servers/plugins/replication/repl5_ruv.c b/ldap/servers/plugins/replication/repl5_ruv.c +index 39449b6..7f34059 100644 +--- a/ldap/servers/plugins/replication/repl5_ruv.c ++++ b/ldap/servers/plugins/replication/repl5_ruv.c +@@ -1502,7 +1502,17 @@ ruv_replica_count (const RUV *ruv) + * Extract all the referral URL's from the RUV (but self URL), + * returning them in an array of strings, that + * the caller must free. ++ * We also check and remove duplicates (caused by unclean RUVs) + */ ++static int ++ruv_referral_exists(unsigned char *purl, char **refs, int count) ++{ ++ for (size_t j=0; jreplica_purl!=NULL) && + (slapi_utf8casecmp((unsigned char *)replica->replica_purl, +- (unsigned char *)mypurl) != 0)) ++ (unsigned char *)mypurl) != 0) && ++ !ruv_referral_exists((unsigned char *)replica->replica_purl, r, i)) + { + r[i]= slapi_ch_strdup(replica->replica_purl); + i++; +-- +2.9.5 + diff --git a/SOURCES/0074-Ticket-48894-harden-valueset_array_to_sorted_quick-v.patch b/SOURCES/0074-Ticket-48894-harden-valueset_array_to_sorted_quick-v.patch new file mode 100644 index 0000000..36fab2f --- /dev/null +++ b/SOURCES/0074-Ticket-48894-harden-valueset_array_to_sorted_quick-v.patch @@ -0,0 +1,39 @@ +From 91c80c06affa3f4bfe106d2291efc360ab2b421d Mon Sep 17 00:00:00 2001 +From: Mark Reynolds +Date: Thu, 26 Oct 2017 10:03:39 -0400 +Subject: [PATCH] Ticket 48894 - harden valueset_array_to_sorted_quick valueset + access + +Description: It's possible during the sorting of a valueset to access an + array element past the allocated size, and also go below the index 0. + +https://pagure.io/389-ds-base/issue/48894 + +Reviewed by: nweiderm (Thanks!) + +(cherry picked from commit 2086d052e338ddcbcf6bd3222617991641573a12) +--- + ldap/servers/slapd/valueset.c | 4 ++-- + 1 file changed, 2 insertions(+), 2 deletions(-) + +diff --git a/ldap/servers/slapd/valueset.c b/ldap/servers/slapd/valueset.c +index 8a824ac4a..e22bc9c39 100644 +--- a/ldap/servers/slapd/valueset.c ++++ b/ldap/servers/slapd/valueset.c +@@ -1054,11 +1054,11 @@ valueset_array_to_sorted_quick (const Slapi_Attr *a, Slapi_ValueSet *vs, size_t + while (1) { + do { + i++; +- } while ( valueset_value_cmp(a, vs->va[vs->sorted[i]], vs->va[pivot]) < 0); ++ } while (i < vs->max && valueset_value_cmp(a, vs->va[vs->sorted[i]], vs->va[pivot]) < 0); + + do { + j--; +- } while ( valueset_value_cmp(a, vs->va[vs->sorted[j]], vs->va[pivot]) > 0); ++ } while (valueset_value_cmp(a, vs->va[vs->sorted[j]], vs->va[pivot]) > 0 && j > 0); + + if (i >= j) { + break; +-- +2.13.6 + diff --git a/SOURCES/0075-Ticket-49401-improve-valueset-sorted-performance-on-.patch b/SOURCES/0075-Ticket-49401-improve-valueset-sorted-performance-on-.patch new file mode 100644 index 0000000..f385154 --- /dev/null +++ b/SOURCES/0075-Ticket-49401-improve-valueset-sorted-performance-on-.patch @@ -0,0 +1,248 @@ +From 64b9d015523b4ae379ff2d72fc73da173be8a712 Mon Sep 17 00:00:00 2001 +From: Mohammad Nweider +Date: Wed, 18 Oct 2017 13:02:15 +0000 +Subject: [PATCH] Ticket 49401 - improve valueset sorted performance on delete + +Bug Description: valueset sorted maintains a list of syntax sorted +references to the attributes of the entry. During addition these are +modified and added properly, so they stay sorted. + +However, in the past to maintain the sorted property, during a delete +we would need to remove the vs->sorted array, and recreate it via qsort, + +While this was an improvement from past (where we would removed +vs->sorted during an attr delete), it still has performance implications +on very very large datasets, IE 50,000 member groups with +addition/deletion, large entry caches and replication. + +Fix Description: Implement a new algorithm that is able to maintain +existing sort data in a near linear time. + +https://pagure.io/389-ds-base/issue/49401 + +Author: nweiderm, wibrown + +Review by: wibrown, lkrispen, tbordaz (Thanks nweiderm!) + +(cherry picked from commit a43a8efc7907116146b505ac40f18fac71f474b0) +--- + ldap/servers/slapd/valueset.c | 171 +++++++++++++++++++++++++----------------- + 1 file changed, 102 insertions(+), 69 deletions(-) + +diff --git a/ldap/servers/slapd/valueset.c b/ldap/servers/slapd/valueset.c +index e22bc9c39..ae0a13fdc 100644 +--- a/ldap/servers/slapd/valueset.c ++++ b/ldap/servers/slapd/valueset.c +@@ -741,7 +741,10 @@ valueset_array_purge(const Slapi_Attr *a, Slapi_ValueSet *vs, const CSN *csn) + size_t i = 0; + size_t j = 0; + int nextValue = 0; ++ int nv = 0; + int numValues = 0; ++ Slapi_Value **va2 = NULL; ++ int *sorted2 = NULL; + + /* Loop over all the values freeing the old ones. */ + for(i = 0; i < vs->num; i++) +@@ -752,91 +755,122 @@ valueset_array_purge(const Slapi_Attr *a, Slapi_ValueSet *vs, const CSN *csn) + } else { + j = i; + } +- csnset_purge(&(vs->va[j]->v_csnset),csn); +- if (vs->va[j]->v_csnset == NULL) { +- slapi_value_free(&vs->va[j]); +- vs->va[j] = NULL; +- } else if (vs->va[j] != NULL) { +- /* This value survived, we should count it. */ +- numValues++; ++ if (vs->va[j]) { ++ csnset_purge(&(vs->va[j]->v_csnset),csn); ++ if (vs->va[j]->v_csnset == NULL) { ++ slapi_value_free(&vs->va[j]); ++ /* Set the removed value to NULL so we know later to skip it */ ++ vs->va[j] = NULL; ++ if (vs->sorted) { ++ /* Mark the value in sorted for removal */ ++ vs->sorted[i] = -1; ++ } ++ } else { ++ /* This value survived, we should count it. */ ++ numValues++; ++ } + } + } + +- /* Now compact the value/sorted list. */ ++ /* Compact vs->va and vs->sorted only when there're ++ * remaining values ie: numValues is greater than 0 */ + /* +- * Because we want to preserve the sorted array, this is complicated. ++ * Algorithm explination: We start with a pair of arrays - the attrs, and the sorted array that provides ++ * a lookup into it: ++ * ++ * va: [d e a c b] sorted: [2 4 3 0 1] ++ * ++ * When we remove the element b, we NULL it, and we have to mark the place where it "was" as a -1 to ++ * flag it's removal. ++ * ++ * va: [d e a c NULL] sorted: [2 -1 3 0 1] ++ * ++ * Now a second va is created with the reduced allocation, ++ * ++ * va2: [ X X X X ] .... + * +- * We have an array of values: +- * [ b, a, c, NULL, e, NULL, NULL, d] +- * And an array of indicies that are sorted. +- * [ 1, 0, 2, 7, 4, 3, 5, 6 ] +- * Were we to iterate over the sorted array, we get refs to the values in +- * some order. +- * The issue is now we must *remove* from both the values *and* the sorted. ++ * Now we loop over sorted, skipping -1 that we find. In a new counter we create new sorted ++ * references, and move the values compacting them in the process. ++ * va: [d e a c NULL] ++ * va2: [a x x x] ++ * sorted: [_0 -1 3 0 1] + * +- * Previously, we just discarded this, because too hard. Now we try to keep +- * it. The issue is that this is surprisingly hard to actually keep in +- * sync. ++ * Looping a few more times would yield: + * +- * We can't just blindly move the values down: That breaks the sorted array +- * and we would need to iterate over the sorted array multiple times to +- * achieve this. ++ * va2: [a c x x] ++ * sorted: [_0 _1 3 0 1] ++ * ++ * va2: [a c d x] ++ * sorted: [_0 _1 _2 0 1] ++ * ++ * va2: [a c d e] ++ * sorted: [_0 _1 _2 _3 1] ++ * ++ * Not only does this sort va, but with sorted, we have a faster lookup, and it will benefit cache ++ * lookup. + * +- * It's actually going to be easier to just ditch the sorted, compact vs +- * and then qsort the array. + */ ++ if (numValues > 0) { ++ if(vs->sorted) { ++ /* Let's allocate va2 and sorted2 */ ++ va2 = (Slapi_Value **) slapi_ch_malloc( (numValues + 1) * sizeof(Slapi_Value *)); ++ sorted2 = (int *) slapi_ch_malloc( (numValues + 1)* sizeof(int)); ++ } + +- j = 0; +- while (nextValue < numValues && j < vs->num) +- { +- /* nextValue is what we are looking at now +- * j tracks along the array getting next elements. +- * +- * [ b, a, c, NULL, e, NULL, NULL, d] +- * ^nv ^j +- * [ b, a, c, e, NULL, NULL, NULL, d] +- * ^nv ^j +- * [ b, a, c, e, NULL, NULL, NULL, d] +- * ^nv ^j +- * [ b, a, c, e, NULL, NULL, NULL, d] +- * ^nv ^j +- * [ b, a, c, e, NULL, NULL, NULL, d] +- * ^nv ^j +- * [ b, a, c, e, d, NULL, NULL, NULL] +- * ^nv ^j +- */ +- if (vs->va[nextValue] == NULL) { +- /* Advance j till we find something */ +- while (vs->va[j] == NULL) { +- j++; ++ /* I is the index for the *new* va2 array */ ++ for(i=0; inum; i++) { ++ if (vs->sorted) { ++ /* Skip any removed values from the index */ ++ while((nv < vs->num) && (-1 == vs->sorted[nv])) { ++ nv++; ++ } ++ /* We have a remaining value, add it to the va */ ++ if(nv < vs->num) { ++ va2[i] = vs->va[vs->sorted[nv]]; ++ sorted2[i] = i; ++ nv++; ++ } ++ } else { ++ while((nextValue < vs->num) && (NULL == vs->va[nextValue])) { ++ nextValue++; ++ } ++ ++ if(nextValue < vs->num) { ++ vs->va[i] = vs->va[nextValue]; ++ nextValue++; ++ } else { ++ break; ++ } + } +- /* We have something! */ +- vs->va[nextValue] = vs->va[j]; ++ } ++ ++ if (vs->sorted) { ++ /* Finally replace the valuearray and adjust num, max */ ++ slapi_ch_free((void **)&vs->va); ++ slapi_ch_free((void **)&vs->sorted); ++ vs->va = va2; ++ vs->sorted = sorted2; ++ vs->num = numValues; ++ vs->max = vs->num + 1; ++ } else { ++ vs->num = numValues; ++ } ++ ++ for (j = vs->num; j < vs->max; j++) { + vs->va[j] = NULL; ++ if (vs->sorted) { ++ vs->sorted[j] = -1; ++ } + } +- nextValue++; +- } +- /* Fix up the number of values */ +- vs->num = numValues; +- /* Should we re-alloc values to be smaller? */ +- /* Other parts of DS are lazy. Lets clean our list */ +- for (j = vs->num; j < vs->max; j++) { +- vs->va[j] = NULL; ++ } else { ++ slapi_valueset_done(vs); + } + +- /* All the values were deleted, we can discard the whole array. */ +- if(vs->num == 0) { +- if(vs->sorted) { +- slapi_ch_free ((void **)&vs->sorted); +- } +- slapi_ch_free ((void **)&vs->va); +- vs->va = NULL; +- vs->max = 0; +- } else if (vs->sorted != NULL) { +- /* We still have values! rebuild the sorted array */ ++ /* We still have values but not sorted array! rebuild it */ ++ if(vs->num > VALUESET_ARRAY_SORT_THRESHOLD && vs->sorted == NULL) { ++ vs->sorted = (int *) slapi_ch_malloc( vs->max* sizeof(int)); + valueset_array_to_sorted(a, vs); + } +- + #ifdef DEBUG + PR_ASSERT(vs->num == 0 || (vs->num > 0 && vs->va[0] != NULL)); + size_t index = 0; +@@ -847,7 +881,6 @@ valueset_array_purge(const Slapi_Attr *a, Slapi_ValueSet *vs, const CSN *csn) + PR_ASSERT(vs->va[index] == NULL); + } + #endif +- + /* return the number of remaining values */ + return numValues; + } +-- +2.13.6 + diff --git a/SOURCES/0076-Ticket-49401-Fix-compiler-incompatible-pointer-types.patch b/SOURCES/0076-Ticket-49401-Fix-compiler-incompatible-pointer-types.patch new file mode 100644 index 0000000..08fdf39 --- /dev/null +++ b/SOURCES/0076-Ticket-49401-Fix-compiler-incompatible-pointer-types.patch @@ -0,0 +1,54 @@ +From 43c73ca572af6a4bdc9b5994a9640f4d4e713cc2 Mon Sep 17 00:00:00 2001 +From: Mohammad Nweider +Date: Wed, 25 Oct 2017 16:26:54 +0000 +Subject: [PATCH] Ticket 49401 - Fix compiler incompatible-pointer-types + warnings + +Bug Description: vs->sorted was integer pointer in older versions, + but now it's size_t pointer, this is causing compiler warnings + during the build + +Fix Description: use size_t pointers instead of integer pointers for vs->sorted and sorted2 + +Review By: mreynolds + +Signed-off-by: Mark Reynolds +(cherry picked from commit 52ba2aba49935989152010aee0d40b01d7a78432) +--- + ldap/servers/slapd/valueset.c | 6 +++--- + 1 file changed, 3 insertions(+), 3 deletions(-) + +diff --git a/ldap/servers/slapd/valueset.c b/ldap/servers/slapd/valueset.c +index ae0a13fdc..8730d9f56 100644 +--- a/ldap/servers/slapd/valueset.c ++++ b/ldap/servers/slapd/valueset.c +@@ -744,7 +744,7 @@ valueset_array_purge(const Slapi_Attr *a, Slapi_ValueSet *vs, const CSN *csn) + int nv = 0; + int numValues = 0; + Slapi_Value **va2 = NULL; +- int *sorted2 = NULL; ++ size_t *sorted2 = NULL; + + /* Loop over all the values freeing the old ones. */ + for(i = 0; i < vs->num; i++) +@@ -814,7 +814,7 @@ valueset_array_purge(const Slapi_Attr *a, Slapi_ValueSet *vs, const CSN *csn) + if(vs->sorted) { + /* Let's allocate va2 and sorted2 */ + va2 = (Slapi_Value **) slapi_ch_malloc( (numValues + 1) * sizeof(Slapi_Value *)); +- sorted2 = (int *) slapi_ch_malloc( (numValues + 1)* sizeof(int)); ++ sorted2 = (size_t *) slapi_ch_malloc( (numValues + 1)* sizeof(size_t)); + } + + /* I is the index for the *new* va2 array */ +@@ -868,7 +868,7 @@ valueset_array_purge(const Slapi_Attr *a, Slapi_ValueSet *vs, const CSN *csn) + + /* We still have values but not sorted array! rebuild it */ + if(vs->num > VALUESET_ARRAY_SORT_THRESHOLD && vs->sorted == NULL) { +- vs->sorted = (int *) slapi_ch_malloc( vs->max* sizeof(int)); ++ vs->sorted = (size_t *) slapi_ch_malloc( vs->max* sizeof(size_t)); + valueset_array_to_sorted(a, vs); + } + #ifdef DEBUG +-- +2.13.6 + diff --git a/SOURCES/0077-Ticket-48235-Remove-memberOf-global-lock.patch b/SOURCES/0077-Ticket-48235-Remove-memberOf-global-lock.patch new file mode 100644 index 0000000..2dccc5c --- /dev/null +++ b/SOURCES/0077-Ticket-48235-Remove-memberOf-global-lock.patch @@ -0,0 +1,874 @@ +From 229f61f5f54aeb9e1a1756f731dfe7bcedbf148c Mon Sep 17 00:00:00 2001 +From: Mark Reynolds +Date: Fri, 13 Oct 2017 07:09:08 -0400 +Subject: [PATCH 06/10] Ticket 48235 - Remove memberOf global lock + +Bug Description: The memberOf global lock no longer servers a purpose since + the plugin is BETXN. This was causing potential deadlocks + when multiple backends are used. + +Fix Description: Remove the lock, and rework the fixup/ancestors caches/hashtables. + Instead of reusing a single cache, we create a fresh cache + when we copy the plugin config (which only happens at the start + of an operation). Then we destroy the caches when we free + the config. + +https://pagure.io/389-ds-base/issue/48235 + +Reviewed by: tbordaz & firstyear(Thanks!!) +--- + ldap/servers/plugins/memberof/memberof.c | 312 +++--------------------- + ldap/servers/plugins/memberof/memberof.h | 17 ++ + ldap/servers/plugins/memberof/memberof_config.c | 152 +++++++++++- + 3 files changed, 200 insertions(+), 281 deletions(-) + +diff --git a/ldap/servers/plugins/memberof/memberof.c b/ldap/servers/plugins/memberof/memberof.c +index 9bbe13c9c..bbf47dd49 100644 +--- a/ldap/servers/plugins/memberof/memberof.c ++++ b/ldap/servers/plugins/memberof/memberof.c +@@ -49,13 +49,10 @@ static void* _PluginID = NULL; + static Slapi_DN* _ConfigAreaDN = NULL; + static Slapi_RWLock *config_rwlock = NULL; + static Slapi_DN* _pluginDN = NULL; +-static PRMonitor *memberof_operation_lock = 0; + MemberOfConfig *qsortConfig = 0; + static int usetxn = 0; + static int premodfn = 0; +-#define MEMBEROF_HASHTABLE_SIZE 1000 +-static PLHashTable *fixup_entry_hashtable = NULL; /* global hash table protected by memberof_lock (memberof_operation_lock) */ +-static PLHashTable *group_ancestors_hashtable = NULL; /* global hash table protected by memberof_lock (memberof_operation_lock) */ ++ + + typedef struct _memberofstringll + { +@@ -73,18 +70,7 @@ typedef struct _memberof_get_groups_data + PRBool use_cache; + } memberof_get_groups_data; + +-/* The key to access the hash table is the normalized DN +- * The normalized DN is stored in the value because: +- * - It is used in slapi_valueset_find +- * - It is used to fill the memberof_get_groups_data.group_norm_vals +- */ +-typedef struct _memberof_cached_value +-{ +- char *key; +- char *group_dn_val; +- char *group_ndn_val; +- int valid; +-} memberof_cached_value; ++ + struct cache_stat + { + int total_lookup; +@@ -189,14 +175,9 @@ static int memberof_fix_memberof_callback(Slapi_Entry *e, void *callback_data); + static int memberof_entry_in_scope(MemberOfConfig *config, Slapi_DN *sdn); + static int memberof_add_objectclass(char *auto_add_oc, const char *dn); + static int memberof_add_memberof_attr(LDAPMod **mods, const char *dn, char *add_oc); +-static PLHashTable *hashtable_new(); +-static void fixup_hashtable_empty(char *msg); +-static PLHashTable *hashtable_new(); +-static void ancestor_hashtable_empty(char *msg); +-static void ancestor_hashtable_entry_free(memberof_cached_value *entry); +-static memberof_cached_value *ancestors_cache_lookup(const char *ndn); +-static PRBool ancestors_cache_remove(const char *ndn); +-static PLHashEntry *ancestors_cache_add(const void *key, void *value); ++static memberof_cached_value *ancestors_cache_lookup(MemberOfConfig *config, const char *ndn); ++static PRBool ancestors_cache_remove(MemberOfConfig *config, const char *ndn); ++static PLHashEntry *ancestors_cache_add(MemberOfConfig *config, const void *key, void *value); + + /*** implementation ***/ + +@@ -375,12 +356,6 @@ int memberof_postop_start(Slapi_PBlock *pb) + slapi_log_err(SLAPI_LOG_TRACE, MEMBEROF_PLUGIN_SUBSYSTEM, + "--> memberof_postop_start\n" ); + +- memberof_operation_lock = PR_NewMonitor(); +- if(0 == memberof_operation_lock) +- { +- rc = -1; +- goto bail; +- } + if(config_rwlock == NULL){ + if((config_rwlock = slapi_new_rwlock()) == NULL){ + rc = -1; +@@ -388,9 +363,6 @@ int memberof_postop_start(Slapi_PBlock *pb) + } + } + +- fixup_entry_hashtable = hashtable_new(); +- group_ancestors_hashtable = hashtable_new(); +- + /* Set the alternate config area if one is defined. */ + slapi_pblock_get(pb, SLAPI_PLUGIN_CONFIG_AREA, &config_area); + if (config_area) +@@ -482,18 +454,7 @@ int memberof_postop_close(Slapi_PBlock *pb) + slapi_sdn_free(&_pluginDN); + slapi_destroy_rwlock(config_rwlock); + config_rwlock = NULL; +- PR_DestroyMonitor(memberof_operation_lock); +- memberof_operation_lock = NULL; +- +- if (fixup_entry_hashtable) { +- fixup_hashtable_empty("memberof_postop_close empty fixup_entry_hastable"); +- PL_HashTableDestroy(fixup_entry_hashtable); +- } + +- if (group_ancestors_hashtable) { +- ancestor_hashtable_empty("memberof_postop_close empty group_ancestors_hashtable"); +- PL_HashTableDestroy(group_ancestors_hashtable); +- } + slapi_log_err(SLAPI_LOG_TRACE, MEMBEROF_PLUGIN_SUBSYSTEM, + "<-- memberof_postop_close\n" ); + return 0; +@@ -554,7 +515,7 @@ int memberof_postop_del(Slapi_PBlock *pb) + { + int ret = SLAPI_PLUGIN_SUCCESS; + MemberOfConfig *mainConfig = NULL; +- MemberOfConfig configCopy = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}; ++ MemberOfConfig configCopy = {0}; + Slapi_DN *sdn; + void *caller_id = NULL; + +@@ -583,9 +544,6 @@ int memberof_postop_del(Slapi_PBlock *pb) + } + memberof_copy_config(&configCopy, memberof_get_config()); + memberof_unlock_config(); +- +- /* get the memberOf operation lock */ +- memberof_lock(); + + /* remove this DN from the + * membership lists of groups +@@ -594,7 +552,6 @@ int memberof_postop_del(Slapi_PBlock *pb) + slapi_log_err(SLAPI_LOG_ERR, MEMBEROF_PLUGIN_SUBSYSTEM, + "memberof_postop_del - Error deleting dn (%s) from group. Error (%d)\n", + slapi_sdn_get_dn(sdn),ret); +- memberof_unlock(); + goto bail; + } + +@@ -618,7 +575,6 @@ int memberof_postop_del(Slapi_PBlock *pb) + } + } + } +- memberof_unlock(); + bail: + memberof_free_config(&configCopy); + } +@@ -813,7 +769,7 @@ memberof_call_foreach_dn(Slapi_PBlock *pb __attribute__((unused)), Slapi_DN *sdn + memberof_cached_value *ht_grp = NULL; + const char *ndn = slapi_sdn_get_ndn(sdn); + +- ht_grp = ancestors_cache_lookup((const void *) ndn); ++ ht_grp = ancestors_cache_lookup(config, (const void *) ndn); + if (ht_grp) { + #if MEMBEROF_CACHE_DEBUG + slapi_log_err(SLAPI_LOG_PLUGIN, MEMBEROF_PLUGIN_SUBSYSTEM, "memberof_call_foreach_dn: Ancestors of %s already cached (%x)\n", ndn, ht_grp); +@@ -960,7 +916,7 @@ int memberof_postop_modrdn(Slapi_PBlock *pb) + if(memberof_oktodo(pb)) + { + MemberOfConfig *mainConfig = 0; +- MemberOfConfig configCopy = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}; ++ MemberOfConfig configCopy = {0}; + struct slapi_entry *pre_e = NULL; + struct slapi_entry *post_e = NULL; + Slapi_DN *pre_sdn = 0; +@@ -988,8 +944,6 @@ int memberof_postop_modrdn(Slapi_PBlock *pb) + 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)) +@@ -1060,7 +1014,6 @@ int memberof_postop_modrdn(Slapi_PBlock *pb) + } + } + } +- memberof_unlock(); + bail: + memberof_free_config(&configCopy); + } +@@ -1220,7 +1173,7 @@ int memberof_postop_modify(Slapi_PBlock *pb) + { + int config_copied = 0; + MemberOfConfig *mainConfig = 0; +- MemberOfConfig configCopy = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}; ++ MemberOfConfig configCopy = {0}; + + /* get the mod set */ + slapi_pblock_get(pb, SLAPI_MODIFY_MODS, &mods); +@@ -1267,8 +1220,6 @@ int memberof_postop_modify(Slapi_PBlock *pb) + { + int op = slapi_mod_get_operation(smod); + +- memberof_lock(); +- + /* the modify op decides the function */ + switch(op & ~LDAP_MOD_BVALUES) + { +@@ -1280,7 +1231,6 @@ int memberof_postop_modify(Slapi_PBlock *pb) + "memberof_postop_modify - Failed to add dn (%s) to target. " + "Error (%d)\n", slapi_sdn_get_dn(sdn), ret ); + slapi_mod_done(next_mod); +- memberof_unlock(); + goto bail; + } + break; +@@ -1299,7 +1249,6 @@ int memberof_postop_modify(Slapi_PBlock *pb) + "memberof_postop_modify - Failed to replace list (%s). " + "Error (%d)\n", slapi_sdn_get_dn(sdn), ret ); + slapi_mod_done(next_mod); +- memberof_unlock(); + goto bail; + } + } +@@ -1311,7 +1260,6 @@ int memberof_postop_modify(Slapi_PBlock *pb) + "memberof_postop_modify: failed to remove dn (%s). " + "Error (%d)\n", slapi_sdn_get_dn(sdn), ret ); + slapi_mod_done(next_mod); +- memberof_unlock(); + goto bail; + } + } +@@ -1326,7 +1274,6 @@ int memberof_postop_modify(Slapi_PBlock *pb) + "memberof_postop_modify - Failed to replace values in dn (%s). " + "Error (%d)\n", slapi_sdn_get_dn(sdn), ret ); + slapi_mod_done(next_mod); +- memberof_unlock(); + goto bail; + } + break; +@@ -1342,8 +1289,6 @@ int memberof_postop_modify(Slapi_PBlock *pb) + break; + } + } +- +- memberof_unlock(); + } + + slapi_mod_done(next_mod); +@@ -1398,7 +1343,7 @@ int memberof_postop_add(Slapi_PBlock *pb) + if(memberof_oktodo(pb) && (sdn = memberof_getsdn(pb))) + { + struct slapi_entry *e = NULL; +- MemberOfConfig configCopy = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}; ++ MemberOfConfig configCopy = {0}; + MemberOfConfig *mainConfig; + slapi_pblock_get( pb, SLAPI_ENTRY_POST_OP, &e ); + +@@ -1424,8 +1369,6 @@ int memberof_postop_add(Slapi_PBlock *pb) + int i = 0; + Slapi_Attr *attr = 0; + +- memberof_lock(); +- + for (i = 0; configCopy.groupattrs && configCopy.groupattrs[i]; i++) + { + if(0 == slapi_entry_attr_find(e, configCopy.groupattrs[i], &attr)) +@@ -1438,8 +1381,6 @@ int memberof_postop_add(Slapi_PBlock *pb) + } + } + } +- +- memberof_unlock(); + memberof_free_config(&configCopy); + } + } +@@ -2201,7 +2142,7 @@ dump_cache_entry(memberof_cached_value *double_check, const char *msg) + * the firsts elements of the array has 'valid=1' and the dn/ndn of group it belong to + */ + static void +-cache_ancestors(Slapi_Value **member_ndn_val, memberof_get_groups_data *groups) ++cache_ancestors(MemberOfConfig *config, Slapi_Value **member_ndn_val, memberof_get_groups_data *groups) + { + Slapi_ValueSet *groupvals = *((memberof_get_groups_data*)groups)->groupvals; + Slapi_Value *sval; +@@ -2298,14 +2239,14 @@ cache_ancestors(Slapi_Value **member_ndn_val, memberof_get_groups_data *groups) + #if MEMBEROF_CACHE_DEBUG + dump_cache_entry(cache_entry, key); + #endif +- if (ancestors_cache_add((const void*) key_copy, (void *) cache_entry) == NULL) { ++ if (ancestors_cache_add(config, (const void*) key_copy, (void *) cache_entry) == NULL) { + slapi_log_err( SLAPI_LOG_FATAL, MEMBEROF_PLUGIN_SUBSYSTEM, "cache_ancestors: Failed to cache ancestor of %s\n", key); + ancestor_hashtable_entry_free(cache_entry); + slapi_ch_free ((void**)&cache_entry); + return; + } + #if MEMBEROF_CACHE_DEBUG +- if (double_check = ancestors_cache_lookup((const void*) key)) { ++ if (double_check = ancestors_cache_lookup(config, (const void*) key)) { + dump_cache_entry(double_check, "read back"); + } + #endif +@@ -2390,9 +2331,9 @@ memberof_get_groups_r(MemberOfConfig *config, Slapi_DN *member_sdn, + memberof_get_groups_callback, &member_data, &cached, member_data.use_cache); + + merge_ancestors(&member_ndn_val, &member_data, data); +- if (!cached && member_data.use_cache) +- cache_ancestors(&member_ndn_val, &member_data); +- ++ if (!cached && member_data.use_cache) { ++ cache_ancestors(config, &member_ndn_val, &member_data); ++ } + + slapi_value_free(&member_ndn_val); + slapi_valueset_free(groupvals); +@@ -2969,46 +2910,9 @@ int memberof_qsort_compare(const void *a, const void *b) + val1, val2); + } + +-/* betxn: This locking mechanism is necessary to guarantee the memberof +- * consistency */ +-void memberof_lock() +-{ +- if (usetxn) { +- PR_EnterMonitor(memberof_operation_lock); +- } +- if (fixup_entry_hashtable) { +- fixup_hashtable_empty("memberof_lock"); +- } +- if (group_ancestors_hashtable) { +- ancestor_hashtable_empty("memberof_lock empty group_ancestors_hashtable"); +- memset(&cache_stat, 0, sizeof(cache_stat)); +- } +-} +- +-void memberof_unlock() +-{ +- if (group_ancestors_hashtable) { +- ancestor_hashtable_empty("memberof_unlock empty group_ancestors_hashtable"); +-#if MEMBEROF_CACHE_DEBUG +- slapi_log_err(SLAPI_LOG_FATAL, MEMBEROF_PLUGIN_SUBSYSTEM, "cache statistics: total lookup %d (success %d), add %d, remove %d, enum %d\n", +- cache_stat.total_lookup, cache_stat.successfull_lookup, +- cache_stat.total_add, cache_stat.total_remove, cache_stat.total_enumerate); +- slapi_log_err(SLAPI_LOG_FATAL, MEMBEROF_PLUGIN_SUBSYSTEM, "cache statistics duration: lookup %ld, add %ld, remove %ld, enum %ld\n", +- cache_stat.cumul_duration_lookup, cache_stat.cumul_duration_add, +- cache_stat.cumul_duration_remove, cache_stat.cumul_duration_enumerate); +-#endif +- } +- if (fixup_entry_hashtable) { +- fixup_hashtable_empty("memberof_lock"); +- } +- if (usetxn) { +- PR_ExitMonitor(memberof_operation_lock); +- } +-} +- + void memberof_fixup_task_thread(void *arg) + { +- MemberOfConfig configCopy = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}; ++ MemberOfConfig configCopy = {0}; + Slapi_Task *task = (Slapi_Task *)arg; + task_data *td = NULL; + int rc = 0; +@@ -3068,14 +2972,8 @@ void memberof_fixup_task_thread(void *arg) + } + } + +- /* get the memberOf operation lock */ +- memberof_lock(); +- + /* do real work */ + rc = memberof_fix_memberof(&configCopy, task, td); +- +- /* release the memberOf operation lock */ +- memberof_unlock(); + + done: + if (usetxn && fixup_pb) { +@@ -3240,7 +3138,7 @@ int memberof_fix_memberof(MemberOfConfig *config, Slapi_Task *task, task_data *t + } + + static memberof_cached_value * +-ancestors_cache_lookup(const char *ndn) ++ancestors_cache_lookup(MemberOfConfig *config, const char *ndn) + { + memberof_cached_value *e; + #if defined(DEBUG) && defined(HAVE_CLOCK_GETTIME) +@@ -3258,7 +3156,7 @@ ancestors_cache_lookup(const char *ndn) + } + #endif + +- e = (memberof_cached_value *) PL_HashTableLookupConst(group_ancestors_hashtable, (const void *) ndn); ++ e = (memberof_cached_value *) PL_HashTableLookupConst(config->ancestors_cache, (const void *) ndn); + + #if defined(DEBUG) && defined(HAVE_CLOCK_GETTIME) + if (start) { +@@ -3274,7 +3172,7 @@ ancestors_cache_lookup(const char *ndn) + + } + static PRBool +-ancestors_cache_remove(const char *ndn) ++ancestors_cache_remove(MemberOfConfig *config, const char *ndn) + { + PRBool rc; + #if defined(DEBUG) && defined(HAVE_CLOCK_GETTIME) +@@ -3292,7 +3190,7 @@ ancestors_cache_remove(const char *ndn) + } + #endif + +- rc = PL_HashTableRemove(group_ancestors_hashtable, (const void *) ndn); ++ rc = PL_HashTableRemove(config->ancestors_cache, (const void *) ndn); + + #if defined(DEBUG) && defined(HAVE_CLOCK_GETTIME) + if (start) { +@@ -3305,7 +3203,7 @@ ancestors_cache_remove(const char *ndn) + } + + static PLHashEntry * +-ancestors_cache_add(const void *key, void *value) ++ancestors_cache_add(MemberOfConfig *config, const void *key, void *value) + { + PLHashEntry *e; + #if defined(DEBUG) && defined(HAVE_CLOCK_GETTIME) +@@ -3322,7 +3220,7 @@ ancestors_cache_add(const void *key, void *value) + } + #endif + +- e = PL_HashTableAdd(group_ancestors_hashtable, key, value); ++ e = PL_HashTableAdd(config->ancestors_cache, key, value); + + #if defined(DEBUG) && defined(HAVE_CLOCK_GETTIME) + if (start) { +@@ -3360,10 +3258,11 @@ int memberof_fix_memberof_callback(Slapi_Entry *e, void *callback_data) + goto bail; + } + +- /* Check if the entry has not already been fixed */ ++ /* Check if the entry has not already been fixed */ + ndn = slapi_sdn_get_ndn(sdn); +- if (ndn && fixup_entry_hashtable && PL_HashTableLookupConst(fixup_entry_hashtable, (void*) ndn)) { +- slapi_log_err(SLAPI_LOG_PLUGIN, MEMBEROF_PLUGIN_SUBSYSTEM, "memberof_fix_memberof_callback: Entry %s already fixed up\n", ndn); ++ if (ndn && config->fixup_cache && PL_HashTableLookupConst(config->fixup_cache, (void*) ndn)) { ++ slapi_log_err(SLAPI_LOG_PLUGIN, MEMBEROF_PLUGIN_SUBSYSTEM, ++ "memberof_fix_memberof_callback: Entry %s already fixed up\n", ndn); + goto bail; + } + +@@ -3383,9 +3282,9 @@ int memberof_fix_memberof_callback(Slapi_Entry *e, void *callback_data) + #if MEMBEROF_CACHE_DEBUG + slapi_log_err(SLAPI_LOG_PLUGIN, MEMBEROF_PLUGIN_SUBSYSTEM, "memberof_fix_memberof_callback: This is NOT a group %s\n", ndn); + #endif +- ht_grp = ancestors_cache_lookup((const void *) ndn); ++ ht_grp = ancestors_cache_lookup(config, (const void *) ndn); + if (ht_grp) { +- if (ancestors_cache_remove((const void *) ndn)) { ++ if (ancestors_cache_remove(config, (const void *) ndn)) { + slapi_log_err(SLAPI_LOG_PLUGIN, MEMBEROF_PLUGIN_SUBSYSTEM, "memberof_fix_memberof_callback: free cached values for %s\n", ndn); + ancestor_hashtable_entry_free(ht_grp); + slapi_ch_free((void **) &ht_grp); +@@ -3400,6 +3299,7 @@ int memberof_fix_memberof_callback(Slapi_Entry *e, void *callback_data) + } + } + } ++ + /* If we found some groups, replace the existing memberOf attribute + * with the found values. */ + if (groups && slapi_valueset_count(groups)) +@@ -3439,9 +3339,9 @@ int memberof_fix_memberof_callback(Slapi_Entry *e, void *callback_data) + slapi_valueset_free(groups); + + /* records that this entry has been fixed up */ +- if (fixup_entry_hashtable) { ++ if (config->fixup_cache) { + dn_copy = slapi_ch_strdup(ndn); +- if (PL_HashTableAdd(fixup_entry_hashtable, dn_copy, dn_copy) == NULL) { ++ if (PL_HashTableAdd(config->fixup_cache, dn_copy, dn_copy) == NULL) { + slapi_log_err(SLAPI_LOG_FATAL, MEMBEROF_PLUGIN_SUBSYSTEM, "memberof_fix_memberof_callback: " + "failed to add dn (%s) in the fixup hashtable; NSPR error - %d\n", + dn_copy, PR_GetError()); +@@ -3539,150 +3439,8 @@ memberof_add_objectclass(char *auto_add_oc, const char *dn) + return rc; + } + +-static PRIntn memberof_hash_compare_keys(const void *v1, const void *v2) +-{ +- PRIntn rc; +- if (0 == strcasecmp((const char *) v1, (const char *) v2)) { +- rc = 1; +- } else { +- rc = 0; +- } +- return rc; +-} +- +-static PRIntn memberof_hash_compare_values(const void *v1, const void *v2) +-{ +- PRIntn rc; +- if ((char *) v1 == (char *) v2) { +- rc = 1; +- } else { +- rc = 0; +- } +- return rc; +-} +- +-/* +- * Hashing function using Bernstein's method +- */ +-static PLHashNumber memberof_hash_fn(const void *key) +-{ +- PLHashNumber hash = 5381; +- unsigned char *x = (unsigned char *)key; +- int c; +- +- while ((c = *x++)){ +- hash = ((hash << 5) + hash) ^ c; +- } +- return hash; +-} +- +-/* allocates the plugin hashtable +- * This hash table is used by operation and is protected from +- * concurrent operations with the memberof_lock (if not usetxn, memberof_lock +- * is not implemented and the hash table will be not used. +- * +- * The hash table contains all the DN of the entries for which the memberof +- * attribute has been computed/updated during the current operation +- * +- * hash table should be empty at the beginning and end of the plugin callback +- */ +-static PLHashTable *hashtable_new() +-{ +- if (!usetxn) { +- return NULL; +- } +- +- return PL_NewHashTable(MEMBEROF_HASHTABLE_SIZE, +- memberof_hash_fn, +- memberof_hash_compare_keys, +- memberof_hash_compare_values, NULL, NULL); +-} +-/* this function called for each hash node during hash destruction */ +-static PRIntn fixup_hashtable_remove(PLHashEntry *he, PRIntn index, void *arg) +-{ +- char *dn_copy; +- +- if (he == NULL) { +- return HT_ENUMERATE_NEXT; +- } +- dn_copy = (char*) he->value; +- slapi_ch_free_string(&dn_copy); +- +- return HT_ENUMERATE_REMOVE; +-} +- +-static void fixup_hashtable_empty(char *msg) +-{ +- if (fixup_entry_hashtable) { +- PL_HashTableEnumerateEntries(fixup_entry_hashtable, fixup_hashtable_remove, msg); +- } +-} +- +- +-/* allocates the plugin hashtable +- * This hash table is used by operation and is protected from +- * concurrent operations with the memberof_lock (if not usetxn, memberof_lock +- * is not implemented and the hash table will be not used. +- * +- * The hash table contains all the DN of the entries for which the memberof +- * attribute has been computed/updated during the current operation +- * +- * hash table should be empty at the beginning and end of the plugin callback +- */ +- +-static +-void ancestor_hashtable_entry_free(memberof_cached_value *entry) +-{ +- int i; +- for (i = 0; entry[i].valid; i++) { +- slapi_ch_free((void **) &entry[i].group_dn_val); +- slapi_ch_free((void **) &entry[i].group_ndn_val); +- } +- /* Here we are at the ending element containing the key */ +- slapi_ch_free((void**) &entry[i].key); +-} +-/* this function called for each hash node during hash destruction */ +-static PRIntn ancestor_hashtable_remove(PLHashEntry *he, PRIntn index, void *arg) ++int ++memberof_use_txn() + { +- memberof_cached_value *group_ancestor_array; +- +- if (he == NULL) +- return HT_ENUMERATE_NEXT; +- +- +- group_ancestor_array = (memberof_cached_value *) he->value; +- ancestor_hashtable_entry_free(group_ancestor_array); +- slapi_ch_free((void **)&group_ancestor_array); +- +- return HT_ENUMERATE_REMOVE; ++ return usetxn; + } +- +-static void ancestor_hashtable_empty(char *msg) +-{ +-#if defined(DEBUG) && defined(HAVE_CLOCK_GETTIME) +- long int start; +- struct timespec tsnow; +-#endif +- +- if (group_ancestors_hashtable) { +- cache_stat.total_enumerate++; +-#if defined(DEBUG) && defined(HAVE_CLOCK_GETTIME) +- if (clock_gettime(CLOCK_REALTIME, &tsnow) != 0) { +- start = 0; +- } else { +- start = tsnow.tv_nsec; +- } +-#endif +- PL_HashTableEnumerateEntries(group_ancestors_hashtable, ancestor_hashtable_remove, msg); +- +-#if defined(DEBUG) && defined(HAVE_CLOCK_GETTIME) +- if (start) { +- if (clock_gettime(CLOCK_REALTIME, &tsnow) == 0) { +- cache_stat.cumul_duration_enumerate += (tsnow.tv_nsec - start); +- } +- } +-#endif +- } +- +-} +- +diff --git a/ldap/servers/plugins/memberof/memberof.h b/ldap/servers/plugins/memberof/memberof.h +index 9a3a6a25d..a01c4d247 100644 +--- a/ldap/servers/plugins/memberof/memberof.h ++++ b/ldap/servers/plugins/memberof/memberof.h +@@ -62,8 +62,22 @@ typedef struct memberofconfig { + int skip_nested; + int fixup_task; + char *auto_add_oc; ++ PLHashTable *ancestors_cache; ++ PLHashTable *fixup_cache; + } MemberOfConfig; + ++/* The key to access the hash table is the normalized DN ++ * The normalized DN is stored in the value because: ++ * - It is used in slapi_valueset_find ++ * - It is used to fill the memberof_get_groups_data.group_norm_vals ++ */ ++typedef struct _memberof_cached_value ++{ ++ char *key; ++ char *group_dn_val; ++ char *group_ndn_val; ++ int valid; ++} memberof_cached_value; + + /* + * functions +@@ -88,5 +102,8 @@ int memberof_apply_config (Slapi_PBlock *pb, Slapi_Entry* entryBefore, Slapi_Ent + void *memberof_get_plugin_id(void); + void memberof_release_config(void); + PRUint64 get_plugin_started(void); ++void ancestor_hashtable_entry_free(memberof_cached_value *entry); ++PLHashTable *hashtable_new(); ++int memberof_use_txn(); + + #endif /* _MEMBEROF_H_ */ +diff --git a/ldap/servers/plugins/memberof/memberof_config.c b/ldap/servers/plugins/memberof/memberof_config.c +index c3474bf2c..3cc7c4d9c 100644 +--- a/ldap/servers/plugins/memberof/memberof_config.c ++++ b/ldap/servers/plugins/memberof/memberof_config.c +@@ -14,12 +14,12 @@ + * memberof_config.c - configuration-related code for memberOf plug-in + * + */ +- ++#include "plhash.h" + #include +- + #include "memberof.h" + + #define MEMBEROF_CONFIG_FILTER "(objectclass=*)" ++#define MEMBEROF_HASHTABLE_SIZE 1000 + + /* + * The configuration attributes are contained in the plugin entry e.g. +@@ -33,7 +33,9 @@ + + /* + * function prototypes +- */ ++ */ ++static void fixup_hashtable_empty( MemberOfConfig *config, char *msg); ++static void ancestor_hashtable_empty(MemberOfConfig *config, char *msg); + static int memberof_validate_config (Slapi_PBlock *pb, Slapi_Entry* entryBefore, Slapi_Entry* e, + int *returncode, char *returntext, void *arg); + static int memberof_search (Slapi_PBlock *pb, Slapi_Entry* entryBefore, Slapi_Entry* e, +@@ -48,7 +50,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 = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}; ++static MemberOfConfig theConfig = {0}; + static Slapi_RWLock *memberof_config_lock = 0; + static int inited = 0; + +@@ -696,6 +698,12 @@ memberof_copy_config(MemberOfConfig *dest, MemberOfConfig *src) + { + if (dest && src) + { ++ /* Allocate our caches here since we only copy the config at the start of an op */ ++ if (memberof_use_txn() == 1){ ++ dest->ancestors_cache = hashtable_new(); ++ dest->fixup_cache = hashtable_new(); ++ } ++ + /* Check if the copy is already up to date */ + if (src->groupattrs) + { +@@ -799,6 +807,14 @@ memberof_free_config(MemberOfConfig *config) + slapi_ch_free_string(&config->memberof_attr); + memberof_free_scope(config->entryScopes, &config->entryScopeCount); + memberof_free_scope(config->entryScopeExcludeSubtrees, &config->entryExcludeScopeCount); ++ if (config->fixup_cache) { ++ fixup_hashtable_empty(config, "memberof_free_config empty fixup_entry_hastable"); ++ PL_HashTableDestroy(config->fixup_cache); ++ } ++ if (config->ancestors_cache) { ++ ancestor_hashtable_empty(config, "memberof_free_config empty group_ancestors_hashtable"); ++ PL_HashTableDestroy(config->ancestors_cache); ++ } + } + } + +@@ -1001,3 +1017,131 @@ bail: + + return ret; + } ++ ++ ++static PRIntn memberof_hash_compare_keys(const void *v1, const void *v2) ++{ ++ PRIntn rc; ++ if (0 == strcasecmp((const char *) v1, (const char *) v2)) { ++ rc = 1; ++ } else { ++ rc = 0; ++ } ++ return rc; ++} ++ ++static PRIntn memberof_hash_compare_values(const void *v1, const void *v2) ++{ ++ PRIntn rc; ++ if ((char *) v1 == (char *) v2) { ++ rc = 1; ++ } else { ++ rc = 0; ++ } ++ return rc; ++} ++ ++/* ++ * Hashing function using Bernstein's method ++ */ ++static PLHashNumber memberof_hash_fn(const void *key) ++{ ++ PLHashNumber hash = 5381; ++ unsigned char *x = (unsigned char *)key; ++ int c; ++ ++ while ((c = *x++)){ ++ hash = ((hash << 5) + hash) ^ c; ++ } ++ return hash; ++} ++ ++/* allocates the plugin hashtable ++ * This hash table is used by operation and is protected from ++ * concurrent operations with the memberof_lock (if not usetxn, memberof_lock ++ * is not implemented and the hash table will be not used. ++ * ++ * The hash table contains all the DN of the entries for which the memberof ++ * attribute has been computed/updated during the current operation ++ * ++ * hash table should be empty at the beginning and end of the plugin callback ++ */ ++PLHashTable *hashtable_new(int usetxn) ++{ ++ if (!usetxn) { ++ return NULL; ++ } ++ ++ return PL_NewHashTable(MEMBEROF_HASHTABLE_SIZE, ++ memberof_hash_fn, ++ memberof_hash_compare_keys, ++ memberof_hash_compare_values, NULL, NULL); ++} ++ ++/* this function called for each hash node during hash destruction */ ++static PRIntn fixup_hashtable_remove(PLHashEntry *he, PRIntn index __attribute__((unused)), void *arg __attribute__((unused))) ++{ ++ char *dn_copy; ++ ++ if (he == NULL) { ++ return HT_ENUMERATE_NEXT; ++ } ++ dn_copy = (char*) he->value; ++ slapi_ch_free_string(&dn_copy); ++ ++ return HT_ENUMERATE_REMOVE; ++} ++ ++static void fixup_hashtable_empty(MemberOfConfig *config, char *msg) ++{ ++ if (config->fixup_cache) { ++ PL_HashTableEnumerateEntries(config->fixup_cache, fixup_hashtable_remove, msg); ++ } ++} ++ ++ ++/* allocates the plugin hashtable ++ * This hash table is used by operation and is protected from ++ * concurrent operations with the memberof_lock (if not usetxn, memberof_lock ++ * is not implemented and the hash table will be not used. ++ * ++ * The hash table contains all the DN of the entries for which the memberof ++ * attribute has been computed/updated during the current operation ++ * ++ * hash table should be empty at the beginning and end of the plugin callback ++ */ ++ ++void ancestor_hashtable_entry_free(memberof_cached_value *entry) ++{ ++ int i; ++ ++ for (i = 0; entry[i].valid; i++) { ++ slapi_ch_free((void **) &entry[i].group_dn_val); ++ slapi_ch_free((void **) &entry[i].group_ndn_val); ++ } ++ /* Here we are at the ending element containing the key */ ++ slapi_ch_free((void**) &entry[i].key); ++} ++ ++/* this function called for each hash node during hash destruction */ ++static PRIntn ancestor_hashtable_remove(PLHashEntry *he, PRIntn index __attribute__((unused)), void *arg __attribute__((unused))) ++{ ++ memberof_cached_value *group_ancestor_array; ++ ++ if (he == NULL) { ++ return HT_ENUMERATE_NEXT; ++ } ++ group_ancestor_array = (memberof_cached_value *) he->value; ++ ancestor_hashtable_entry_free(group_ancestor_array); ++ slapi_ch_free((void **)&group_ancestor_array); ++ ++ return HT_ENUMERATE_REMOVE; ++} ++ ++static void ancestor_hashtable_empty(MemberOfConfig *config, char *msg) ++{ ++ if (config->ancestors_cache) { ++ PL_HashTableEnumerateEntries(config->ancestors_cache, ancestor_hashtable_remove, msg); ++ } ++ ++} +-- +2.13.6 + diff --git a/SOURCES/0078-Ticket-49402-Adding-a-database-entry-with-the-same-d.patch b/SOURCES/0078-Ticket-49402-Adding-a-database-entry-with-the-same-d.patch new file mode 100644 index 0000000..a0a24dc --- /dev/null +++ b/SOURCES/0078-Ticket-49402-Adding-a-database-entry-with-the-same-d.patch @@ -0,0 +1,41 @@ +From bc190eeaaffbb514f69664b4d033dc593a78683b Mon Sep 17 00:00:00 2001 +From: Mark Reynolds +Date: Mon, 16 Oct 2017 12:52:46 -0400 +Subject: [PATCH] Ticket 49402 - Adding a database entry with the same database + name that was deleted hangs server at shutdown + +Bug Description: At shutdown, after a backend was deleted, which also had a import + task run, the server hangs at shutdown. The issue is that the + import task destructor used the ldbm inst struct to see if it was + busy, but the inst was freed and the destructor was checking invalid + memory which caused a false positive on the "busy" check. + +Fix Description: Do not check if the instance is busy to tell if it's okay to remove + the task, instead just check the task's state. + +https://pagure.io/389-ds-base/issue/49402 + +Reviewed by: lkrispen(Thanks!) + +(cherry picked from commit bc6dbf15c160ac7e6c553133b2b936a981cfb7b6) +(cherry picked from commit 2ef4e813b8f6b92908ff553a704808cbbd425b5d) +--- + ldap/servers/slapd/back-ldbm/import.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/ldap/servers/slapd/back-ldbm/import.c b/ldap/servers/slapd/back-ldbm/import.c +index 7161bace1..f60df194b 100644 +--- a/ldap/servers/slapd/back-ldbm/import.c ++++ b/ldap/servers/slapd/back-ldbm/import.c +@@ -234,7 +234,7 @@ static void import_task_destroy(Slapi_Task *task) + return; + } + +- while(is_instance_busy(job->inst)){ ++ while (task->task_state == SLAPI_TASK_RUNNING) { + /* wait for the job to finish before freeing it */ + DS_Sleep(PR_SecondsToInterval(1)); + } +-- +2.13.6 + diff --git a/SOURCES/0079-Ticket-49439-cleanallruv-is-not-logging-information.patch b/SOURCES/0079-Ticket-49439-cleanallruv-is-not-logging-information.patch new file mode 100644 index 0000000..da266a6 --- /dev/null +++ b/SOURCES/0079-Ticket-49439-cleanallruv-is-not-logging-information.patch @@ -0,0 +1,239 @@ +From 8031684255007b42df3d08b80e674aefb0ebfb2d Mon Sep 17 00:00:00 2001 +From: Mark Reynolds +Date: Thu, 2 Nov 2017 12:55:11 -0400 +Subject: [PATCH] Ticket 49439 - cleanallruv is not logging information + +Bug Description: During the logging refector effro from ticket 48978 + a mistake was made and cleanruv_log() was using + LOG_NOTICE (which is not a true log level), it was + supposed to be SLAPI_LOG_NOTICE. + + We also use DEBUG defines to contorl the logging for + debug builds + +Fix Description: Remove the LDAP_DEBUG defines in cleanruv_log, and set + the correct logging severity level. + +https://pagure.io/389-ds-base/issue/49439 + +Reviewed by: firstyear(Thanks!) + +(cherry picked from commit e1f866a5e3ccce8e061e361c0e3dd11175a8acf2) +--- + .../plugins/replication/repl5_replica_config.c | 101 +++++++++++---------- + 1 file changed, 51 insertions(+), 50 deletions(-) + +diff --git a/ldap/servers/plugins/replication/repl5_replica_config.c b/ldap/servers/plugins/replication/repl5_replica_config.c +index 053103bd0..814f1cac0 100644 +--- a/ldap/servers/plugins/replication/repl5_replica_config.c ++++ b/ldap/servers/plugins/replication/repl5_replica_config.c +@@ -1911,12 +1911,13 @@ replica_cleanallruv_thread(void *arg) + /* + * need to sleep between passes + */ +- cleanruv_log(data->task, data->rid, CLEANALLRUV_ID, LOG_NOTICE, "Not all replicas have received the " +- "cleanallruv extended op, retrying in %d seconds",interval); +- if(!slapi_is_shutting_down()){ +- PR_Lock( notify_lock ); +- PR_WaitCondVar( notify_cvar, PR_SecondsToInterval(interval) ); +- PR_Unlock( notify_lock ); ++ cleanruv_log(data->task, data->rid, CLEANALLRUV_ID, SLAPI_LOG_NOTICE, "Not all replicas have received the " ++ "cleanallruv extended op, retrying in %d seconds", ++ interval); ++ if (!slapi_is_shutting_down()) { ++ PR_Lock(notify_lock); ++ PR_WaitCondVar(notify_cvar, PR_SecondsToInterval(interval)); ++ PR_Unlock(notify_lock); + } + if(interval < 14400){ /* 4 hour max */ + interval = interval * 2; +@@ -1952,8 +1953,8 @@ replica_cleanallruv_thread(void *arg) + found_dirty_rid = 0; + } else { + found_dirty_rid = 1; +- cleanruv_log(data->task, data->rid, CLEANALLRUV_ID, LOG_NOTICE, "Replica is not cleaned yet (%s)", +- agmt_get_long_name(agmt)); ++ cleanruv_log(data->task, data->rid, CLEANALLRUV_ID, SLAPI_LOG_NOTICE, "Replica is not cleaned yet (%s)", ++ agmt_get_long_name(agmt)); + break; + } + agmt_obj = agmtlist_get_next_agreement_for_replica (data->replica, agmt_obj); +@@ -1969,12 +1970,13 @@ replica_cleanallruv_thread(void *arg) + /* + * Need to sleep between passes unless we are shutting down + */ +- if (!slapi_is_shutting_down()){ +- cleanruv_log(data->task, data->rid, CLEANALLRUV_ID, LOG_NOTICE, "Replicas have not been cleaned yet, " +- "retrying in %d seconds", interval); +- PR_Lock( notify_lock ); +- PR_WaitCondVar( notify_cvar, PR_SecondsToInterval(interval) ); +- PR_Unlock( notify_lock ); ++ if (!slapi_is_shutting_down()) { ++ cleanruv_log(data->task, data->rid, CLEANALLRUV_ID, SLAPI_LOG_NOTICE, "Replicas have not been cleaned yet, " ++ "retrying in %d seconds", ++ interval); ++ PR_Lock(notify_lock); ++ PR_WaitCondVar(notify_cvar, PR_SecondsToInterval(interval)); ++ PR_Unlock(notify_lock); + } + + if(interval < 14400){ /* 4 hour max */ +@@ -2008,11 +2010,11 @@ done: + /* + * Shutdown or abort + */ +- if(!is_task_aborted(data->rid) || slapi_is_shutting_down()){ +- cleanruv_log(data->task, data->rid, CLEANALLRUV_ID, LOG_NOTICE, +- "Server shutting down. Process will resume at server startup"); ++ if (!is_task_aborted(data->rid) || slapi_is_shutting_down()) { ++ cleanruv_log(data->task, data->rid, CLEANALLRUV_ID, SLAPI_LOG_NOTICE, ++ "Server shutting down. Process will resume at server startup"); + } else { +- cleanruv_log(data->task, data->rid, CLEANALLRUV_ID, LOG_NOTICE, "Task aborted for rid(%d).",data->rid); ++ cleanruv_log(data->task, data->rid, CLEANALLRUV_ID, SLAPI_LOG_NOTICE, "Task aborted for rid(%d).", data->rid); + delete_cleaned_rid_config(data); + remove_cleaned_rid(data->rid); + } +@@ -2180,7 +2182,7 @@ check_replicas_are_done_cleaning(cleanruv_data *data ) + break; + } + +- cleanruv_log(data->task, data->rid, CLEANALLRUV_ID, LOG_NOTICE, ++ cleanruv_log(data->task, data->rid, CLEANALLRUV_ID, SLAPI_LOG_NOTICE, + "Not all replicas finished cleaning, retrying in %d seconds", + interval); + if(!slapi_is_shutting_down()){ +@@ -2289,12 +2291,12 @@ check_replicas_are_done_aborting(cleanruv_data *data ) + if(not_all_aborted == 0){ + break; + } +- cleanruv_log(data->task, data->rid, ABORT_CLEANALLRUV_ID, LOG_NOTICE, +- "Not all replicas finished aborting, retrying in %d seconds",interval); +- if(!slapi_is_shutting_down()){ +- PR_Lock( notify_lock ); +- PR_WaitCondVar( notify_cvar, PR_SecondsToInterval(interval) ); +- PR_Unlock( notify_lock ); ++ cleanruv_log(data->task, data->rid, ABORT_CLEANALLRUV_ID, SLAPI_LOG_NOTICE, ++ "Not all replicas finished aborting, retrying in %d seconds", interval); ++ if (!slapi_is_shutting_down()) { ++ PR_Lock(notify_lock); ++ PR_WaitCondVar(notify_cvar, PR_SecondsToInterval(interval)); ++ PR_Unlock(notify_lock); + } + if(interval < 14400){ /* 4 hour max */ + interval = interval * 2; +@@ -2336,8 +2338,8 @@ check_agmts_are_caught_up(cleanruv_data *data, char *maxcsn) + not_all_caughtup = 0; + } else { + not_all_caughtup = 1; +- cleanruv_log(data->task, data->rid, CLEANALLRUV_ID, LOG_NOTICE, +- "Replica not caught up (%s)",agmt_get_long_name(agmt)); ++ cleanruv_log(data->task, data->rid, CLEANALLRUV_ID, SLAPI_LOG_NOTICE, ++ "Replica not caught up (%s)", agmt_get_long_name(agmt)); + break; + } + agmt_obj = agmtlist_get_next_agreement_for_replica (data->replica, agmt_obj); +@@ -2346,12 +2348,12 @@ check_agmts_are_caught_up(cleanruv_data *data, char *maxcsn) + if(not_all_caughtup == 0 || is_task_aborted(data->rid) ){ + break; + } +- cleanruv_log(data->task, data->rid, CLEANALLRUV_ID, LOG_NOTICE, +- "Not all replicas caught up, retrying in %d seconds",interval); +- if(!slapi_is_shutting_down()){ +- PR_Lock( notify_lock ); +- PR_WaitCondVar( notify_cvar, PR_SecondsToInterval(interval) ); +- PR_Unlock( notify_lock ); ++ cleanruv_log(data->task, data->rid, CLEANALLRUV_ID, SLAPI_LOG_NOTICE, ++ "Not all replicas caught up, retrying in %d seconds", interval); ++ if (!slapi_is_shutting_down()) { ++ PR_Lock(notify_lock); ++ PR_WaitCondVar(notify_cvar, PR_SecondsToInterval(interval)); ++ PR_Unlock(notify_lock); + } + if(interval < 14400){ /* 4 hour max */ + interval = interval * 2; +@@ -2396,8 +2398,8 @@ check_agmts_are_alive(Replica *replica, ReplicaId rid, Slapi_Task *task) + not_all_alive = 0; + } else { + not_all_alive = 1; +- cleanruv_log(task, rid, CLEANALLRUV_ID, LOG_NOTICE, "Replica not online (%s)", +- agmt_get_long_name(agmt)); ++ cleanruv_log(task, rid, CLEANALLRUV_ID, SLAPI_LOG_NOTICE, "Replica not online (%s)", ++ agmt_get_long_name(agmt)); + break; + } + agmt_obj = agmtlist_get_next_agreement_for_replica (replica, agmt_obj); +@@ -2406,8 +2408,8 @@ check_agmts_are_alive(Replica *replica, ReplicaId rid, Slapi_Task *task) + if(not_all_alive == 0 || is_task_aborted(rid)){ + break; + } +- cleanruv_log(task, rid, CLEANALLRUV_ID, LOG_NOTICE, "Not all replicas online, retrying in %d seconds...", +- interval); ++ cleanruv_log(task, rid, CLEANALLRUV_ID, SLAPI_LOG_NOTICE, "Not all replicas online, retrying in %d seconds...", ++ interval); + + if(!slapi_is_shutting_down()){ + PR_Lock( notify_lock ); +@@ -3174,11 +3176,11 @@ replica_abort_task_thread(void *arg) + /* + * Need to sleep between passes. unless we are shutting down + */ +- if (!slapi_is_shutting_down()){ +- cleanruv_log(data->task, data->rid, ABORT_CLEANALLRUV_ID, LOG_NOTICE, "Retrying in %d seconds",interval); +- PR_Lock( notify_lock ); +- PR_WaitCondVar( notify_cvar, PR_SecondsToInterval(interval) ); +- PR_Unlock( notify_lock ); ++ if (!slapi_is_shutting_down()) { ++ cleanruv_log(data->task, data->rid, ABORT_CLEANALLRUV_ID, SLAPI_LOG_NOTICE, "Retrying in %d seconds", interval); ++ PR_Lock(notify_lock); ++ PR_WaitCondVar(notify_cvar, PR_SecondsToInterval(interval)); ++ PR_Unlock(notify_lock); + } + + if(interval < 14400){ /* 4 hour max */ +@@ -3296,9 +3298,10 @@ replica_cleanallruv_send_extop(Repl_Agmt *ra, cleanruv_data *clean_data, int che + /* extop was accepted */ + rc = 0; + } else { +- cleanruv_log(clean_data->task, clean_data->rid, CLEANALLRUV_ID, LOG_NOTICE, +- "Replica %s does not support the CLEANALLRUV task. " +- "Sending replica CLEANRUV task...", slapi_sdn_get_dn(agmt_get_dn_byref(ra))); ++ cleanruv_log(clean_data->task, clean_data->rid, CLEANALLRUV_ID, SLAPI_LOG_NOTICE, ++ "Replica %s does not support the CLEANALLRUV task. " ++ "Sending replica CLEANRUV task...", ++ slapi_sdn_get_dn(agmt_get_dn_byref(ra))); + /* + * Ok, this replica doesn't know about CLEANALLRUV, so just manually + * add the CLEANRUV task to the replica. +@@ -3463,9 +3466,9 @@ replica_cleanallruv_check_maxcsn(Repl_Agmt *agmt, char *basedn, char *rid_text, + csn_init_by_string(repl_max, remote_maxcsn); + if(csn_compare (repl_max, max) < 0){ + /* we are not caught up yet, free, and return */ +- cleanruv_log(task, atoi(rid_text), CLEANALLRUV_ID, LOG_NOTICE, +- "Replica maxcsn (%s) is not caught up with deleted replica's maxcsn(%s)", +- remote_maxcsn, maxcsn); ++ cleanruv_log(task, atoi(rid_text), CLEANALLRUV_ID, SLAPI_LOG_NOTICE, ++ "Replica maxcsn (%s) is not caught up with deleted replica's maxcsn(%s)", ++ remote_maxcsn, maxcsn); + rc = -1; + } else { + /* ok this replica is caught up */ +@@ -3636,7 +3639,6 @@ stop_ruv_cleaning() + void + cleanruv_log(Slapi_Task *task, int rid, char *task_type, int sev_level, char *fmt, ...) + { +-#ifdef LDAP_DEBUG + va_list ap1; + va_list ap2; + va_list ap3; +@@ -3661,7 +3663,6 @@ cleanruv_log(Slapi_Task *task, int rid, char *task_type, int sev_level, char *fm + va_end(ap2); + va_end(ap3); + va_end(ap4); +-#endif + } + + char * +-- +2.13.6 + diff --git a/SOURCES/0080-Ticket-49436-double-free-in-COS-in-some-conditions.patch b/SOURCES/0080-Ticket-49436-double-free-in-COS-in-some-conditions.patch new file mode 100644 index 0000000..fc70af1 --- /dev/null +++ b/SOURCES/0080-Ticket-49436-double-free-in-COS-in-some-conditions.patch @@ -0,0 +1,1008 @@ +From fc9a206c294fb5ea2401a9365f01ef2565799478 Mon Sep 17 00:00:00 2001 +From: William Brown +Date: Thu, 2 Nov 2017 13:32:41 +1000 +Subject: [PATCH] Ticket 49436 - double free in COS in some conditions + +Bug Description: virtualattrs and COS have some serious memory +ownership issues. What was happening is that COS with multiple +attributes using the same sp_handle would cause a structure +to be registered twice. During shutdown we would then trigger +a double free in the process. + +Fix Description: Change the behaviour of sp_handles to use a +handle *per* attribute we register to guarantee the assocation +between them. + +https://pagure.io/389-ds-base/issue/49436 + +Author: wibrown + +Review by: mreynolds, vashirov (Thanks!) + +(cherry pick from commit ee4428a3f5d2d8e37a7107c7dce9d622fc17d41c) +--- + dirsrvtests/tests/suites/cos/indirect_cos_test.py | 43 +- + ldap/servers/plugins/cos/cos_cache.c | 723 +++++++++++----------- + ldap/servers/plugins/roles/roles_cache.c | 50 +- + ldap/servers/slapd/vattr.c | 34 +- + 4 files changed, 406 insertions(+), 444 deletions(-) + +diff --git a/dirsrvtests/tests/suites/cos/indirect_cos_test.py b/dirsrvtests/tests/suites/cos/indirect_cos_test.py +index 1aac6b8ed..452edcdf8 100644 +--- a/dirsrvtests/tests/suites/cos/indirect_cos_test.py ++++ b/dirsrvtests/tests/suites/cos/indirect_cos_test.py +@@ -7,6 +7,7 @@ import subprocess + + from lib389 import Entry + from lib389.idm.user import UserAccounts ++from lib389.idm.domain import Domain + from lib389.topologies import topology_st as topo + from lib389._constants import (DEFAULT_SUFFIX, DN_DM, PASSWORD, HOST_STANDALONE, + SERVERID_STANDALONE, PORT_STANDALONE) +@@ -48,14 +49,8 @@ def check_user(inst): + def setup_subtree_policy(topo): + """Set up subtree password policy + """ +- try: +- topo.standalone.modify_s("cn=config", [(ldap.MOD_REPLACE, +- 'nsslapd-pwpolicy-local', +- 'on')]) +- except ldap.LDAPError as e: +- log.error('Failed to set fine-grained policy: error {}'.format( +- e.message['desc'])) +- raise e ++ ++ topo.standalone.config.set('nsslapd-pwpolicy-local', 'on') + + log.info('Create password policy for subtree {}'.format(OU_PEOPLE)) + try: +@@ -68,15 +63,9 @@ def setup_subtree_policy(topo): + OU_PEOPLE, e.message['desc'])) + raise e + +- log.info('Add pwdpolicysubentry attribute to {}'.format(OU_PEOPLE)) +- try: +- topo.standalone.modify_s(DEFAULT_SUFFIX, [(ldap.MOD_REPLACE, +- 'pwdpolicysubentry', +- PW_POLICY_CONT_PEOPLE2)]) +- except ldap.LDAPError as e: +- log.error('Failed to pwdpolicysubentry pw policy ' +- 'policy for {}: error {}'.format(OU_PEOPLE, e.message['desc'])) +- raise e ++ domain = Domain(topo.standalone, DEFAULT_SUFFIX) ++ domain.replace('pwdpolicysubentry', PW_POLICY_CONT_PEOPLE2) ++ + time.sleep(1) + + +@@ -116,12 +105,9 @@ def setup(topo, request): + """ + log.info('Add custom schema...') + try: +- ATTR_1 = ("( 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' )") +- ATTR_2 = ("( 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' )") +- OC = ("( xPerson-oid NAME 'xPerson' DESC '' SUP person STRUCTURAL MAY " + +- "( x-department $ x-en-ou ) X-ORIGIN 'user defined' )") ++ 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' )") ++ 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' )") ++ OC = (b"( xPerson-oid NAME 'xPerson' DESC '' SUP person STRUCTURAL MAY ( x-department $ x-en-ou ) X-ORIGIN 'user defined' )") + topo.standalone.modify_s("cn=schema", [(ldap.MOD_ADD, 'attributeTypes', ATTR_1), + (ldap.MOD_ADD, 'attributeTypes', ATTR_2), + (ldap.MOD_ADD, 'objectClasses', OC)]) +@@ -142,14 +128,9 @@ def setup(topo, request): + 'homeDirectory': '/home/test_user', + 'seeAlso': 'cn=cosTemplate,dc=example,dc=com' + } +- users.create(properties=user_properties) +- try: +- topo.standalone.modify_s(TEST_USER_DN, [(ldap.MOD_ADD, +- 'objectclass', +- 'xPerson')]) +- except ldap.LDAPError as e: +- log.fatal('Failed to add objectclass to user') +- raise e ++ user = users.create(properties=user_properties) ++ ++ user.add('objectClass', 'xPerson') + + # Setup COS + log.info("Setup indirect COS...") +diff --git a/ldap/servers/plugins/cos/cos_cache.c b/ldap/servers/plugins/cos/cos_cache.c +index 87d48908c..0e93183d2 100644 +--- a/ldap/servers/plugins/cos/cos_cache.c ++++ b/ldap/servers/plugins/cos/cos_cache.c +@@ -108,9 +108,6 @@ void * cos_get_plugin_identity(void); + #define COSTYPE_INDIRECT 3 + #define COS_DEF_ERROR_NO_TEMPLATES -2 + +-/* the global plugin handle */ +-static volatile vattr_sp_handle *vattr_handle = NULL; +- + /* both variables are protected by change_lock */ + static int cos_cache_notify_flag = 0; + static PRBool cos_cache_at_work = PR_FALSE; +@@ -289,75 +286,61 @@ static Slapi_CondVar *start_cond = NULL; + */ + int cos_cache_init(void) + { +- int ret = 0; +- +- slapi_log_err(SLAPI_LOG_TRACE, COS_PLUGIN_SUBSYSTEM, "--> cos_cache_init\n"); +- +- slapi_vattrcache_cache_none(); +- cache_lock = slapi_new_mutex(); +- change_lock = slapi_new_mutex(); +- stop_lock = slapi_new_mutex(); +- something_changed = slapi_new_condvar(change_lock); +- keeprunning =1; +- start_lock = slapi_new_mutex(); +- start_cond = slapi_new_condvar(start_lock); +- started = 0; +- +- if (stop_lock == NULL || +- change_lock == NULL || +- cache_lock == NULL || +- stop_lock == NULL || +- start_lock == NULL || +- start_cond == NULL || +- something_changed == NULL) +- { +- slapi_log_err(SLAPI_LOG_ERR, COS_PLUGIN_SUBSYSTEM, +- "cos_cache_init - Cannot create mutexes\n" ); +- ret = -1; +- goto out; +- } +- +- /* grab the views interface */ +- if(slapi_apib_get_interface(Views_v1_0_GUID, &views_api)) +- { +- /* lets be tolerant if views is disabled */ +- views_api = 0; +- } ++ int ret = 0; + +- if (slapi_vattrspi_register((vattr_sp_handle **)&vattr_handle, +- cos_cache_vattr_get, +- cos_cache_vattr_compare, +- cos_cache_vattr_types) != 0) +- { +- slapi_log_err(SLAPI_LOG_ERR, COS_PLUGIN_SUBSYSTEM, +- "cos_cache_init - Cannot register as service provider\n" ); +- ret = -1; +- goto out; +- } ++ slapi_log_err(SLAPI_LOG_TRACE, COS_PLUGIN_SUBSYSTEM, "--> cos_cache_init\n"); ++ ++ slapi_vattrcache_cache_none(); ++ cache_lock = slapi_new_mutex(); ++ change_lock = slapi_new_mutex(); ++ stop_lock = slapi_new_mutex(); ++ something_changed = slapi_new_condvar(change_lock); ++ keeprunning = 1; ++ start_lock = slapi_new_mutex(); ++ start_cond = slapi_new_condvar(start_lock); ++ started = 0; ++ ++ if (stop_lock == NULL || ++ change_lock == NULL || ++ cache_lock == NULL || ++ stop_lock == NULL || ++ start_lock == NULL || ++ start_cond == NULL || ++ something_changed == NULL) { ++ slapi_log_err(SLAPI_LOG_ERR, COS_PLUGIN_SUBSYSTEM, ++ "cos_cache_init - Cannot create mutexes\n"); ++ ret = -1; ++ goto out; ++ } + +- if ( PR_CreateThread (PR_USER_THREAD, +- cos_cache_wait_on_change, +- NULL, +- PR_PRIORITY_NORMAL, +- PR_GLOBAL_THREAD, +- PR_UNJOINABLE_THREAD, +- SLAPD_DEFAULT_THREAD_STACKSIZE) == NULL ) +- { +- slapi_log_err(SLAPI_LOG_ERR, COS_PLUGIN_SUBSYSTEM, +- "cos_cache_init - PR_CreateThread failed\n" ); +- ret = -1; +- goto out; +- } ++ /* grab the views interface */ ++ if (slapi_apib_get_interface(Views_v1_0_GUID, &views_api)) { ++ /* lets be tolerant if views is disabled */ ++ views_api = 0; ++ } + +- /* wait for that thread to get started */ +- if (ret == 0) { +- slapi_lock_mutex(start_lock); +- while (!started) { +- while (slapi_wait_condvar(start_cond, NULL) == 0); +- } +- slapi_unlock_mutex(start_lock); +- } ++ if (PR_CreateThread(PR_USER_THREAD, ++ cos_cache_wait_on_change, ++ NULL, ++ PR_PRIORITY_NORMAL, ++ PR_GLOBAL_THREAD, ++ PR_UNJOINABLE_THREAD, ++ SLAPD_DEFAULT_THREAD_STACKSIZE) == NULL) { ++ slapi_log_err(SLAPI_LOG_ERR, COS_PLUGIN_SUBSYSTEM, ++ "cos_cache_init - PR_CreateThread failed\n"); ++ ret = -1; ++ goto out; ++ } + ++ /* wait for that thread to get started */ ++ if (ret == 0) { ++ slapi_lock_mutex(start_lock); ++ while (!started) { ++ while (slapi_wait_condvar(start_cond, NULL) == 0) ++ ; ++ } ++ slapi_unlock_mutex(start_lock); ++ } + + out: + slapi_log_err(SLAPI_LOG_TRACE, COS_PLUGIN_SUBSYSTEM, "<-- cos_cache_init\n"); +@@ -752,321 +735,311 @@ struct dn_defs_info { + static int + cos_dn_defs_cb (Slapi_Entry* e, void *callback_data) + { +- struct dn_defs_info *info; +- cosAttrValue **pSneakyVal = 0; +- cosAttrValue *pObjectclass = 0; +- cosAttrValue *pCosTargetTree = 0; +- cosAttrValue *pCosTemplateDn = 0; +- cosAttrValue *pCosSpecifier = 0; +- cosAttrValue *pCosAttribute = 0; +- cosAttrValue *pCosOverrides = 0; +- cosAttrValue *pCosOperational = 0; +- cosAttrValue *pCosOpDefault = 0; +- cosAttrValue *pCosMerge = 0; +- cosAttrValue *pDn = 0; +- struct berval **dnVals; +- int cosType = 0; +- int valIndex = 0; +- Slapi_Attr *dnAttr; +- char *attrType = 0; +- char *norm_dn = NULL; +- info=(struct dn_defs_info *)callback_data; +- +- cos_cache_add_attrval(&pDn, slapi_entry_get_dn(e)); +- if(slapi_entry_first_attr(e, &dnAttr)) { +- goto bail; +- } ++ struct dn_defs_info *info; ++ cosAttrValue **pSneakyVal = 0; ++ cosAttrValue *pObjectclass = 0; ++ cosAttrValue *pCosTargetTree = 0; ++ cosAttrValue *pCosTemplateDn = 0; ++ cosAttrValue *pCosSpecifier = 0; ++ cosAttrValue *pCosAttribute = 0; ++ cosAttrValue *pCosOverrides = 0; ++ cosAttrValue *pCosOperational = 0; ++ cosAttrValue *pCosOpDefault = 0; ++ cosAttrValue *pCosMerge = 0; ++ cosAttrValue *pDn = 0; ++ struct berval **dnVals; ++ int cosType = 0; ++ int valIndex = 0; ++ Slapi_Attr *dnAttr; ++ char *attrType = 0; ++ char *norm_dn = NULL; ++ info = (struct dn_defs_info *)callback_data; ++ ++ cos_cache_add_attrval(&pDn, slapi_entry_get_dn(e)); ++ if (slapi_entry_first_attr(e, &dnAttr)) { ++ goto bail; ++ } + +- do { +- attrType = 0; +- /* we need to fill in the details of the definition now */ +- slapi_attr_get_type(dnAttr, &attrType); +- if(!attrType) { +- continue; +- } +- pSneakyVal = 0; +- if(!slapi_utf8casecmp((unsigned char*)attrType, (unsigned char*)"objectclass")) +- pSneakyVal = &pObjectclass; +- else if(!slapi_utf8casecmp((unsigned char*)attrType, (unsigned char*)"cosTargetTree")){ +- if(pCosTargetTree){ +- norm_dn = slapi_create_dn_string("%s", pCosTargetTree->val); +- if(norm_dn){ +- slapi_ch_free_string(&pCosTargetTree->val); +- pCosTargetTree->val = norm_dn; +- } +- } +- pSneakyVal = &pCosTargetTree; +- } else if(!slapi_utf8casecmp((unsigned char*)attrType, (unsigned char*)"cosTemplateDn")) +- pSneakyVal = &pCosTemplateDn; +- else if(!slapi_utf8casecmp((unsigned char*)attrType, (unsigned char*)"cosSpecifier")) +- pSneakyVal = &pCosSpecifier; +- else if(!slapi_utf8casecmp((unsigned char*)attrType, (unsigned char*)"cosAttribute")) +- pSneakyVal = &pCosAttribute; +- else if(!slapi_utf8casecmp((unsigned char*)attrType, (unsigned char*)"cosIndirectSpecifier")) +- pSneakyVal = &pCosSpecifier; +- if(!pSneakyVal) { +- continue; +- } +- /* It's a type we're interested in */ +- if(slapi_attr_get_bervals_copy(dnAttr, &dnVals)) { +- continue; +- } +- valIndex = 0; +- if(!dnVals) { +- continue; +- } +- for (valIndex = 0; dnVals[valIndex]; valIndex++) +- { +- if(!dnVals[valIndex]->bv_val) { +- continue; +- } +- /* +- parse any overide or default values +- and deal with them +- */ +- if(pSneakyVal == &pCosAttribute) +- { +- int qualifier_hit = 0; +- int op_qualifier_hit = 0; +- int merge_schemes_qualifier_hit = 0; +- int override_qualifier_hit =0; +- int default_qualifier_hit = 0; +- int operational_default_qualifier_hit = 0; +- do +- { +- qualifier_hit = 0; ++ do { ++ attrType = 0; ++ /* we need to fill in the details of the definition now */ ++ slapi_attr_get_type(dnAttr, &attrType); ++ if (!attrType) { ++ continue; ++ } ++ pSneakyVal = 0; ++ if (!slapi_utf8casecmp((unsigned char *)attrType, (unsigned char *)"objectclass")) ++ pSneakyVal = &pObjectclass; ++ else if (!slapi_utf8casecmp((unsigned char *)attrType, (unsigned char *)"cosTargetTree")) { ++ if (pCosTargetTree) { ++ norm_dn = slapi_create_dn_string("%s", pCosTargetTree->val); ++ if (norm_dn) { ++ slapi_ch_free_string(&pCosTargetTree->val); ++ pCosTargetTree->val = norm_dn; ++ } ++ } ++ pSneakyVal = &pCosTargetTree; ++ } else if (!slapi_utf8casecmp((unsigned char *)attrType, (unsigned char *)"cosTemplateDn")) ++ pSneakyVal = &pCosTemplateDn; ++ else if (!slapi_utf8casecmp((unsigned char *)attrType, (unsigned char *)"cosSpecifier")) ++ pSneakyVal = &pCosSpecifier; ++ else if (!slapi_utf8casecmp((unsigned char *)attrType, (unsigned char *)"cosAttribute")) ++ pSneakyVal = &pCosAttribute; ++ else if (!slapi_utf8casecmp((unsigned char *)attrType, (unsigned char *)"cosIndirectSpecifier")) ++ pSneakyVal = &pCosSpecifier; ++ if (!pSneakyVal) { ++ continue; ++ } ++ /* It's a type we're interested in */ ++ if (slapi_attr_get_bervals_copy(dnAttr, &dnVals)) { ++ continue; ++ } ++ valIndex = 0; ++ if (!dnVals) { ++ continue; ++ } ++ for (valIndex = 0; dnVals[valIndex]; valIndex++) { ++ if (!dnVals[valIndex]->bv_val) { ++ continue; ++ } ++ /* ++ parse any overide or default values ++ and deal with them ++ */ ++ if (pSneakyVal == &pCosAttribute) { ++ int qualifier_hit = 0; ++ int op_qualifier_hit = 0; ++ int merge_schemes_qualifier_hit = 0; ++ int override_qualifier_hit = 0; ++ int default_qualifier_hit = 0; ++ int operational_default_qualifier_hit = 0; ++ do { ++ qualifier_hit = 0; ++ ++ if (cos_cache_backwards_stricmp_and_clip(dnVals[valIndex]->bv_val, " operational")) { ++ /* matched */ ++ op_qualifier_hit = 1; ++ qualifier_hit = 1; ++ } ++ ++ if (cos_cache_backwards_stricmp_and_clip(dnVals[valIndex]->bv_val, " merge-schemes")) { ++ /* matched */ ++ merge_schemes_qualifier_hit = 1; ++ qualifier_hit = 1; ++ } ++ ++ if (cos_cache_backwards_stricmp_and_clip(dnVals[valIndex]->bv_val, " override")) { ++ /* matched */ ++ override_qualifier_hit = 1; ++ qualifier_hit = 1; ++ } ++ ++ if (cos_cache_backwards_stricmp_and_clip(dnVals[valIndex]->bv_val, " default")) { ++ default_qualifier_hit = 1; ++ qualifier_hit = 1; ++ } ++ ++ if (cos_cache_backwards_stricmp_and_clip(dnVals[valIndex]->bv_val, " operational-default")) { ++ operational_default_qualifier_hit = 1; ++ qualifier_hit = 1; ++ } ++ } while (qualifier_hit == 1); ++ ++ /* ++ * At this point, dnVals[valIndex]->bv_val ++ * is the value of cosAttribute, stripped of ++ * any qualifiers, so add this pure attribute type to ++ * the appropriate lists. ++ */ ++ ++ if (op_qualifier_hit) { ++ cos_cache_add_attrval(&pCosOperational, ++ dnVals[valIndex]->bv_val); ++ } ++ if (merge_schemes_qualifier_hit) { ++ cos_cache_add_attrval(&pCosMerge, dnVals[valIndex]->bv_val); ++ } ++ if (override_qualifier_hit) { ++ cos_cache_add_attrval(&pCosOverrides, ++ dnVals[valIndex]->bv_val); ++ } ++ if (default_qualifier_hit) { ++ /* attr is added below in pSneakyVal, in any case */ ++ } ++ ++ if (operational_default_qualifier_hit) { ++ cos_cache_add_attrval(&pCosOpDefault, ++ dnVals[valIndex]->bv_val); ++ } ++ ++ /* ++ * Each SP_handle is associated to one and only one vattr. ++ * We could consider making this a single function rather ++ * than the double-call. ++ */ ++ ++ vattr_sp_handle *vattr_handle = NULL; ++ ++ if (slapi_vattrspi_register((vattr_sp_handle **)&vattr_handle, ++ cos_cache_vattr_get, ++ cos_cache_vattr_compare, ++ cos_cache_vattr_types) != 0) { ++ slapi_log_err(SLAPI_LOG_ERR, COS_PLUGIN_SUBSYSTEM, "cos_cache_init - Cannot register as service provider for %s\n", dnVals[valIndex]->bv_val); ++ } else { ++ slapi_vattrspi_regattr((vattr_sp_handle *)vattr_handle, dnVals[valIndex]->bv_val, NULL, NULL); ++ } ++ ++ } /* if(attrType is cosAttribute) */ ++ ++ /* ++ * Add the attributetype to the appropriate ++ * list. ++ */ ++ cos_cache_add_attrval(pSneakyVal, dnVals[valIndex]->bv_val); ++ ++ } /* for (valIndex = 0; dnVals[valIndex]; valIndex++) */ ++ ++ ber_bvecfree(dnVals); ++ dnVals = NULL; ++ } while (!slapi_entry_next_attr(e, dnAttr, &dnAttr)); ++ ++ if (pCosAttribute && (!pCosTargetTree || !pCosTemplateDn)) { ++ /* get the parent of the definition */ ++ char *orig = slapi_dn_parent(pDn->val); ++ char *parent = NULL; ++ if (orig) { ++ parent = slapi_create_dn_string("%s", orig); ++ if (!parent) { ++ parent = orig; ++ slapi_log_err(SLAPI_LOG_ERR, COS_PLUGIN_SUBSYSTEM, ++ "cos_dn_defs_cb - " ++ "Failed to normalize parent dn %s. " ++ "Adding the pre normalized dn.\n", ++ parent); ++ } ++ if (!pCosTargetTree) { ++ cos_cache_add_attrval(&pCosTargetTree, parent); ++ } ++ if (!pCosTemplateDn) { ++ cos_cache_add_attrval(&pCosTemplateDn, parent); ++ } ++ if (parent != orig) { ++ slapi_ch_free_string(&parent); ++ } ++ slapi_ch_free_string(&orig); ++ } else { ++ slapi_log_err(SLAPI_LOG_ERR, COS_PLUGIN_SUBSYSTEM, ++ "cos_dn_defs_cb - " ++ "Failed to get parent dn of cos definition %s.\n", ++ pDn->val); ++ if (!pCosTemplateDn) { ++ if (!pCosTargetTree) { ++ slapi_log_err(SLAPI_LOG_ERR, COS_PLUGIN_SUBSYSTEM, "cos_dn_defs_cb - cosTargetTree and cosTemplateDn are not set.\n"); ++ } else { ++ slapi_log_err(SLAPI_LOG_ERR, COS_PLUGIN_SUBSYSTEM, "cos_dn_defs_cb - cosTemplateDn is not set.\n"); ++ } ++ } else if (!pCosTargetTree) { ++ slapi_log_err(SLAPI_LOG_ERR, COS_PLUGIN_SUBSYSTEM, "cos_dn_defs_cb - cosTargetTree is not set.\n"); ++ } ++ } ++ } + +- if(cos_cache_backwards_stricmp_and_clip(dnVals[valIndex]->bv_val, " operational")) +- { +- /* matched */ +- op_qualifier_hit = 1; +- qualifier_hit = 1; +- } +- +- if(cos_cache_backwards_stricmp_and_clip(dnVals[valIndex]->bv_val, " merge-schemes")) +- { +- /* matched */ +- merge_schemes_qualifier_hit = 1; +- qualifier_hit = 1; +- } ++ /* ++ determine the type of class of service scheme ++ */ + +- if(cos_cache_backwards_stricmp_and_clip(dnVals[valIndex]->bv_val, " override")) +- { +- /* matched */ +- override_qualifier_hit = 1; +- qualifier_hit = 1; +- } +- +- if(cos_cache_backwards_stricmp_and_clip(dnVals[valIndex]->bv_val, " default")) { +- default_qualifier_hit = 1; +- qualifier_hit = 1; +- } ++ if (pObjectclass) { ++ if (cos_cache_attrval_exists(pObjectclass, "cosDefinition")) { ++ cosType = COSTYPE_CLASSIC; ++ } else if (cos_cache_attrval_exists(pObjectclass, "cosClassicDefinition")) { ++ cosType = COSTYPE_CLASSIC; + +- if(cos_cache_backwards_stricmp_and_clip(dnVals[valIndex]->bv_val, " operational-default")) { +- operational_default_qualifier_hit = 1; +- qualifier_hit = 1; +- } +- } +- while(qualifier_hit == 1); ++ } else if (cos_cache_attrval_exists(pObjectclass, "cosPointerDefinition")) { ++ cosType = COSTYPE_POINTER; + +- /* +- * At this point, dnVals[valIndex]->bv_val +- * is the value of cosAttribute, stripped of +- * any qualifiers, so add this pure attribute type to +- * the appropriate lists. +- */ +- +- if ( op_qualifier_hit ) { +- cos_cache_add_attrval(&pCosOperational, +- dnVals[valIndex]->bv_val); +- } +- if ( merge_schemes_qualifier_hit ) { +- cos_cache_add_attrval(&pCosMerge, dnVals[valIndex]->bv_val); +- } +- if ( override_qualifier_hit ) { +- cos_cache_add_attrval(&pCosOverrides, +- dnVals[valIndex]->bv_val); +- } +- if ( default_qualifier_hit ) { +- /* attr is added below in pSneakyVal, in any case */ +- } ++ } else if (cos_cache_attrval_exists(pObjectclass, "cosIndirectDefinition")) { ++ cosType = COSTYPE_INDIRECT; + +- if ( operational_default_qualifier_hit ) { +- cos_cache_add_attrval(&pCosOpDefault, +- dnVals[valIndex]->bv_val); +- } ++ } else ++ cosType = COSTYPE_BADTYPE; ++ } + +- slapi_vattrspi_regattr((vattr_sp_handle *)vattr_handle, +- dnVals[valIndex]->bv_val, NULL, NULL); +- } /* if(attrType is cosAttribute) */ ++ /* ++ we should now have a full definition, ++ do some sanity checks because we don't ++ want bogus entries in the cache ++ then ship it ++ */ ++ ++ /* these must exist */ ++ if (pDn && pObjectclass && ++ ((cosType == COSTYPE_CLASSIC && ++ pCosTemplateDn && ++ pCosSpecifier && ++ pCosAttribute) || ++ (cosType == COSTYPE_POINTER && ++ pCosTemplateDn && ++ pCosAttribute) || ++ (cosType == COSTYPE_INDIRECT && ++ pCosSpecifier && ++ pCosAttribute))) { ++ int rc = 0; ++ /* ++ we'll leave the referential integrity stuff ++ up to the referint plug-in and assume all ++ is good - if it's not then we just have a ++ useless definition and we'll nag copiously later. ++ */ ++ char *pTmpDn = slapi_ch_strdup(pDn->val); /* because dn gets hosed on error */ ++ ++ if (!(rc = cos_cache_add_defn(info->pDefs, &pDn, cosType, ++ &pCosTargetTree, &pCosTemplateDn, ++ &pCosSpecifier, &pCosAttribute, ++ &pCosOverrides, &pCosOperational, ++ &pCosMerge, &pCosOpDefault))) { ++ info->ret = 0; /* we have succeeded to add the defn*/ ++ } else { ++ /* ++ * Failed but we will continue the search for other defs ++ * Don't reset info->ret....it keeps track of any success ++ */ ++ if (rc == COS_DEF_ERROR_NO_TEMPLATES) { ++ slapi_log_err(SLAPI_LOG_ERR, COS_PLUGIN_SUBSYSTEM, "cos_dn_defs_cb - Skipping CoS Definition %s" ++ "--no CoS Templates found, which should be added before the CoS Definition.\n", ++ pTmpDn); ++ } else { ++ slapi_log_err(SLAPI_LOG_ERR, COS_PLUGIN_SUBSYSTEM, "cos_dn_defs_cb - Skipping CoS Definition %s\n" ++ "--error(%d)\n", ++ pTmpDn, rc); ++ } ++ } + +- /* +- * Add the attributetype to the appropriate +- * list. +- */ +- cos_cache_add_attrval(pSneakyVal, dnVals[valIndex]->bv_val); +- +- }/* for (valIndex = 0; dnVals[valIndex]; valIndex++) */ +- +- ber_bvecfree( dnVals ); +- dnVals = NULL; +- } while(!slapi_entry_next_attr(e, dnAttr, &dnAttr)); +- +- if (pCosAttribute && (!pCosTargetTree || !pCosTemplateDn)) { +- /* get the parent of the definition */ +- char *orig = slapi_dn_parent(pDn->val); +- char *parent = NULL; +- if (orig) { +- parent = slapi_create_dn_string("%s", orig); +- if (!parent) { +- parent = orig; +- slapi_log_err(SLAPI_LOG_ERR, COS_PLUGIN_SUBSYSTEM, +- "cos_dn_defs_cb - " +- "Failed to normalize parent dn %s. " +- "Adding the pre normalized dn.\n", +- parent); +- } +- if (!pCosTargetTree) { +- cos_cache_add_attrval(&pCosTargetTree, parent); +- } +- if (!pCosTemplateDn) { +- cos_cache_add_attrval(&pCosTemplateDn, parent); +- } +- if (parent != orig) { +- slapi_ch_free_string(&parent); +- } +- slapi_ch_free_string(&orig); +- } else { +- slapi_log_err(SLAPI_LOG_ERR, COS_PLUGIN_SUBSYSTEM, +- "cos_dn_defs_cb - " +- "Failed to get parent dn of cos definition %s.\n", +- pDn->val); +- if (!pCosTemplateDn) { +- if (!pCosTargetTree) { +- slapi_log_err(SLAPI_LOG_ERR, COS_PLUGIN_SUBSYSTEM, "cos_dn_defs_cb - cosTargetTree and cosTemplateDn are not set.\n"); +- } else { +- slapi_log_err(SLAPI_LOG_ERR, COS_PLUGIN_SUBSYSTEM, "cos_dn_defs_cb - cosTemplateDn is not set.\n"); +- } +- } else if (!pCosTargetTree) { +- slapi_log_err(SLAPI_LOG_ERR, COS_PLUGIN_SUBSYSTEM, "cos_dn_defs_cb - cosTargetTree is not set.\n"); +- } +- } +- } +- +- /* +- determine the type of class of service scheme +- */ +- +- if(pObjectclass) +- { +- if(cos_cache_attrval_exists(pObjectclass, "cosDefinition")) +- { +- cosType = COSTYPE_CLASSIC; +- } +- else if(cos_cache_attrval_exists(pObjectclass, "cosClassicDefinition")) +- { +- cosType = COSTYPE_CLASSIC; +- +- } +- else if(cos_cache_attrval_exists(pObjectclass, "cosPointerDefinition")) +- { +- cosType = COSTYPE_POINTER; +- +- } +- else if(cos_cache_attrval_exists(pObjectclass, "cosIndirectDefinition")) +- { +- cosType = COSTYPE_INDIRECT; +- +- } +- else +- cosType = COSTYPE_BADTYPE; +- } +- +- /* +- we should now have a full definition, +- do some sanity checks because we don't +- want bogus entries in the cache +- then ship it +- */ +- +- /* these must exist */ +- if(pDn && pObjectclass && +- ( +- (cosType == COSTYPE_CLASSIC && +- pCosTemplateDn && +- pCosSpecifier && +- pCosAttribute ) +- || +- (cosType == COSTYPE_POINTER && +- pCosTemplateDn && +- pCosAttribute ) +- || +- (cosType == COSTYPE_INDIRECT && +- pCosSpecifier && +- pCosAttribute ) +- ) +- ) +- { +- int rc = 0; +- /* +- we'll leave the referential integrity stuff +- up to the referint plug-in and assume all +- is good - if it's not then we just have a +- useless definition and we'll nag copiously later. +- */ +- char *pTmpDn = slapi_ch_strdup(pDn->val); /* because dn gets hosed on error */ +- +- if(!(rc = cos_cache_add_defn(info->pDefs, &pDn, cosType, +- &pCosTargetTree, &pCosTemplateDn, +- &pCosSpecifier, &pCosAttribute, +- &pCosOverrides, &pCosOperational, +- &pCosMerge, &pCosOpDefault))) { +- info->ret = 0; /* we have succeeded to add the defn*/ +- } else { +- /* +- * Failed but we will continue the search for other defs +- * Don't reset info->ret....it keeps track of any success +- */ +- if ( rc == COS_DEF_ERROR_NO_TEMPLATES) { +- slapi_log_err(SLAPI_LOG_ERR, COS_PLUGIN_SUBSYSTEM, "cos_dn_defs_cb - Skipping CoS Definition %s" +- "--no CoS Templates found, which should be added before the CoS Definition.\n", +- pTmpDn); +- } else { +- slapi_log_err(SLAPI_LOG_ERR, COS_PLUGIN_SUBSYSTEM, "cos_dn_defs_cb - Skipping CoS Definition %s\n" +- "--error(%d)\n", +- pTmpDn, rc); +- } +- } +- +- slapi_ch_free_string(&pTmpDn); +- } +- else +- { +- /* +- this definition is brain dead - bail +- if we have a dn use it to report, if not then *really* bad +- things are going on +- */ +- if(pDn) +- { +- slapi_log_err(SLAPI_LOG_ERR, COS_PLUGIN_SUBSYSTEM, "cos_dn_defs_cb - " +- "Incomplete cos definition detected in %s, discarding from cache.\n",pDn->val); +- } +- else +- slapi_log_err(SLAPI_LOG_ERR, COS_PLUGIN_SUBSYSTEM, "cos_dn_defs_cb - " +- "Incomplete cos definition detected, no DN to report, discarding from cache.\n"); +- +- if(pCosTargetTree) +- cos_cache_del_attrval_list(&pCosTargetTree); +- if(pCosTemplateDn) +- cos_cache_del_attrval_list(&pCosTemplateDn); +- if(pCosSpecifier) +- cos_cache_del_attrval_list(&pCosSpecifier); +- if(pCosAttribute) +- cos_cache_del_attrval_list(&pCosAttribute); +- if(pDn) +- cos_cache_del_attrval_list(&pDn); +- } ++ slapi_ch_free_string(&pTmpDn); ++ } else { ++ /* ++ this definition is brain dead - bail ++ if we have a dn use it to report, if not then *really* bad ++ things are going on ++ */ ++ if (pDn) { ++ slapi_log_err(SLAPI_LOG_ERR, COS_PLUGIN_SUBSYSTEM, "cos_dn_defs_cb - " ++ "Incomplete cos definition detected in %s, discarding from cache.\n", ++ pDn->val); ++ } else ++ slapi_log_err(SLAPI_LOG_ERR, COS_PLUGIN_SUBSYSTEM, "cos_dn_defs_cb - " ++ "Incomplete cos definition detected, no DN to report, discarding from cache.\n"); ++ ++ if (pCosTargetTree) ++ cos_cache_del_attrval_list(&pCosTargetTree); ++ if (pCosTemplateDn) ++ cos_cache_del_attrval_list(&pCosTemplateDn); ++ if (pCosSpecifier) ++ cos_cache_del_attrval_list(&pCosSpecifier); ++ if (pCosAttribute) ++ cos_cache_del_attrval_list(&pCosAttribute); ++ if (pDn) ++ cos_cache_del_attrval_list(&pDn); ++ } + bail: + /* we don't keep the objectclasses, so lets free them */ + if(pObjectclass) { +diff --git a/ldap/servers/plugins/roles/roles_cache.c b/ldap/servers/plugins/roles/roles_cache.c +index 3697eaa97..3e1724963 100644 +--- a/ldap/servers/plugins/roles/roles_cache.c ++++ b/ldap/servers/plugins/roles/roles_cache.c +@@ -48,9 +48,6 @@ static char *allUserAttributes[] = { + /* views scoping */ + static void **views_api; + +-/* Service provider handler */ +-static vattr_sp_handle *vattr_handle = NULL; +- + /* List of nested roles */ + typedef struct _role_object_nested { + Slapi_DN *dn; /* value of attribute nsroledn in a nested role definition */ +@@ -224,13 +221,16 @@ int roles_cache_init() + + /* Register a callback on backends creation|modification|deletion, + so that we update the corresponding cache */ +- slapi_register_backend_state_change(NULL, roles_cache_trigger_update_suffix); +- +- if ( slapi_vattrspi_register((vattr_sp_handle **)&vattr_handle, +- roles_sp_get_value, +- roles_sp_compare_value, +- roles_sp_list_types) ) +- { ++ slapi_register_backend_state_change(NULL, roles_cache_trigger_update_suffix); ++ ++ /* Service provider handler - only used once! and freed by vattr! */ ++ vattr_sp_handle *vattr_handle = NULL; ++ ++ ++ if (slapi_vattrspi_register((vattr_sp_handle **)&vattr_handle, ++ roles_sp_get_value, ++ roles_sp_compare_value, ++ roles_sp_list_types)) { + slapi_log_err(SLAPI_LOG_ERR, ROLES_PLUGIN_SUBSYSTEM, + "roles_cache_init - slapi_vattrspi_register failed\n"); + +@@ -648,22 +648,20 @@ void roles_cache_stop() + + slapi_log_err(SLAPI_LOG_PLUGIN, ROLES_PLUGIN_SUBSYSTEM, "--> roles_cache_stop\n"); + +- /* Go through all the roles list and trigger the associated structure */ +- slapi_rwlock_wrlock(global_lock); +- current_role = roles_list; +- while ( current_role ) +- { +- slapi_lock_mutex(current_role->change_lock); +- current_role->keeprunning = 0; +- next_role = current_role->next; +- slapi_notify_condvar(current_role->something_changed, 1 ); +- slapi_unlock_mutex(current_role->change_lock); +- +- current_role = next_role; +- } +- slapi_rwlock_unlock(global_lock); +- slapi_ch_free((void **)&vattr_handle); +- roles_list = NULL; ++ /* Go through all the roles list and trigger the associated structure */ ++ slapi_rwlock_wrlock(global_lock); ++ current_role = roles_list; ++ while (current_role) { ++ slapi_lock_mutex(current_role->change_lock); ++ current_role->keeprunning = 0; ++ next_role = current_role->next; ++ slapi_notify_condvar(current_role->something_changed, 1); ++ slapi_unlock_mutex(current_role->change_lock); ++ ++ current_role = next_role; ++ } ++ slapi_rwlock_unlock(global_lock); ++ roles_list = NULL; + + slapi_log_err(SLAPI_LOG_PLUGIN, ROLES_PLUGIN_SUBSYSTEM, "<-- roles_cache_stop\n"); + } +diff --git a/ldap/servers/slapd/vattr.c b/ldap/servers/slapd/vattr.c +index ef4d7f279..84e01cd62 100644 +--- a/ldap/servers/slapd/vattr.c ++++ b/ldap/servers/slapd/vattr.c +@@ -1843,8 +1843,15 @@ static int vattr_map_create(void) + return 0; + } + +-void vattr_map_entry_free(vattr_map_entry *vae) { +- slapi_ch_free((void **)&(vae->sp_list)); ++void ++vattr_map_entry_free(vattr_map_entry *vae) ++{ ++ vattr_sp_handle *list_entry = vae->sp_list; ++ while (list_entry != NULL) { ++ vattr_sp_handle *next_entry = list_entry->next; ++ slapi_ch_free((void **)&list_entry); ++ list_entry = next_entry; ++ } + slapi_ch_free_string(&(vae->type_name)); + slapi_ch_free((void **)&vae); + } +@@ -2134,16 +2141,9 @@ int slapi_vattr_schema_check_type(Slapi_Entry *e, char *type) + + vattr_map_entry *vattr_map_entry_new(char *type_name, vattr_sp_handle *sph, void* hint) + { +- vattr_map_entry *result = NULL; +- vattr_sp_handle *sp_copy = NULL; +- +- sp_copy = (vattr_sp_handle*)slapi_ch_calloc(1, sizeof (vattr_sp_handle)); +- sp_copy->sp = sph->sp; +- sp_copy->hint = hint; +- +- result = (vattr_map_entry*)slapi_ch_calloc(1, sizeof (vattr_map_entry)); +- result->type_name = slapi_ch_strdup(type_name); +- result->sp_list = sp_copy; ++ vattr_map_entry *result = (vattr_map_entry *)slapi_ch_calloc(1, sizeof(vattr_map_entry)); ++ result->type_name = slapi_ch_strdup(type_name); ++ result->sp_list = sph; + + /* go get schema */ + result->objectclasses = vattr_map_entry_build_schema(type_name); +@@ -2259,6 +2259,16 @@ we'd need to hold a lock on the read path, which we don't want to do. + So any SP which relinquishes its need to handle a type needs to continue + to handle the calls on it, but return nothing */ + /* DBDB need to sort out memory ownership here, it's not quite right */ ++/* ++ * This function was inconsistent. We would allocated and "kind of", ++ * copy the sp_handle here for the vattr_map_entry_new path. But we ++ * would "take ownership" for the existing entry and the list addition ++ * path. Instead now, EVERY sp_handle we take, we take ownership of ++ * and the CALLER must allocate a new one each time. ++ * ++ * Better idea, is that regattr should just take the fn pointers ++ * and callers never *see* the sp_handle structure at all. ++ */ + + int vattr_map_sp_insert(char *type_to_add, vattr_sp_handle *sp, void *hint) + { +-- +2.13.6 + diff --git a/SOURCES/0081-Ticket-49441-Import-crashes-with-large-indexed-binar.patch b/SOURCES/0081-Ticket-49441-Import-crashes-with-large-indexed-binar.patch new file mode 100644 index 0000000..3e65c49 --- /dev/null +++ b/SOURCES/0081-Ticket-49441-Import-crashes-with-large-indexed-binar.patch @@ -0,0 +1,96 @@ +From df5000efced2d00aa0fc6546fcf6fc7b02e27256 Mon Sep 17 00:00:00 2001 +From: Mark Reynolds +Date: Mon, 6 Nov 2017 22:30:55 -0500 +Subject: [PATCH] Ticket 49441 - Import crashes with large indexed binary + attributes + +Bug Description: Importing an ldif file that contains entries with large + binary attributes that are indexed crashes the server. + The crash occurs when "encoding" the binary value to a + string for debug logging, where we "underflow" the buffer + space index which then allows the string buffer to overflow. + +Fix Description: While filling the string buffer with the encoded binary + value we need to make sure if the buffer space is greater + than zero before decrementing it. + + Also check if trace logging is being used before we actually + call the logging function which calls the "encoded" function + first. This way we avoid this costly "encoding" on every + index call we make. + +https://pagure.io/389-ds-base/issue/49441 + +Reviewed by: firstyear(Thanks!) +--- + ldap/servers/slapd/back-ldbm/index.c | 21 ++++++++++----------- + 1 file changed, 10 insertions(+), 11 deletions(-) + +diff --git a/ldap/servers/slapd/back-ldbm/index.c b/ldap/servers/slapd/back-ldbm/index.c +index d4de28ca3..d62052a22 100644 +--- a/ldap/servers/slapd/back-ldbm/index.c ++++ b/ldap/servers/slapd/back-ldbm/index.c +@@ -808,7 +808,10 @@ encode (const struct berval* data, char buf[BUFSIZ]) + bufSpace -= (s - first); + } + do { +- *bufNext++ = '\\'; --bufSpace; ++ if (bufSpace) { ++ *bufNext++ = '\\'; ++ --bufSpace; ++ } + if (bufSpace < 2) { + memcpy (bufNext, "..", 2); + bufNext += 2; +@@ -903,8 +906,10 @@ index_read_ext_allids( + slapi_log_err(SLAPI_LOG_ERR, "index_read_ext_allids", "NULL prefix\n"); + return NULL; + } +- slapi_log_err(SLAPI_LOG_TRACE, "index_read_ext_allids", "=> ( \"%s\" %s \"%s\" )\n", +- type, prefix, encode (val, buf)); ++ if (slapi_is_loglevel_set(LDAP_DEBUG_TRACE)) { ++ slapi_log_err(SLAPI_LOG_TRACE, "index_read_ext_allids", "=> ( \"%s\" %s \"%s\" )\n", ++ type, prefix, encode (val, buf)); ++ } + + basetype = typebuf; + if ( (basetmp = slapi_attr_basetype( type, typebuf, sizeof(typebuf) )) +@@ -1737,16 +1742,13 @@ addordel_values( + */ + key.flags = DB_DBT_USERMEM; + key.ulen = tmpbuflen; +-#ifdef LDAP_ERROR_LOGGING +- /* XXX if ( slapd_ldap_debug & LDAP_DEBUG_TRACE ) XXX */ +- { ++ if (slapi_is_loglevel_set(LDAP_DEBUG_TRACE)) { + char encbuf[BUFSIZ]; + + slapi_log_err(SLAPI_LOG_TRACE, "addordel_values", "%s_value(\"%s\")\n", + (flags & BE_INDEX_ADD) ? "add" : "del", + encoded (&key, encbuf)); + } +-#endif + + if (NULL != txn) { + db_txn = txn->back_txn_txn; +@@ -1907,16 +1909,13 @@ addordel_values_sv( + */ + key.flags = DB_DBT_USERMEM; + key.ulen = tmpbuflen; +-#ifdef LDAP_ERROR_LOGGING +- /* XXX if ( slapd_ldap_debug & LDAP_DEBUG_TRACE ) XXX */ +- { ++ if (slapi_is_loglevel_set(LDAP_DEBUG_TRACE)) { + char encbuf[BUFSIZ]; + + slapi_log_err(SLAPI_LOG_TRACE, "addordel_values_sv", "%s_value(\"%s\")\n", + (flags & BE_INDEX_ADD) ? "add" : "del", + encoded (&key, encbuf)); + } +-#endif + + if (NULL != txn) { + db_txn = txn->back_txn_txn; +-- +2.13.6 + diff --git a/SOURCES/0082-Ticket-49431-replicated-MODRDN-fails-breaking-replic.patch b/SOURCES/0082-Ticket-49431-replicated-MODRDN-fails-breaking-replic.patch new file mode 100644 index 0000000..502dda1 --- /dev/null +++ b/SOURCES/0082-Ticket-49431-replicated-MODRDN-fails-breaking-replic.patch @@ -0,0 +1,38 @@ +From 557f4d4ed5e37f09691d383dd8189b642ade8f2b Mon Sep 17 00:00:00 2001 +From: Ludwig Krispenz +Date: Sun, 29 Oct 2017 16:32:11 +0100 +Subject: [PATCH] Ticket 49431 - replicated MODRDN fails breaking replication + +Bug: in urp modrdn_operation not a full dn was passed to generate the conflict rdn passed + and so the sufix test failed + +Fix: provide full dn of new entry + +Reviewed by: Mark, Thanks +--- + ldap/servers/plugins/replication/urp.c | 9 ++++++++- + 1 file changed, 8 insertions(+), 1 deletion(-) + +diff --git a/ldap/servers/plugins/replication/urp.c b/ldap/servers/plugins/replication/urp.c +index 64810e9d4..21319d9f7 100644 +--- a/ldap/servers/plugins/replication/urp.c ++++ b/ldap/servers/plugins/replication/urp.c +@@ -433,7 +433,14 @@ urp_modrdn_operation( Slapi_PBlock *pb ) + /* The target entry is a loser */ + + char *newrdn_with_uniqueid; +- newrdn_with_uniqueid= get_rdn_plus_uniqueid (sessionid, newrdn, op_uniqueid); ++ char *newdn = NULL; ++ if (new_parent_entry) { ++ newdn = slapi_ch_smprintf("%s,%s", newrdn, slapi_entry_get_dn(new_parent_entry)); ++ } else { ++ newdn = slapi_ch_smprintf("%s,%s", newrdn, slapi_entry_get_dn(parent_entry)); ++ } ++ newrdn_with_uniqueid= get_rdn_plus_uniqueid (sessionid, newdn, op_uniqueid); ++ slapi_ch_free_string(&newdn); + if(newrdn_with_uniqueid==NULL) + { + op_result= LDAP_OPERATIONS_ERROR; +-- +2.13.6 + diff --git a/SPECS/389-ds-base.spec b/SPECS/389-ds-base.spec index 2547084..ed028d0 100644 --- a/SPECS/389-ds-base.spec +++ b/SPECS/389-ds-base.spec @@ -30,7 +30,7 @@ Summary: 389 Directory Server (base) Name: 389-ds-base Version: 1.3.6.1 -Release: %{?relprefix}21%{?prerel}%{?dist} +Release: %{?relprefix}24%{?prerel}%{?dist} License: GPLv3+ URL: https://www.port389.org/ Group: System Environment/Daemons @@ -206,6 +206,16 @@ Patch69: 0069-Ticket-49327-password-expired-control-not-sent-durin.patc Patch70: 0070-Ticket-49379-Allowed-sasl-mapping-requires-restart.patch Patch71: 0071-Fix-cherry-pick-error-from-sasl-mech-commit.patch Patch72: 0072-Ticket-49389-unable-to-retrieve-specific-cosAttribut.patch +Patch73: 0073-Ticket-49180-backport-1.3.6-errors-log-filled-with-a.patch +Patch74: 0074-Ticket-48894-harden-valueset_array_to_sorted_quick-v.patch +Patch75: 0075-Ticket-49401-improve-valueset-sorted-performance-on-.patch +Patch76: 0076-Ticket-49401-Fix-compiler-incompatible-pointer-types.patch +Patch77: 0077-Ticket-48235-Remove-memberOf-global-lock.patch +Patch78: 0078-Ticket-49402-Adding-a-database-entry-with-the-same-d.patch +Patch79: 0079-Ticket-49439-cleanallruv-is-not-logging-information.patch +Patch80: 0080-Ticket-49436-double-free-in-COS-in-some-conditions.patch +Patch81: 0081-Ticket-49441-Import-crashes-with-large-indexed-binar.patch +Patch82: 0082-Ticket-49431-replicated-MODRDN-fails-breaking-replic.patch %description 389 Directory Server is an LDAPv3 compliant server. The base package includes @@ -348,6 +358,17 @@ cp %{SOURCE2} README.devel %patch70 -p1 %patch71 -p1 %patch72 -p1 +%patch73 -p1 +%patch74 -p1 +%patch75 -p1 +%patch76 -p1 +%patch77 -p1 +%patch78 -p1 +%patch79 -p1 +%patch80 -p1 +%patch81 -p1 +%patch82 -p1 + %build OPENLDAP_FLAG="--with-openldap" @@ -579,6 +600,23 @@ fi %{_sysconfdir}/%{pkgname}/dirsrvtests %changelog +* Fri Nov 10 2017 Mark Reynolds - 1.3.6.1-24 +- Bump version to 1.3.6.1-24 +- Resolves: Bug 1508978 - replicated MODRDN fails breaking replication +- Resolves: Bug 1511940 - heap corruption during import +- Resolves: Bug 1510319 - [abrt] 389-ds-base: SLL_Next(): ns-slapd killed by SIGSEGV +- Resolves: Bug 1509347 - cleanallruv task is not logging any information + +* Fri Oct 27 2017 Mark Reynolds - 1.3.6.1-23 +- Bump version to 1.3.6.1-23 +- Resolves: Bug 1504536 - [memberOf Plugin] bulk deleting users causes deadlock when there are multiple backends +- Resolves: Bug 1503001 - Adding a database entry fails if the same database was deleted after an import +- Resolves: Bug 1506912 - Improve valueset sort performance during valueset purging + +* Mon Oct 9 2017 Mark Reynolds - 1.3.6.1-22 +- Bump version to 1.3.6.1-22 +- Resolves: Bug 1499668 - Errors log filled with attrlist_replace + * Thu Oct 5 2017 Mark Reynolds - 1.3.6.1-21 - Bump verions to 1.3.6.1-21 - Resolves: Bug 1498958 - unable to retrieve specific cosAttribute when subtree password policy is configured