[389-commits] Branch '389-ds-base-1.2.11' - 2 commits - ldap/servers
Richard Allen Megginson
rmeggins at fedoraproject.org
Tue Aug 21 16:40:56 UTC 2012
ldap/servers/plugins/memberof/memberof.c | 3
ldap/servers/plugins/replication/repl5_agmt.c | 4
ldap/servers/plugins/replication/repl5_replica.c | 7 -
ldap/servers/plugins/replication/windows_private.c | 83 ++++++++++-----
ldap/servers/plugins/replication/windows_protocol_util.c | 27 +++-
5 files changed, 85 insertions(+), 39 deletions(-)
New commits:
commit 487932d3a797122257fa66747f748b6ff9ea1290
Author: Rich Megginson <rmeggins at redhat.com>
Date: Tue Aug 21 10:00:21 2012 -0600
coverity - mbo dead code - winsync leaks, deadcode, null check, test code
Reviewed by: mreynolds (Thanks!)
Cherry-picked from 3dcca62d79ece2ef02b8796a1ffd1465d3e69487
This fixes the following issues:
13060 Logically dead code
In memberof_test_membership_callback(): Code can never be reached because of a logical contradiction
13059 Use after free
In windows_plugin_cleanup_agmt(): A pointer to freed memory is dereferenced, used as a function argument, or otherwise used
13058 Dereference before null check
In windows_generate_update_mods(): All paths that lead to this null pointer comparison already dereference the pointer earlier
13057 Dereference before null check
In windows_generate_update_mods(): All paths that lead to this null pointer comparison already dereference the pointer earlier
13056 Resource leak
In windows_plugin_add(): Leak of memory or pointers to system resources
13055 Dereference after null check
In windows_generate_update_mods(): Pointer is checked against null but then dereferenced anyway
13054 Dereference after null check
In test_winsync_pre_ds_search_all_cb(): Pointer is checked against null but then dereferenced anyway
I've also commented out any code from the test winsync plugin except for
logging the entrance and exit of each function.
diff --git a/ldap/servers/plugins/memberof/memberof.c b/ldap/servers/plugins/memberof/memberof.c
index a2fdbd9..598f4d9 100644
--- a/ldap/servers/plugins/memberof/memberof.c
+++ b/ldap/servers/plugins/memberof/memberof.c
@@ -1873,12 +1873,11 @@ int memberof_test_membership_callback(Slapi_Entry *e, void *callback_data)
entry_sdn = slapi_entry_get_sdn(e);
entry_dn = slapi_value_new_string(slapi_entry_get_ndn(e));
- slapi_value_set_flags(entry_dn, SLAPI_ATTR_FLAG_NORMALIZED_CIS);
-
if(0 == entry_dn)
{
goto bail;
}
+ slapi_value_set_flags(entry_dn, SLAPI_ATTR_FLAG_NORMALIZED_CIS);
/* divide groups into member and non-member lists */
slapi_entry_attr_find(e, config->memberof_attr, &attr );
diff --git a/ldap/servers/plugins/replication/windows_private.c b/ldap/servers/plugins/replication/windows_private.c
index 85b585a..5c7b725 100644
--- a/ldap/servers/plugins/replication/windows_private.c
+++ b/ldap/servers/plugins/replication/windows_private.c
@@ -1103,13 +1103,15 @@ windows_plugin_add(void **theapi, int maxapi)
while (elem && (elem != &winsync_plugin_list)) {
if (precedence < elem->precedence) {
PR_INSERT_BEFORE(wpi, elem);
+ wpi = NULL; /* owned by list now */
break;
}
elem = PR_NEXT_LINK(elem);
}
- if (elem == &winsync_plugin_list) {
+ if (wpi) { /* was not added - precedence too high */
/* just add to end of list */
PR_INSERT_BEFORE(wpi, elem);
+ wpi = NULL; /* owned by list now */
}
return 0;
}
@@ -1232,8 +1234,10 @@ windows_plugin_cleanup_agmt(Repl_Agmt *ra)
while (list && !PR_CLIST_IS_EMPTY(list)) {
elem = PR_LIST_HEAD(list);
- PR_REMOVE_LINK(elem);
- slapi_ch_free((void **)&elem);
+ if (elem != list) {
+ PR_REMOVE_LINK(elem);
+ slapi_ch_free((void **)&elem);
+ }
}
slapi_ch_free((void **)&list);
windows_private_set_api_cookie(ra, NULL);
@@ -1681,12 +1685,19 @@ test_winsync_pre_ds_search_all_cb(void *cbdata, const char *agmt_dn,
"--> test_winsync_pre_ds_search_all_cb -- orig filter [%s] -- begin\n",
((filter && *filter) ? *filter : "NULL"));
- /* We only want to grab users from the ds side - no groups */
- slapi_ch_free_string(filter);
- /* maybe use ntUniqueId=* - only get users that have already been
- synced with AD already - ntUniqueId and ntUserDomainId are
- indexed for equality only - need to add presence? */
- *filter = slapi_ch_strdup("(&(objectclass=ntuser)(ntUserDomainId=*))");
+#ifdef THIS_IS_JUST_AN_EXAMPLE
+ if (filter) {
+ /* We only want to grab users from the ds side - no groups */
+ slapi_ch_free_string(filter);
+ /* maybe use ntUniqueId=* - only get users that have already been
+ synced with AD already - ntUniqueId and ntUserDomainId are
+ indexed for equality only - need to add presence? */
+ *filter = slapi_ch_strdup("(&(objectclass=ntuser)(ntUserDomainId=*))");
+ slapi_log_error(SLAPI_LOG_PLUGIN, test_winsync_plugin_name,
+ "--> test_winsync_pre_ds_search_all_cb -- new filter [%s]\n",
+ *filter ? *filter : "NULL"));
+ }
+#endif
slapi_log_error(SLAPI_LOG_PLUGIN, test_winsync_plugin_name,
"<-- test_winsync_pre_ds_search_all_cb -- end\n");
@@ -1781,13 +1792,12 @@ test_winsync_get_new_ds_user_dn_cb(void *cbdata, const Slapi_Entry *rawentry,
Slapi_Entry *ad_entry, char **new_dn_string,
const Slapi_DN *ds_suffix, const Slapi_DN *ad_suffix)
{
- char **rdns = NULL;
-
slapi_log_error(SLAPI_LOG_PLUGIN, test_winsync_plugin_name,
"--> test_winsync_get_new_ds_user_dn_cb -- old dn [%s] -- begin\n",
*new_dn_string);
- rdns = slapi_ldap_explode_dn(*new_dn_string, 0);
+#ifdef THIS_IS_JUST_AN_EXAMPLE
+ char **rdns = slapi_ldap_explode_dn(*new_dn_string, 0);
if (!rdns || !rdns[0]) {
slapi_ldap_value_free(rdns);
return;
@@ -1796,6 +1806,7 @@ test_winsync_get_new_ds_user_dn_cb(void *cbdata, const Slapi_Entry *rawentry,
slapi_ch_free_string(new_dn_string);
*new_dn_string = PR_smprintf("%s,%s", rdns[0], slapi_sdn_get_dn(ds_suffix));
slapi_ldap_value_free(rdns);
+#endif
slapi_log_error(SLAPI_LOG_PLUGIN, test_winsync_plugin_name,
"<-- test_winsync_get_new_ds_user_dn_cb -- new dn [%s] -- end\n",
@@ -1909,10 +1920,12 @@ test_winsync_post_ad_mod_user_cb(void *cookie, const Slapi_Entry *rawentry, Slap
slapi_log_error(SLAPI_LOG_PLUGIN, test_winsync_plugin_name,
"--> test_winsync_post_ad_mod_user_cb -- begin\n");
+#ifdef THIS_IS_JUST_AN_EXAMPLE
slapi_log_error(SLAPI_LOG_PLUGIN, test_winsync_plugin_name,
"Result of modifying AD entry [%s] was [%d:%s]\n",
slapi_entry_get_dn(ad_entry), *result, ldap_err2string(*result));
-
+#endif
+
slapi_log_error(SLAPI_LOG_PLUGIN, test_winsync_plugin_name,
"<-- test_winsync_post_ad_mod_user_cb -- end\n");
@@ -1925,10 +1938,12 @@ test_winsync_post_ad_mod_group_cb(void *cookie, const Slapi_Entry *rawentry, Sla
slapi_log_error(SLAPI_LOG_PLUGIN, test_winsync_plugin_name,
"--> test_winsync_post_ad_mod_group_cb -- begin\n");
+#ifdef THIS_IS_JUST_AN_EXAMPLE
slapi_log_error(SLAPI_LOG_PLUGIN, test_winsync_plugin_name,
"Result of modifying AD entry [%s] was [%d:%s]\n",
slapi_entry_get_dn(ad_entry), *result, ldap_err2string(*result));
-
+#endif
+
slapi_log_error(SLAPI_LOG_PLUGIN, test_winsync_plugin_name,
"<-- test_winsync_post_ad_mod_group_cb -- end\n");
@@ -1941,10 +1956,12 @@ test_winsync_post_ds_mod_user_cb(void *cookie, const Slapi_Entry *rawentry, Slap
slapi_log_error(SLAPI_LOG_PLUGIN, test_winsync_plugin_name,
"--> test_winsync_post_ds_mod_user_cb -- begin\n");
+#ifdef THIS_IS_JUST_AN_EXAMPLE
slapi_log_error(SLAPI_LOG_PLUGIN, test_winsync_plugin_name,
"Result of modifying DS entry [%s] was [%d:%s]\n",
slapi_entry_get_dn(ds_entry), *result, ldap_err2string(*result));
-
+#endif
+
slapi_log_error(SLAPI_LOG_PLUGIN, test_winsync_plugin_name,
"<-- test_winsync_post_ds_mod_user_cb -- end\n");
@@ -1957,10 +1974,12 @@ test_winsync_post_ds_mod_group_cb(void *cookie, const Slapi_Entry *rawentry, Sla
slapi_log_error(SLAPI_LOG_PLUGIN, test_winsync_plugin_name,
"--> test_winsync_post_ds_mod_group_cb -- begin\n");
+#ifdef THIS_IS_JUST_AN_EXAMPLE
slapi_log_error(SLAPI_LOG_PLUGIN, test_winsync_plugin_name,
"Result of modifying DS entry [%s] was [%d:%s]\n",
slapi_entry_get_dn(ds_entry), *result, ldap_err2string(*result));
-
+#endif
+
slapi_log_error(SLAPI_LOG_PLUGIN, test_winsync_plugin_name,
"<-- test_winsync_post_ds_mod_group_cb -- end\n");
@@ -1973,10 +1992,12 @@ test_winsync_post_ds_add_user_cb(void *cookie, const Slapi_Entry *rawentry, Slap
slapi_log_error(SLAPI_LOG_PLUGIN, test_winsync_plugin_name,
"--> test_winsync_post_ds_add_user_cb -- begin\n");
+#ifdef THIS_IS_JUST_AN_EXAMPLE
slapi_log_error(SLAPI_LOG_PLUGIN, test_winsync_plugin_name,
"Result of adding DS entry [%s] was [%d:%s]\n",
slapi_entry_get_dn(ds_entry), *result, ldap_err2string(*result));
-
+#endif
+
slapi_log_error(SLAPI_LOG_PLUGIN, test_winsync_plugin_name,
"<-- test_winsync_post_ds_add_user_cb -- end\n");
@@ -1989,10 +2010,12 @@ test_winsync_post_ds_add_group_cb(void *cookie, const Slapi_Entry *rawentry, Sla
slapi_log_error(SLAPI_LOG_PLUGIN, test_winsync_plugin_name,
"--> test_winsync_post_ds_add_group_cb -- begin\n");
+#ifdef THIS_IS_JUST_AN_EXAMPLE
slapi_log_error(SLAPI_LOG_PLUGIN, test_winsync_plugin_name,
"Result of adding DS entry [%s] was [%d:%s]\n",
slapi_entry_get_dn(ds_entry), *result, ldap_err2string(*result));
-
+#endif
+
slapi_log_error(SLAPI_LOG_PLUGIN, test_winsync_plugin_name,
"<-- test_winsync_post_ds_add_group_cb -- end\n");
@@ -2005,11 +2028,13 @@ test_winsync_pre_ad_add_user_cb(void *cookie, Slapi_Entry *ds_entry, Slapi_Entry
slapi_log_error(SLAPI_LOG_PLUGIN, test_winsync_plugin_name,
"--> test_winsync_pre_ad_add_user_cb -- begin\n");
+#ifdef THIS_IS_JUST_AN_EXAMPLE
slapi_log_error(SLAPI_LOG_PLUGIN, test_winsync_plugin_name,
"Adding AD entry [%s] from add of DS entry [%s]\n",
slapi_entry_get_dn(ad_entry), slapi_entry_get_dn(ds_entry));
/* make modifications to ad_entry here */
-
+#endif
+
slapi_log_error(SLAPI_LOG_PLUGIN, test_winsync_plugin_name,
"<-- test_winsync_pre_ad_add_user_cb -- end\n");
@@ -2022,11 +2047,13 @@ test_winsync_pre_ad_add_group_cb(void *cookie, Slapi_Entry *ds_entry, Slapi_Entr
slapi_log_error(SLAPI_LOG_PLUGIN, test_winsync_plugin_name,
"--> test_winsync_pre_ad_add_group_cb -- begin\n");
+#ifdef THIS_IS_JUST_AN_EXAMPLE
slapi_log_error(SLAPI_LOG_PLUGIN, test_winsync_plugin_name,
"Adding AD entry [%s] from add of DS entry [%s]\n",
slapi_entry_get_dn(ad_entry), slapi_entry_get_dn(ds_entry));
/* make modifications to ad_entry here */
-
+#endif
+
slapi_log_error(SLAPI_LOG_PLUGIN, test_winsync_plugin_name,
"<-- test_winsync_pre_ad_add_group_cb -- end\n");
@@ -2039,10 +2066,12 @@ test_winsync_post_ad_add_user_cb(void *cookie, Slapi_Entry *ds_entry, Slapi_Entr
slapi_log_error(SLAPI_LOG_PLUGIN, test_winsync_plugin_name,
"--> test_winsync_post_ad_add_user_cb -- begin\n");
+#ifdef THIS_IS_JUST_AN_EXAMPLE
slapi_log_error(SLAPI_LOG_PLUGIN, test_winsync_plugin_name,
"Result of adding AD entry [%s] was [%d:%s]\n",
slapi_entry_get_dn(ad_entry), *result, ldap_err2string(*result));
-
+#endif
+
slapi_log_error(SLAPI_LOG_PLUGIN, test_winsync_plugin_name,
"<-- test_winsync_post_ad_add_user_cb -- end\n");
@@ -2055,10 +2084,12 @@ test_winsync_post_ad_add_group_cb(void *cookie, Slapi_Entry *ds_entry, Slapi_Ent
slapi_log_error(SLAPI_LOG_PLUGIN, test_winsync_plugin_name,
"--> test_winsync_post_ad_add_group_cb -- begin\n");
+#ifdef THIS_IS_JUST_AN_EXAMPLE
slapi_log_error(SLAPI_LOG_PLUGIN, test_winsync_plugin_name,
"Result of adding AD entry [%s] was [%d:%s]\n",
slapi_entry_get_dn(ad_entry), *result, ldap_err2string(*result));
-
+#endif
+
slapi_log_error(SLAPI_LOG_PLUGIN, test_winsync_plugin_name,
"<-- test_winsync_post_ad_add_group_cb -- end\n");
@@ -2071,10 +2102,12 @@ test_winsync_post_ad_mod_user_mods_cb(void *cookie, const Slapi_Entry *rawentry,
slapi_log_error(SLAPI_LOG_PLUGIN, test_winsync_plugin_name,
"--> test_winsync_post_ad_mod_user_mods_cb -- begin\n");
+#ifdef THIS_IS_JUST_AN_EXAMPLE
slapi_log_error(SLAPI_LOG_PLUGIN, test_winsync_plugin_name,
"Result of modifying AD entry [%s] was [%d:%s]\n",
slapi_sdn_get_dn(remote_dn), *result, ldap_err2string(*result));
-
+#endif
+
slapi_log_error(SLAPI_LOG_PLUGIN, test_winsync_plugin_name,
"<-- test_winsync_post_ad_mod_user_mods_cb -- end\n");
@@ -2087,10 +2120,12 @@ test_winsync_post_ad_mod_group_mods_cb(void *cookie, const Slapi_Entry *rawentry
slapi_log_error(SLAPI_LOG_PLUGIN, test_winsync_plugin_name,
"--> test_winsync_post_ad_mod_group_mods_cb -- begin\n");
+#ifdef THIS_IS_JUST_AN_EXAMPLE
slapi_log_error(SLAPI_LOG_PLUGIN, test_winsync_plugin_name,
"Result of modifying AD entry [%s] was [%d:%s]\n",
slapi_sdn_get_dn(remote_dn), *result, ldap_err2string(*result));
-
+#endif
+
slapi_log_error(SLAPI_LOG_PLUGIN, test_winsync_plugin_name,
"<-- test_winsync_post_ad_mod_group_mods_cb -- end\n");
diff --git a/ldap/servers/plugins/replication/windows_protocol_util.c b/ldap/servers/plugins/replication/windows_protocol_util.c
index 518a133..79e5c7c 100644
--- a/ldap/servers/plugins/replication/windows_protocol_util.c
+++ b/ldap/servers/plugins/replication/windows_protocol_util.c
@@ -4145,6 +4145,16 @@ windows_generate_update_mods(Private_Repl_Protocol *prp,Slapi_Entry *remote_entr
LDAPDebug( LDAP_DEBUG_TRACE, "=> windows_generate_update_mods\n", 0, 0, 0 );
*do_modify = 0;
+
+ if (!remote_entry || !local_entry) {
+ slapi_log_error(SLAPI_LOG_REPL, windows_repl_plugin_name,
+ "%s: windows_generate_update_mods: remote_entry is [%s] local_entry is [%s] "
+ "cannot generate update mods\n", agmt_get_long_name(prp->agmt),
+ remote_entry ? slapi_entry_get_dn_const(remote_entry) : "NULL",
+ local_entry ? slapi_entry_get_dn_const(local_entry) : "NULL");
+ goto bail;
+ }
+
if (to_windows)
{
windows_is_local_entry_user_or_group(remote_entry,&is_user,&is_group);
@@ -4153,8 +4163,8 @@ windows_generate_update_mods(Private_Repl_Protocol *prp,Slapi_Entry *remote_entr
windows_is_remote_entry_user_or_group(remote_entry,&is_user,&is_group);
}
- for (rc = slapi_entry_first_attr(remote_entry, &attr); rc == 0;
- rc = slapi_entry_next_attr(remote_entry, attr, &attr))
+ for (rc = slapi_entry_first_attr(remote_entry, &attr); rc == 0;
+ rc = slapi_entry_next_attr(remote_entry, attr, &attr))
{
int is_present_local = 0;
char *type = NULL;
@@ -4324,9 +4334,9 @@ windows_generate_update_mods(Private_Repl_Protocol *prp,Slapi_Entry *remote_entr
"local attribute %s in local entry %s for remote attribute "
"%s in remote entry %s\n",
local_type ? local_type : "NULL",
- local_entry ? slapi_entry_get_dn(local_entry) : "NULL",
+ slapi_entry_get_dn(local_entry),
type ? type : "NULL",
- remote_entry ? slapi_entry_get_dn(remote_entry) : "NULL");
+ slapi_entry_get_dn(remote_entry));
}
slapi_valueset_free(local_values);
local_values = NULL;
@@ -4336,9 +4346,9 @@ windows_generate_update_mods(Private_Repl_Protocol *prp,Slapi_Entry *remote_entr
"local attribute %s in local entry %s for remote attribute "
"%s in remote entry %s\n",
local_type ? local_type : "NULL",
- local_entry ? slapi_entry_get_dn(local_entry) : "NULL",
+ slapi_entry_get_dn(local_entry),
type ? type : "NULL",
- remote_entry ? slapi_entry_get_dn(remote_entry) : "NULL");
+ slapi_entry_get_dn(remote_entry));
}
slapi_valueset_free(mapped_remote_values);
mapped_remote_values = NULL;
@@ -4348,9 +4358,9 @@ windows_generate_update_mods(Private_Repl_Protocol *prp,Slapi_Entry *remote_entr
"local attribute %s in local entry %s for remote attribute "
"%s in remote entry %s\n",
local_type ? local_type : "NULL",
- local_entry ? slapi_entry_get_dn(local_entry) : "NULL",
+ slapi_entry_get_dn(local_entry),
type ? type : "NULL",
- remote_entry ? slapi_entry_get_dn(remote_entry) : "NULL");
+ slapi_entry_get_dn(remote_entry));
}
}
} else
@@ -4521,6 +4531,7 @@ windows_generate_update_mods(Private_Repl_Protocol *prp,Slapi_Entry *remote_entr
{
slapi_mods_dump(smods,"windows sync");
}
+bail:
LDAPDebug( LDAP_DEBUG_TRACE, "<= windows_generate_update_mods: %d\n", retval, 0, 0 );
return retval;
}
commit 2734a710d4283f6584768a3f15b583d4952e42cb
Author: Mark Reynolds <mreynolds at redhat.com>
Date: Tue Aug 21 12:18:21 2012 -0400
CLEANALLRUV coverity fixes
There was code that allowed for a static buffer overflow(CLEANRIDSIZ) in a few places,
but there are previous checks that make this impossible to over run.
A small potential memory leak was also fixed.
Reviewed by: Noriko(Thanks!)
(cherry picked from commit 37e0121445b69405a7d11f9990eb91dc3d3a0d7c)
diff --git a/ldap/servers/plugins/replication/repl5_agmt.c b/ldap/servers/plugins/replication/repl5_agmt.c
index 5817f99..e573d40 100644
--- a/ldap/servers/plugins/replication/repl5_agmt.c
+++ b/ldap/servers/plugins/replication/repl5_agmt.c
@@ -434,7 +434,7 @@ agmt_new_from_entry(Slapi_Entry *e)
for (i = 0; i < CLEANRIDSIZ && clean_vals[i]; i++){
ra->cleanruv_notified[i] = atoi(clean_vals[i]);
}
- if(i <= CLEANRIDSIZ)
+ if(i < CLEANRIDSIZ)
ra->cleanruv_notified[i + 1] = 0;
slapi_ch_array_free(clean_vals);
} else {
@@ -2676,7 +2676,7 @@ agmt_set_cleanruv_notified_from_entry(Repl_Agmt *ra, Slapi_Entry *e){
for (i = 0; i < CLEANRIDSIZ && attr_vals[i]; i++){
ra->cleanruv_notified[i] = atoi(attr_vals[i]);
}
- if( i <= CLEANRIDSIZ )
+ if( i < CLEANRIDSIZ )
ra->cleanruv_notified[i + 1] = 0;
slapi_ch_array_free(attr_vals);
} else {
diff --git a/ldap/servers/plugins/replication/repl5_replica.c b/ldap/servers/plugins/replication/repl5_replica.c
index 75f82ab..b79280c 100644
--- a/ldap/servers/plugins/replication/repl5_replica.c
+++ b/ldap/servers/plugins/replication/repl5_replica.c
@@ -1954,6 +1954,7 @@ done:
if(payload == NULL){
slapi_log_error(SLAPI_LOG_FATAL, repl_plugin_name, "Abort CleanAllRUV Task: failed to create extended "
"op payload\n");
+ slapi_ch_free((void **)&data);
} else {
/* setup the data */
data->repl_obj = NULL;
@@ -3796,10 +3797,10 @@ replica_add_cleanruv_data(Replica *r, char *val)
PR_Lock(r->repl_lock);
for (i = 0; i < CLEANRIDSIZ && r->repl_cleanruv_data[i] != NULL; i++); /* goto the end of the list */
- if( i < CLEANRIDSIZ)
+ if( i < CLEANRIDSIZ){
r->repl_cleanruv_data[i] = slapi_ch_strdup(val); /* append to list */
- if(i <= CLEANRIDSIZ)
- r->repl_cleanruv_data[i + 1] = NULL;
+ r->repl_cleanruv_data[i + 1] = 0;
+ }
PR_Unlock(r->repl_lock);
}
More information about the 389-commits
mailing list