From 7d7ea3a78c57a06faec864ec9f181f57f202d241 Mon Sep 17 00:00:00 2001
From: Rich Megginson <rmeggins@xxxxxxxxxx>
Date: Mon, 27 Aug 2012 12:31:30 -0600
Subject: [PATCH 2/3] fix mem leaks with parent dn log message, setting winsync windows domain
---
.../plugins/posix-winsync/posix-winsync-config.c | 4 +++-
ldap/servers/plugins/replication/windows_private.c | 2 ++
2 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/ldap/servers/plugins/posix-winsync/posix-winsync-config.c b/ldap/servers/plugins/posix-winsync/posix-winsync-config.c
index 8239456..a2d21be 100644
--- a/ldap/servers/plugins/posix-winsync/posix-winsync-config.c
+++ b/ldap/servers/plugins/posix-winsync/posix-winsync-config.c
@@ -81,8 +81,10 @@ posix_winsync_agmt_init(const Slapi_DN *ds_subtree, const Slapi_DN *ad_subtree)
sdn = slapi_get_next_suffix(&node, 0);
}
if (!sdn) {
+ char *pardn = slapi_dn_parent(slapi_sdn_get_dn(ds_subtree));
slapi_log_error(SLAPI_LOG_FATAL, POSIX_WINSYNC_PLUGIN_NAME, "suffix not found for '%s'\n",
- slapi_dn_parent(slapi_sdn_get_dn(ds_subtree)));
+ pardn);
+ slapi_ch_free_string(&pardn);
}
slapi_log_error(SLAPI_LOG_PLUGIN, POSIX_WINSYNC_PLUGIN_NAME,
diff --git a/ldap/servers/plugins/replication/windows_private.c b/ldap/servers/plugins/replication/windows_private.c
index ec619bc..5698d37 100644
--- a/ldap/servers/plugins/replication/windows_private.c
+++ b/ldap/servers/plugins/replication/windows_private.c
@@ -363,6 +363,7 @@ void windows_agreement_delete(Repl_Agmt *ra)
slapi_filter_free(dp->directory_filter, 1);
slapi_filter_free(dp->deleted_filter, 1);
slapi_entry_free(dp->raw_entry);
+ slapi_ch_free_string(&dp->windows_domain);
dp->raw_entry = NULL;
dp->api_cookie = NULL;
slapi_ch_free((void **)dp);
@@ -529,6 +530,7 @@ windows_private_set_windows_domain(const Repl_Agmt *ra, char *domain)
dp = (Dirsync_Private *) agmt_get_priv(ra);
PR_ASSERT (dp);
+ slapi_ch_free_string(&dp->windows_domain);
dp->windows_domain = domain;
LDAPDebug0Args( LDAP_DEBUG_TRACE, "<= windows_private_set_windows_domain\n" );
--
1.7.1
From 63205cac204a55027aa4b07232c7440355857139 Mon Sep 17 00:00:00 2001
From: Rich Megginson <rmeggins@xxxxxxxxxx>
Date: Mon, 27 Aug 2012 14:23:22 -0600
Subject: [PATCH 3/3] coverity - posix winsync mem leaks, null check, deadcode, null ref, use after free
Fixes the following issues:
13068 Resource leak
In posix_winsync_pre_ds_mod_group_cb(): Leak of memory or pointers to system resources.
13067 Resource leak
In posix_winsync_end_update_cb(): Leak of memory or pointers to system resources
13066 Resource leak
In modGroupMembership(): Leak of memory or pointers to system resources
13065 Resource leak
In dn_in_set(): Leak of memory or pointers to system resources
13064 Resource leak
In dn_in_set(): Leak of memory or pointers to system resources
13063 Dereference after null check
In windows_plugin_add(): Pointer is checked against null but then dereferenced anyway
13062 Explicit null dereferenced
In searchUid(): Dereference of an explicit null value
13061 Logically dead code
In check_account_lock(): 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
---
.../plugins/posix-winsync/posix-group-func.c | 14 ++++++++++----
ldap/servers/plugins/posix-winsync/posix-winsync.c | 9 +++++----
ldap/servers/plugins/replication/windows_private.c | 8 +++-----
3 files changed, 18 insertions(+), 13 deletions(-)
diff --git a/ldap/servers/plugins/posix-winsync/posix-group-func.c b/ldap/servers/plugins/posix-winsync/posix-group-func.c
index 72963ac..d4bbcfe 100644
--- a/ldap/servers/plugins/posix-winsync/posix-group-func.c
+++ b/ldap/servers/plugins/posix-winsync/posix-group-func.c
@@ -86,7 +86,7 @@ searchUid(const char *udn)
}
slapi_free_search_results_internal(int_search_pb);
slapi_pblock_destroy(int_search_pb);
- if (posix_winsync_config_get_lowercase()) {
+ if (uid && posix_winsync_config_get_lowercase()) {
return slapi_dn_ignore_case(uid);
}
return uid;
@@ -103,11 +103,15 @@ int
dn_in_set(const char* uid, char **uids)
{
int i;
- Slapi_DN *sdn_uid = slapi_sdn_new_dn_byval(uid);
- Slapi_DN *sdn_ul = slapi_sdn_new();
+ Slapi_DN *sdn_uid = NULL;
+ Slapi_DN *sdn_ul = NULL;
if (uids == NULL || uid == NULL)
return false;
+
+ sdn_uid = slapi_sdn_new_dn_byval(uid);
+ sdn_ul = slapi_sdn_new();
+
for (i = 0; uids[i]; i++) {
slapi_sdn_set_dn_byref(sdn_ul, uids[i]);
if (slapi_sdn_compare(sdn_uid, sdn_ul) == 0) {
@@ -346,7 +350,9 @@ modGroupMembership(Slapi_Entry *entry, Slapi_Mods *smods, int *do_modify)
"modGroupMembership: add to modlist %s\n", uid);
doModify = true;
}
- /* slapi_value_free(&v); */
+ slapi_valueset_free(vs);
+ slapi_value_init_berval(v, NULL); /* otherwise we will try to free memory we do not own */
+ slapi_value_free(&v);
}
}
}
diff --git a/ldap/servers/plugins/posix-winsync/posix-winsync.c b/ldap/servers/plugins/posix-winsync/posix-winsync.c
index 6eddc23..dc56475 100644
--- a/ldap/servers/plugins/posix-winsync/posix-winsync.c
+++ b/ldap/servers/plugins/posix-winsync/posix-winsync.c
@@ -184,8 +184,8 @@ check_account_lock(Slapi_Entry *ds_entry, int *isvirt)
rc = 1; /* no attr == entry is enabled */
slapi_log_error(SLAPI_LOG_PLUGIN, posix_winsync_plugin_name,
"<-- check_account_lock - entry [%s] does not "
- "have attribute nsAccountLock - entry %s locked\n",
- slapi_entry_get_dn_const(ds_entry), rc ? "is not" : "is");
+ "have attribute nsAccountLock - entry is not locked\n",
+ slapi_entry_get_dn_const(ds_entry));
}
return rc;
@@ -918,7 +918,7 @@ posix_winsync_pre_ds_mod_group_cb(void *cbdata, const Slapi_Entry *rawentry, Sla
slapi_value_init_string(voc, "posixGroup");
slapi_entry_attr_find(ds_entry, "objectClass", &oc_attr);
if (slapi_attr_value_find(oc_attr, slapi_value_get_berval(voc)) != 0) {
- Slapi_ValueSet *oc_vs = slapi_valueset_new();
+ Slapi_ValueSet *oc_vs = NULL;
Slapi_Value *oc_nv = slapi_value_new();
slapi_attr_get_valueset(oc_attr, &oc_vs);
@@ -933,7 +933,7 @@ posix_winsync_pre_ds_mod_group_cb(void *cbdata, const Slapi_Entry *rawentry, Sla
slapi_value_free(&oc_nv);
slapi_log_error(SLAPI_LOG_PLUGIN, posix_winsync_plugin_name,
"_pre_ds_mod_group_cb step\n");
- /* slapi_valuset_free(oc_vs); */
+ slapi_valueset_free(oc_vs);
slapi_log_error(SLAPI_LOG_PLUGIN, posix_winsync_plugin_name,
"_pre_ds_mod_group_cb step\n");
}
@@ -1292,6 +1292,7 @@ posix_winsync_end_update_cb(void *cbdata, const Slapi_DN *ds_subtree, const Slap
slapi_log_error(SLAPI_LOG_PLUGIN, posix_winsync_plugin_name,
"--> posix_winsync_end_update_cb, create task %s\n", dn);
if (NULL == dn) {
+ slapi_pblock_destroy(pb);
slapi_log_error(SLAPI_LOG_FATAL, posix_winsync_plugin_name,
"posix_winsync_end_update_cb: "
"failed to create task dn: cn=%s,%s,cn=tasks,cn=config\n",
diff --git a/ldap/servers/plugins/replication/windows_private.c b/ldap/servers/plugins/replication/windows_private.c
index 5698d37..66e1e98 100644
--- a/ldap/servers/plugins/replication/windows_private.c
+++ b/ldap/servers/plugins/replication/windows_private.c
@@ -1241,7 +1241,7 @@ windows_plugin_add(void **theapi, int maxapi)
}
elem = PR_NEXT_LINK(elem);
}
- if (wpi) { /* was not added - precedence too high */
+ if (elem && wpi) { /* was not added - precedence too high */
/* just add to end of list */
PR_INSERT_BEFORE(wpi, elem);
wpi = NULL; /* owned by list now */
@@ -1367,10 +1367,8 @@ windows_plugin_cleanup_agmt(Repl_Agmt *ra)
while (list && !PR_CLIST_IS_EMPTY(list)) {
elem = PR_LIST_HEAD(list);
- if (elem != list) {
- PR_REMOVE_LINK(elem);
- slapi_ch_free((void **)&elem);
- }
+ PR_REMOVE_LINK(elem);
+ slapi_ch_free((void **)&elem);
}
slapi_ch_free((void **)&list);
windows_private_set_api_cookie(ra, NULL);
--
1.7.1
--
389-devel mailing list
389-devel@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/389-devel
