ldap/servers/plugins/automember/automember.c | 19 ++++++-
ldap/servers/plugins/dna/dna.c | 39 +++++++++++++++
ldap/servers/plugins/linkedattrs/linked_attrs.c | 54 +++++++++++++++++++++
ldap/servers/plugins/mep/mep.c | 59 ++++++++++++++++++++++++
4 files changed, 168 insertions(+), 3 deletions(-)
New commits:
commit 22107650f6c09c38420976c988cdb94c1cbc6764
Author: Nathan Kinder <nkinder(a)redhat.com>
Date: Thu Apr 28 10:40:11 2011 -0700
Bug 700557 - Linked attrs callbacks access free'd pointers after close
The linked attributes plug-in callbacks can still try to access free'd
resources after it's close callback has been called. This can cause
ns-slapd to crash.
This same issue actually applies to the following plug-ins:
- DNA
- Linked Attributes
- Managed Entries
- Auto Membership
We should address this issue in all of these plug-ins. We need to
unset the started flag in the close() callbacks, which needs to be
done with the write lock on the config held. To prevent problems
with waiting readers, we need to make all readers check if the started
flag is set after they obtain a reader lock. This will deal with the
case where the plug-in was stopped while a thread was waiting on a
reader lock. We also need to be sure to NOT free the lock in the
close() function so that these waiting readers don't crash the
server.
diff --git a/ldap/servers/plugins/automember/automember.c b/ldap/servers/plugins/automember/automember.c
index a015eae..670ae64 100644
--- a/ldap/servers/plugins/automember/automember.c
+++ b/ldap/servers/plugins/automember/automember.c
@@ -349,15 +349,22 @@ automember_close(Slapi_PBlock * pb)
goto done;
}
+ automember_config_write_lock();
+ g_plugin_started = 0;
automember_delete_config();
+ automember_config_unlock();
slapi_ch_free((void **)&g_automember_config);
slapi_sdn_free(&_PluginDN);
slapi_sdn_free(&_ConfigAreaDN);
- if (g_automember_config_lock) {
- PR_DestroyRWLock(g_automember_config_lock);
- }
+ /* We explicitly don't destroy the config lock here. If we did,
+ * there is the slight possibility that another thread that just
+ * passed the g_plugin_started check is about to try to obtain
+ * a reader lock. We leave the lock around so these threads
+ * don't crash the process. If we always check the started
+ * flag again after obtaining a reader lock, no free'd resources
+ * will be used. */
done:
slapi_log_error(SLAPI_LOG_TRACE, AUTOMEMBER_PLUGIN_SUBSYSTEM,
@@ -1737,6 +1744,12 @@ automember_add_post_op(Slapi_PBlock *pb)
* to the entry being added. */
automember_config_read_lock();
+ /* Bail out if the plug-in close function was just called. */
+ if (!g_plugin_started) {
+ automember_config_unlock();
+ return 0;
+ }
+
if (!PR_CLIST_IS_EMPTY(g_automember_config)) {
list = PR_LIST_HEAD(g_automember_config);
while (list != g_automember_config) {
diff --git a/ldap/servers/plugins/dna/dna.c b/ldap/servers/plugins/dna/dna.c
index fc520c9..a245a8d 100644
--- a/ldap/servers/plugins/dna/dna.c
+++ b/ldap/servers/plugins/dna/dna.c
@@ -568,7 +568,14 @@ dna_close(Slapi_PBlock * pb)
slapi_log_error(SLAPI_LOG_TRACE, DNA_PLUGIN_SUBSYSTEM,
"--> dna_close\n");
+ if (!g_plugin_started) {
+ goto done;
+ }
+
+ dna_write_lock();
+ g_plugin_started = 0;
dna_delete_config();
+ dna_unlock();
slapi_ch_free((void **)&dna_global_config);
@@ -576,6 +583,15 @@ dna_close(Slapi_PBlock * pb)
slapi_ch_free_string(&portnum);
slapi_ch_free_string(&secureportnum);
+ /* We explicitly don't destroy the config lock here. If we did,
+ * there is the slight possibility that another thread that just
+ * passed the g_plugin_started check is about to try to obtain
+ * a reader lock. We leave the lock around so these threads
+ * don't crash the process. If we always check the started
+ * flag again after obtaining a reader lock, no free'd resources
+ * will be used. */
+
+done:
slapi_log_error(SLAPI_LOG_TRACE, DNA_PLUGIN_SUBSYSTEM,
"<-- dna_close\n");
@@ -1207,6 +1223,11 @@ dna_update_config_event(time_t event_time, void *arg)
/* Get read lock to prevent config changes */
dna_read_lock();
+ /* Bail out if the plug-in close function was just called. */
+ if (!g_plugin_started) {
+ goto bail;
+ }
+
/* Loop through all config entries and update the shared
* config entries. */
if (!PR_CLIST_IS_EMPTY(dna_global_config)) {
@@ -2836,6 +2857,12 @@ static int dna_pre_op(Slapi_PBlock * pb, int modtype)
dna_read_lock();
+ /* Bail out if the plug-in close function was just called. */
+ if (!g_plugin_started) {
+ dna_unlock();
+ goto bail;
+ }
+
if (!PR_CLIST_IS_EMPTY(dna_global_config)) {
list = PR_LIST_HEAD(dna_global_config);
@@ -3287,6 +3314,12 @@ dna_release_range(char *range_dn, PRUint64 *lower, PRUint64 *upper)
dna_read_lock();
+ /* Bail out if the plug-in close function was just called. */
+ if (!g_plugin_started) {
+ dna_unlock();
+ return 0;
+ }
+
/* Go through the config entries to see if we
* have a shared range configured that matches
* the range from the exop request. */
@@ -3467,6 +3500,11 @@ void dna_dump_config()
dna_read_lock();
+ /* Bail out if the plug-in close function was just called. */
+ if (!g_plugin_started) {
+ goto bail;
+ }
+
if (!PR_CLIST_IS_EMPTY(dna_global_config)) {
list = PR_LIST_HEAD(dna_global_config);
while (list != dna_global_config) {
@@ -3475,6 +3513,7 @@ void dna_dump_config()
}
}
+bail:
dna_unlock();
}
diff --git a/ldap/servers/plugins/linkedattrs/linked_attrs.c b/ldap/servers/plugins/linkedattrs/linked_attrs.c
index b69b2cd..1080436 100644
--- a/ldap/servers/plugins/linkedattrs/linked_attrs.c
+++ b/ldap/servers/plugins/linkedattrs/linked_attrs.c
@@ -352,11 +352,27 @@ linked_attrs_close(Slapi_PBlock * pb)
slapi_log_error(SLAPI_LOG_TRACE, LINK_PLUGIN_SUBSYSTEM,
"--> linked_attrs_close\n");
+ if (!g_plugin_started) {
+ goto done;
+ }
+
+ linked_attrs_write_lock();
+ g_plugin_started = 0;
linked_attrs_delete_config();
+ linked_attrs_unlock();
slapi_ch_free((void **)&g_link_config);
slapi_ch_free((void **)&g_managed_config_index);
+ /* We explicitly don't destroy the config lock here. If we did,
+ * there is the slight possibility that another thread that just
+ * passed the g_plugin_started check is about to try to obtain
+ * a reader lock. We leave the lock around so these threads
+ * don't crash the process. If we always check the started
+ * flag again after obtaining a reader lock, no free'd resources
+ * will be used. */
+
+done:
slapi_log_error(SLAPI_LOG_TRACE, LINK_PLUGIN_SUBSYSTEM,
"<-- linked_attrs_close\n");
@@ -1584,6 +1600,13 @@ linked_attrs_mod_post_op(Slapi_PBlock *pb)
/* See if there is an applicable link configured. */
linked_attrs_read_lock();
+
+ /* Bail out if the plug-in close function was just called. */
+ if (!g_plugin_started) {
+ linked_attrs_unlock();
+ return 0;
+ }
+
linked_attrs_find_config(dn, type, &config);
/* If we have a matching config entry, we have
@@ -1675,6 +1698,13 @@ linked_attrs_add_post_op(Slapi_PBlock *pb)
/* See if there is an applicable link configured. */
linked_attrs_read_lock();
+
+ /* Bail out if the plug-in close function was just called. */
+ if (!g_plugin_started) {
+ linked_attrs_unlock();
+ return 0;
+ }
+
linked_attrs_find_config(dn, type, &config);
/* If config was found, add the backpointers to this entry. */
@@ -1747,6 +1777,13 @@ linked_attrs_del_post_op(Slapi_PBlock *pb)
/* See if there is an applicable link configured. */
linked_attrs_read_lock();
+
+ /* Bail out if the plug-in close function was just called. */
+ if (!g_plugin_started) {
+ linked_attrs_unlock();
+ return 0;
+ }
+
linked_attrs_find_config(dn, type, &config);
/* If config was found, delete the backpointers to this entry. */
@@ -1866,6 +1903,13 @@ linked_attrs_modrdn_post_op(Slapi_PBlock *pb)
/* See if there is an applicable link configured. */
linked_attrs_read_lock();
+
+ /* Bail out if the plug-in close function was just called. */
+ if (!g_plugin_started) {
+ linked_attrs_unlock();
+ return 0;
+ }
+
linked_attrs_find_config(old_dn, type, &config);
/* If config was found for the old dn, delete the backpointers
@@ -1969,6 +2013,10 @@ linked_attrs_dump_config()
linked_attrs_read_lock();
+ /* Bail out if the plug-in close function was just called. */
+ if (!g_plugin_started)
+ goto bail;
+
if (!PR_CLIST_IS_EMPTY(g_link_config)) {
list = PR_LIST_HEAD(g_link_config);
while (list != g_link_config) {
@@ -1977,6 +2025,7 @@ linked_attrs_dump_config()
}
}
+bail:
linked_attrs_unlock();
}
@@ -1987,6 +2036,10 @@ linked_attrs_dump_config_index()
linked_attrs_read_lock();
+ /* Bail out if the plug-in close function was just called. */
+ if (!g_plugin_started)
+ goto bail;
+
if (!PR_CLIST_IS_EMPTY(g_managed_config_index)) {
list = PR_LIST_HEAD(g_managed_config_index);
while (list != g_managed_config_index) {
@@ -1995,6 +2048,7 @@ linked_attrs_dump_config_index()
}
}
+bail:
linked_attrs_unlock();
}
diff --git a/ldap/servers/plugins/mep/mep.c b/ldap/servers/plugins/mep/mep.c
index cc4be8e..ee7c7c0 100644
--- a/ldap/servers/plugins/mep/mep.c
+++ b/ldap/servers/plugins/mep/mep.c
@@ -349,10 +349,26 @@ mep_close(Slapi_PBlock * pb)
slapi_log_error(SLAPI_LOG_TRACE, MEP_PLUGIN_SUBSYSTEM,
"--> mep_close\n");
+ if (!g_plugin_started) {
+ goto done;
+ }
+
+ mep_config_write_lock();
+ g_plugin_started = 0;
mep_delete_config();
+ mep_config_unlock();
slapi_ch_free((void **)&g_mep_config);
+ /* We explicitly don't destroy the config lock here. If we did,
+ * there is the slight possibility that another thread that just
+ * passed the g_plugin_started check is about to try to obtain
+ * a reader lock. We leave the lock around so these threads
+ * don't crash the process. If we always check the started
+ * flag again after obtaining a reader lock, no free'd resources
+ * will be used. */
+
+done:
slapi_log_error(SLAPI_LOG_TRACE, MEP_PLUGIN_SUBSYSTEM,
"<-- mep_close\n");
@@ -1739,6 +1755,13 @@ mep_pre_op(Slapi_PBlock * pb, int modop)
} else {
/* Check if an active template entry is being updated. If so, validate it. */
mep_config_read_lock();
+
+ /* Bail out if the plug-in close function was just called. */
+ if (!g_plugin_started) {
+ mep_config_unlock();
+ goto bail;
+ }
+
mep_find_config_by_template_dn(dn, &config);
if (config) {
Slapi_Entry *test_entry = NULL;
@@ -1863,6 +1886,13 @@ mep_pre_op(Slapi_PBlock * pb, int modop)
if (origin_e) {
/* Fetch the config. */
mep_config_read_lock();
+
+ /* Bail out if the plug-in close function was just called. */
+ if (!g_plugin_started) {
+ mep_config_unlock();
+ goto bail;
+ }
+
mep_find_config(origin_e, &config);
if (config) {
@@ -2019,6 +2049,13 @@ mep_mod_post_op(Slapi_PBlock *pb)
managed_dn = slapi_entry_attr_get_charptr(e, MEP_MANAGED_ENTRY_TYPE);
if (managed_dn) {
mep_config_read_lock();
+
+ /* Bail out if the plug-in close function was just called. */
+ if (!g_plugin_started) {
+ mep_config_unlock();
+ goto bail;
+ }
+
mep_find_config(e, &config);
if (config) {
smods = mep_get_mapped_mods(config, e, &mapped_dn);
@@ -2123,6 +2160,13 @@ mep_add_post_op(Slapi_PBlock *pb)
/* Check if a config entry applies
* to the entry being added. */
mep_config_read_lock();
+
+ /* Bail out if the plug-in close function was just called. */
+ if (!g_plugin_started) {
+ mep_config_unlock();
+ return 0;
+ }
+
mep_find_config(e, &config);
if (config) {
mep_add_managed_entry(config, e);
@@ -2273,6 +2317,14 @@ mep_modrdn_post_op(Slapi_PBlock *pb)
Slapi_Mods *smods = NULL;
mep_config_read_lock();
+
+ /* Bail out if the plug-in close function was just called. */
+ if (!g_plugin_started) {
+ mep_config_unlock();
+ slapi_pblock_destroy(mep_pb);
+ return 0;
+ }
+
mep_find_config(post_e, &config);
if (!config) {
LDAPMod mod2;
@@ -2410,6 +2462,13 @@ mep_modrdn_post_op(Slapi_PBlock *pb)
* If so, treat like an add and create the new managed
* entry and links. */
mep_config_read_lock();
+
+ /* Bail out if the plug-in close function was just called. */
+ if (!g_plugin_started) {
+ mep_config_unlock();
+ return 0;
+ }
+
mep_find_config(post_e, &config);
if (config) {
mep_add_managed_entry(config, post_e);
