From 5b2efd34c07c65e24f4129430064f7299803dbf8 Mon Sep 17 00:00:00 2001
From: Noriko Hosoi <nhosoi@redhat.com>
Date: Fri, 2 Oct 2015 11:38:01 -0700
Subject: [PATCH 68/68] Ticket #48298 - ns-slapd crash during
ipa-replica-manage del
Bug Description: The cause of the problem is rather not a race condition but
accessing an already freed agreement in a plug-in:
> The crashed thread is deleting an agreement object, which calls mep_pre_op.
> It eventually calls op_shared_search with the deleted agreement object with
> base scope and filter "(|(objectclass=*)(objectclass=ldapsubentry))"
> Since it is a DSE entry it goes to dse_search, in which it calls agmt_get_
> replarea and crashes in slapi_sdn_copy by NULL dereference in from SDN...
Fix Description: This patch adds the check to agmt_get_replarea, in which if
the agreement is not in the agreement list, it returnes NULL repl area. When
the NULL repl area is returned the callers back off with an error.
https://fedorahosted.org/389/ticket/48298
Reviewed by rmeggins@redhat.com (Thanks, Rich!)
(cherry picked from commit 3cbdfa613ed8668337213fe9c3c15cf54ce798aa)
(cherry picked from commit f09eb8c0f8ee315b2a20d6460c975a546207411e)
---
ldap/servers/plugins/replication/repl5.h | 1 +
ldap/servers/plugins/replication/repl5_agmt.c | 17 +++++++++--
ldap/servers/plugins/replication/repl5_agmtlist.c | 34 ++++++++++++++++++----
.../plugins/replication/repl5_inc_protocol.c | 10 ++++++-
.../plugins/replication/repl5_replica_config.c | 6 +++-
.../plugins/replication/repl5_tot_protocol.c | 12 ++++++--
.../plugins/replication/repl_session_plugin.c | 22 +++++++++++---
.../plugins/replication/windows_protocol_util.c | 12 ++++++++
8 files changed, 98 insertions(+), 16 deletions(-)
diff --git a/ldap/servers/plugins/replication/repl5.h b/ldap/servers/plugins/replication/repl5.h
index 17282bb..df92ca0 100644
--- a/ldap/servers/plugins/replication/repl5.h
+++ b/ldap/servers/plugins/replication/repl5.h
@@ -390,6 +390,7 @@ void agmtlist_shutdown();
void agmtlist_notify_all(Slapi_PBlock *pb);
Object* agmtlist_get_first_agreement_for_replica (Replica *r);
Object* agmtlist_get_next_agreement_for_replica (Replica *r, Object *prev);
+int agmtlist_agmt_exists(const Repl_Agmt *ra);
/* In repl5_backoff.c */
typedef struct backoff_timer Backoff_Timer;
diff --git a/ldap/servers/plugins/replication/repl5_agmt.c b/ldap/servers/plugins/replication/repl5_agmt.c
index f84eacb..76d26a1 100644
--- a/ldap/servers/plugins/replication/repl5_agmt.c
+++ b/ldap/servers/plugins/replication/repl5_agmt.c
@@ -696,6 +696,12 @@ agmt_start(Repl_Agmt *ra)
* index.
*/
repl_sdn = agmt_get_replarea(ra);
+ if (!repl_sdn) {
+ slapi_log_error(SLAPI_LOG_REPL, repl_plugin_name,
+ "agmt_start: failed to get repl area. Please check agreement.\n");
+ prot_free(&prot);
+ return -1;
+ }
pb = slapi_pblock_new();
attrs[0] = (char*)type_agmtMaxCSN;
@@ -770,7 +776,7 @@ agmt_start(Repl_Agmt *ra)
slapi_rdn_get_value_by_ref(slapi_rdn_get_rdn(ra->rdn)),
ra->hostname, ra->port);
if(strstr(maxcsns[i], buf) || strstr(maxcsns[i], unavail_buf)){
- /* Set the maxcsn */
+ /* Set the maxcsn */
slapi_ch_free_string(&ra->maxcsn);
ra->maxcsn = slapi_ch_strdup(maxcsns[i]);
ra->consumerRID = agmt_maxcsn_get_rid(maxcsns[i]);
@@ -976,8 +982,11 @@ agmt_get_bindmethod(const Repl_Agmt *ra)
Slapi_DN *
agmt_get_replarea(const Repl_Agmt *ra)
{
- Slapi_DN *return_value;
+ Slapi_DN *return_value = NULL;
PR_ASSERT(NULL != ra);
+ if (!agmtlist_agmt_exists(ra)) {
+ return return_value;
+ }
PR_Lock(ra->lock);
return_value = slapi_sdn_new();
slapi_sdn_copy(ra->replarea, return_value);
@@ -2690,6 +2699,9 @@ get_agmt_status(Slapi_PBlock *pb, Slapi_Entry* e, Slapi_Entry* entryAfter,
Object *repl_obj = NULL;
replarea_sdn = agmt_get_replarea(ra);
+ if (!replarea_sdn) {
+ goto bail;
+ }
repl_obj = replica_get_replica_from_dn(replarea_sdn);
slapi_sdn_free(&replarea_sdn);
if (repl_obj) {
@@ -2748,6 +2760,7 @@ get_agmt_status(Slapi_PBlock *pb, Slapi_Entry* e, Slapi_Entry* entryAfter,
slapi_entry_add_string(e, "nsds5replicaLastInitStatus", ra->last_init_status);
}
}
+bail:
return SLAPI_DSE_CALLBACK_OK;
}
diff --git a/ldap/servers/plugins/replication/repl5_agmtlist.c b/ldap/servers/plugins/replication/repl5_agmtlist.c
index 34650b4..f50862f 100644
--- a/ldap/servers/plugins/replication/repl5_agmtlist.c
+++ b/ldap/servers/plugins/replication/repl5_agmtlist.c
@@ -109,6 +109,24 @@ agmtlist_release_agmt(Repl_Agmt *ra)
}
}
+int
+agmtlist_agmt_exists(const Repl_Agmt *ra)
+{
+ Object *ro;
+ int exists = 0;
+
+ PR_ASSERT(NULL != agmt_set);
+ if (!ra) {
+ return exists;
+ }
+ ro = objset_find(agmt_set, agmt_ptr_cmp, (const void *)ra);
+ if (ro) {
+ exists = 1;
+ object_release(ro);
+ }
+ return exists;
+}
+
/*
* Note: when we add the new object, we have a reference to it. We hold
@@ -135,6 +153,9 @@ add_new_agreement(Slapi_Entry *e)
/* get the replica for this agreement */
replarea_sdn = agmt_get_replarea(ra);
+ if (!replarea_sdn) {
+ return 1;
+ }
repl_obj = replica_get_replica_from_dn(replarea_sdn);
slapi_sdn_free(&replarea_sdn);
if (repl_obj) {
@@ -841,13 +862,16 @@ Object* agmtlist_get_next_agreement_for_replica (Replica *r, Object *prev)
else
obj = objset_first_obj(agmt_set);
- while (obj)
- {
+ for ( ; obj; obj = objset_next_obj(agmt_set, obj)) {
agmt = (Repl_Agmt*)object_get_data (obj);
- PR_ASSERT (agmt);
+ if (!agmt) {
+ continue;
+ }
agmt_root = agmt_get_replarea(agmt);
- PR_ASSERT (agmt_root);
+ if (!agmt_root) {
+ continue;
+ }
if (slapi_sdn_compare (replica_root, agmt_root) == 0)
{
@@ -856,7 +880,7 @@ Object* agmtlist_get_next_agreement_for_replica (Replica *r, Object *prev)
}
slapi_sdn_free (&agmt_root);
- obj = objset_next_obj(agmt_set, obj);
+
}
return NULL;
diff --git a/ldap/servers/plugins/replication/repl5_inc_protocol.c b/ldap/servers/plugins/replication/repl5_inc_protocol.c
index 7680340..244bbb2 100644
--- a/ldap/servers/plugins/replication/repl5_inc_protocol.c
+++ b/ldap/servers/plugins/replication/repl5_inc_protocol.c
@@ -1906,6 +1906,7 @@ send_updates(Private_Repl_Protocol *prp, RUV *remote_update_vector, PRUint32 *nu
{
Replica *replica;
ReplicaId rid = -1; /* Used to create the replica keep alive subentry */
+ Slapi_DN *replarea_sdn = NULL;
replica = (Replica*) object_get_data(prp->replica_object);
if (replica)
{
@@ -1914,7 +1915,14 @@ send_updates(Private_Repl_Protocol *prp, RUV *remote_update_vector, PRUint32 *nu
slapi_log_error(SLAPI_LOG_REPL, repl_plugin_name,
"%s: skipped updates was definitely too high (%d) update the subentry now\n",
agmt_get_long_name(prp->agmt), skipped_updates);
- replica_subentry_update(agmt_get_replarea(prp->agmt), rid);
+ replarea_sdn = agmt_get_replarea(prp->agmt);
+ if (!replarea_sdn) {
+ slapi_log_error(SLAPI_LOG_FATAL, repl_plugin_name,
+ "send_updates: Unknown replication area due to agreement not found.");
+ return_value = UPDATE_FATAL_ERROR;
+ } else {
+ replica_subentry_update(replarea_sdn, rid);
+ }
}
/* Terminate the results reading thread */
if (!prp->repl50consumer)
diff --git a/ldap/servers/plugins/replication/repl5_replica_config.c b/ldap/servers/plugins/replication/repl5_replica_config.c
index 8d3c481..e85ae3e 100644
--- a/ldap/servers/plugins/replication/repl5_replica_config.c
+++ b/ldap/servers/plugins/replication/repl5_replica_config.c
@@ -2368,13 +2368,17 @@ replica_send_cleanruv_task(Repl_Agmt *agmt, cleanruv_data *clean_data)
conn_delete_internal_ext(conn);
return;
}
- val.bv_len = PR_snprintf(data, sizeof(data), "CLEANRUV%d", clean_data->rid);
sdn = agmt_get_replarea(agmt);
+ if (!sdn) {
+ conn_delete_internal_ext(conn);
+ return;
+ }
mod.mod_op = LDAP_MOD_ADD|LDAP_MOD_BVALUES;
mod.mod_type = "nsds5task";
mod.mod_bvalues = vals;
vals [0] = &val;
vals [1] = NULL;
+ val.bv_len = PR_snprintf(data, sizeof(data), "CLEANRUV%d", clean_data->rid);
val.bv_val = data;
mods[0] = &mod;
mods[1] = NULL;
diff --git a/ldap/servers/plugins/replication/repl5_tot_protocol.c b/ldap/servers/plugins/replication/repl5_tot_protocol.c
index 7da893a..16b51b5 100644
--- a/ldap/servers/plugins/replication/repl5_tot_protocol.c
+++ b/ldap/servers/plugins/replication/repl5_tot_protocol.c
@@ -329,6 +329,13 @@ repl5_tot_run(Private_Repl_Protocol *prp)
goto done;
}
+ area_sdn = agmt_get_replarea(prp->agmt);
+ if (!area_sdn) {
+ slapi_log_error(SLAPI_LOG_FATAL, repl_plugin_name, "Warning: unable to "
+ "get repl area. Please check agreement.\n");
+ goto done;
+ }
+
conn_set_timeout(prp->conn, agmt_get_timeout(prp->agmt));
/* acquire remote replica */
@@ -387,11 +394,10 @@ repl5_tot_run(Private_Repl_Protocol *prp)
slapi_log_error(SLAPI_LOG_FATAL, repl_plugin_name, "Beginning total update of replica "
"\"%s\".\n", agmt_get_long_name(prp->agmt));
- pb = slapi_pblock_new ();
+ /* RMREPL - need to send schema here */
- /* RMREPL - need to send schema here */
+ pb = slapi_pblock_new ();
- area_sdn = agmt_get_replarea(prp->agmt);
/* we need to provide managedsait control so that referral entries can
be replicated */
ctrls = (LDAPControl **)slapi_ch_calloc (3, sizeof (LDAPControl *));
diff --git a/ldap/servers/plugins/replication/repl_session_plugin.c b/ldap/servers/plugins/replication/repl_session_plugin.c
index 1c04089..2fa993d 100644
--- a/ldap/servers/plugins/replication/repl_session_plugin.c
+++ b/ldap/servers/plugins/replication/repl_session_plugin.c
@@ -48,6 +48,10 @@ repl_session_plugin_call_agmt_init_cb(Repl_Agmt *ra)
}
if (initfunc) {
replarea = agmt_get_replarea(ra);
+ if (!replarea) {
+ LDAPDebug0Args(LDAP_DEBUG_ANY, "repl_session_plugin_call_agmt_init_cb -- Aborted -- No replication area\n");
+ return;
+ }
cookie = (*initfunc)(replarea);
slapi_sdn_free(&replarea);
}
@@ -73,8 +77,11 @@ repl_session_plugin_call_pre_acquire_cb(const Repl_Agmt *ra, int is_total,
if (thefunc) {
replarea = agmt_get_replarea(ra);
- rc = (*thefunc)(agmt_get_priv(ra), replarea, is_total,
- data_guid, data);
+ if (!replarea) {
+ LDAPDebug0Args(LDAP_DEBUG_ANY, "repl_session_plugin_call_pre_acquire_cb -- Aborted -- No replication area\n");
+ return 1;
+ }
+ rc = (*thefunc)(agmt_get_priv(ra), replarea, is_total, data_guid, data);
slapi_sdn_free(&replarea);
}
@@ -95,8 +102,11 @@ repl_session_plugin_call_post_acquire_cb(const Repl_Agmt *ra, int is_total,
if (thefunc) {
replarea = agmt_get_replarea(ra);
- rc = (*thefunc)(agmt_get_priv(ra), replarea,
- is_total, data_guid, data);
+ if (!replarea) {
+ LDAPDebug0Args(LDAP_DEBUG_ANY, "repl_session_plugin_call_post_acquire_cb -- Aborted -- No replication area\n");
+ return 1;
+ }
+ rc = (*thefunc)(agmt_get_priv(ra), replarea, is_total, data_guid, data);
slapi_sdn_free(&replarea);
}
@@ -151,6 +161,10 @@ repl_session_plugin_call_destroy_agmt_cb(const Repl_Agmt *ra)
if (thefunc) {
replarea = agmt_get_replarea(ra);
+ if (!replarea) {
+ LDAPDebug0Args(LDAP_DEBUG_ANY, "repl_session_plugin_call_destroy_agmt_cb -- Aborted -- No replication area\n");
+ return;
+ }
(*thefunc)(agmt_get_priv(ra), replarea);
slapi_sdn_free(&replarea);
}
diff --git a/ldap/servers/plugins/replication/windows_protocol_util.c b/ldap/servers/plugins/replication/windows_protocol_util.c
index 5c12af7..084b520 100644
--- a/ldap/servers/plugins/replication/windows_protocol_util.c
+++ b/ldap/servers/plugins/replication/windows_protocol_util.c
@@ -5319,6 +5319,13 @@ windows_update_local_entry(Private_Repl_Protocol *prp,Slapi_Entry *remote_entry,
* in the groups caused by moving member entries.
* We need to update the local groups manually... */
local_subtree = agmt_get_replarea(prp->agmt);
+ if (!local_subtree) {
+ slapi_log_error(SLAPI_LOG_FATAL, windows_repl_plugin_name,
+ "failed to get local subtree from agreement\n");
+ local_entry = orig_local_entry;
+ orig_local_entry = NULL;
+ goto bail;
+ }
local_subtree_sdn = local_subtree;
orig_local_sdn = slapi_entry_get_sdn_const(orig_local_entry);
escaped_filter_val = slapi_escape_filter_value((char *)slapi_sdn_get_ndn(orig_local_sdn),
@@ -5651,6 +5658,11 @@ windows_search_local_entry_by_uniqueid(Private_Repl_Protocol *prp,
*ret_entry = NULL;
if (is_global) { /* Search from the suffix (rename case) */
local_subtree = agmt_get_replarea(prp->agmt);
+ if (!local_subtree) {
+ slapi_log_error(SLAPI_LOG_FATAL, windows_repl_plugin_name,
+ "failed to get local subtree from agreement\n");
+ return LDAP_PARAM_ERROR;
+ }
local_subtree_sdn = local_subtree;
} else {
local_subtree_sdn = windows_private_get_directory_treetop(prp->agmt);
--
1.9.3