Re: [Freeipa-devel] [PATCHES] #2037 Coverity issues
On Thu, 03 Nov 2011, Simo Sorce wrote: These are actual issues. Most are resource leaks. And one is a bad sizeof() computation that will cause us later to overwrite out of bound memory, so potentially a segfault. Went through all of these. ACK. https://fedorahosted.org/freeipa/ticket/2037 --- daemons/ipa-kdb/ipa_kdb.c | 13 +++-- 1 files changed, 7 insertions(+), 6 deletions(-) diff --git a/daemons/ipa-kdb/ipa_kdb.c b/daemons/ipa-kdb/ipa_kdb.c index 6a6c2063902f8b2a76d97f3510f09333c5af168d..481b1f392766498c5d7c6333fe73bafefde87dae 100644 --- a/daemons/ipa-kdb/ipa_kdb.c +++ b/daemons/ipa-kdb/ipa_kdb.c @@ -263,6 +263,13 @@ int ipadb_get_connection(struct ipadb_context *ipactx) done: ldap_msgfree(res); + +ldap_value_free_len(vals); +for (i = 0; i c cvals[i]; i++) { +free(cvals[i]); +} +free(cvals); + if (ret) { if (ipactx-lcontext) { ldap_unbind_ext_s(ipactx-lcontext, NULL, NULL); @@ -274,12 +281,6 @@ done: return EIO; } -ldap_value_free_len(vals); -for (i = 0; i c; i++) { -free(cvals[i]); -} -free(cvals); - return 0; } ACK. https://fedorahosted.org/freeipa/ticket/2037 --- daemons/ipa-kdb/ipa_kdb_passwords.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/daemons/ipa-kdb/ipa_kdb_passwords.c b/daemons/ipa-kdb/ipa_kdb_passwords.c index 93e9e206081af412a472ab0c7624611a628a15b7..0bb7fa72496789241e27ecc852a1c6ede7f8e40a 100644 --- a/daemons/ipa-kdb/ipa_kdb_passwords.c +++ b/daemons/ipa-kdb/ipa_kdb_passwords.c @@ -203,6 +203,7 @@ krb5_error_code ipadb_change_pwd(krb5_context context, ret = asprintf(ied-pw_policy_dn, cn=global_policy,%s, ipactx-realm_base); if (ret == -1) { +free(ied); return ENOMEM; } db_entry-e_data = (krb5_octet *)ied; ACK. https://fedorahosted.org/freeipa/ticket/2037 --- util/ipa_pwd.c |4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) diff --git a/util/ipa_pwd.c b/util/ipa_pwd.c index c41617533ec25e8e656e9bb7a69d5b8b5dd8b5f7..fda6cb34ef24059362207325db61aedb62d7b665 100644 --- a/util/ipa_pwd.c +++ b/util/ipa_pwd.c @@ -560,7 +560,7 @@ int ipapwd_generate_new_history(char *password, unsigned char *hash = NULL; unsigned int hash_len; char *new_element; -char **ordered; +char **ordered = NULL; int c, i, n; int len; int ret; @@ -626,9 +626,11 @@ int ipapwd_generate_new_history(char *password, *new_pwd_history = ordered; *new_pwd_hlen = n; +ordered = NULL; ret = IPAPWD_POLICY_OK; done: +free(ordered); free(hash); return ret; } ACK https://fedorahosted.org/freeipa/ticket/2037 --- daemons/ipa-kdb/ipa_kdb_principals.c |7 +++ 1 files changed, 7 insertions(+), 0 deletions(-) diff --git a/daemons/ipa-kdb/ipa_kdb_principals.c b/daemons/ipa-kdb/ipa_kdb_principals.c index fdd834f355fd9e056058fa205b217e9e1f142e51..117eea86952ee4662930b80ba5e54c75aa587faf 100644 --- a/daemons/ipa-kdb/ipa_kdb_principals.c +++ b/daemons/ipa-kdb/ipa_kdb_principals.c @@ -1571,6 +1571,7 @@ static krb5_error_code ipadb_entry_to_mods(krb5_context kcontext, char **new_history; int nh_len; int ret; +int i; ied = (struct ipadb_e_data *)entry-e_data; if (ied-magic != IPA_E_DATA_MAGIC) { @@ -1619,6 +1620,12 @@ static krb5_error_code ipadb_entry_to_mods(krb5_context kcontext, kerr = ipadb_get_ldap_mod_str_list(imods, passwordHistory, new_history, nh_len, mod_op); + +for (i = 0; i nh_len; i++) { +free(new_history[i]); +} +free(new_history); + if (kerr) { goto done; } ACK. https://fedorahosted.org/freeipa/ticket/2037 --- daemons/ipa-kdb/ipa_kdb_principals.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/daemons/ipa-kdb/ipa_kdb_principals.c b/daemons/ipa-kdb/ipa_kdb_principals.c index 117eea86952ee4662930b80ba5e54c75aa587faf..bb1356a01a27a639c15439187ffb8d3537c1fcec 100644 --- a/daemons/ipa-kdb/ipa_kdb_principals.c +++ b/daemons/ipa-kdb/ipa_kdb_principals.c @@ -334,6 +334,7 @@ done: free(keys[i].key_data_contents[0]); free(keys[i].key_data_contents[1]); } +free(keys); *result = NULL; *num = 0; } ACK. https://fedorahosted.org/freeipa/ticket/2037 --- daemons/ipa-kdb/ipa_kdb_principals.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/daemons/ipa-kdb/ipa_kdb_principals.c b/daemons/ipa-kdb/ipa_kdb_principals.c index
Re: [Freeipa-devel] [PATCHES] #2037 Coverity issues
On Tue, 2011-11-08 at 00:22 +0200, Alexander Bokovoy wrote: Went through all of these. ACK. Thanks again, pushed to master. Simo. -- Simo Sorce * Red Hat, Inc * New York ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCHES] #2037 Coverity issues
These are actual issues. Most are resource leaks. And one is a bad sizeof() computation that will cause us later to overwrite out of bound memory, so potentially a segfault. Simo. -- Simo Sorce * Red Hat, Inc * New York From dff903a1f3a15516c36f40d4dffeaf751dba6423 Mon Sep 17 00:00:00 2001 From: Simo Sorce sso...@redhat.com Date: Thu, 3 Nov 2011 13:21:59 -0400 Subject: [PATCH 1/9] Fix CID 11019: Resource leak https://fedorahosted.org/freeipa/ticket/2037 --- daemons/ipa-kdb/ipa_kdb.c | 13 +++-- 1 files changed, 7 insertions(+), 6 deletions(-) diff --git a/daemons/ipa-kdb/ipa_kdb.c b/daemons/ipa-kdb/ipa_kdb.c index 6a6c2063902f8b2a76d97f3510f09333c5af168d..481b1f392766498c5d7c6333fe73bafefde87dae 100644 --- a/daemons/ipa-kdb/ipa_kdb.c +++ b/daemons/ipa-kdb/ipa_kdb.c @@ -263,6 +263,13 @@ int ipadb_get_connection(struct ipadb_context *ipactx) done: ldap_msgfree(res); + +ldap_value_free_len(vals); +for (i = 0; i c cvals[i]; i++) { +free(cvals[i]); +} +free(cvals); + if (ret) { if (ipactx-lcontext) { ldap_unbind_ext_s(ipactx-lcontext, NULL, NULL); @@ -274,12 +281,6 @@ done: return EIO; } -ldap_value_free_len(vals); -for (i = 0; i c; i++) { -free(cvals[i]); -} -free(cvals); - return 0; } -- 1.7.6.4 From d83e698e2f998767cec8c2505dff666f9c51 Mon Sep 17 00:00:00 2001 From: Simo Sorce sso...@redhat.com Date: Thu, 3 Nov 2011 13:26:45 -0400 Subject: [PATCH 2/9] Fix CID 11020: Resource leak https://fedorahosted.org/freeipa/ticket/2037 --- daemons/ipa-kdb/ipa_kdb_passwords.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/daemons/ipa-kdb/ipa_kdb_passwords.c b/daemons/ipa-kdb/ipa_kdb_passwords.c index 93e9e206081af412a472ab0c7624611a628a15b7..0bb7fa72496789241e27ecc852a1c6ede7f8e40a 100644 --- a/daemons/ipa-kdb/ipa_kdb_passwords.c +++ b/daemons/ipa-kdb/ipa_kdb_passwords.c @@ -203,6 +203,7 @@ krb5_error_code ipadb_change_pwd(krb5_context context, ret = asprintf(ied-pw_policy_dn, cn=global_policy,%s, ipactx-realm_base); if (ret == -1) { +free(ied); return ENOMEM; } db_entry-e_data = (krb5_octet *)ied; -- 1.7.6.4 From bd68245ff2ea1a1a039dda19bb04c308500ac57d Mon Sep 17 00:00:00 2001 From: Simo Sorce sso...@redhat.com Date: Thu, 3 Nov 2011 13:40:46 -0400 Subject: [PATCH 3/9] Fix CID 11021: Resource leak https://fedorahosted.org/freeipa/ticket/2037 --- util/ipa_pwd.c |4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) diff --git a/util/ipa_pwd.c b/util/ipa_pwd.c index c41617533ec25e8e656e9bb7a69d5b8b5dd8b5f7..fda6cb34ef24059362207325db61aedb62d7b665 100644 --- a/util/ipa_pwd.c +++ b/util/ipa_pwd.c @@ -560,7 +560,7 @@ int ipapwd_generate_new_history(char *password, unsigned char *hash = NULL; unsigned int hash_len; char *new_element; -char **ordered; +char **ordered = NULL; int c, i, n; int len; int ret; @@ -626,9 +626,11 @@ int ipapwd_generate_new_history(char *password, *new_pwd_history = ordered; *new_pwd_hlen = n; +ordered = NULL; ret = IPAPWD_POLICY_OK; done: +free(ordered); free(hash); return ret; } -- 1.7.6.4 From 44c5f0588cbce8ee38b14e17fe0a07fe33cdfb0f Mon Sep 17 00:00:00 2001 From: Simo Sorce sso...@redhat.com Date: Thu, 3 Nov 2011 13:45:06 -0400 Subject: [PATCH 4/9] Fix CID 11022: Resource leak https://fedorahosted.org/freeipa/ticket/2037 --- daemons/ipa-kdb/ipa_kdb_principals.c |7 +++ 1 files changed, 7 insertions(+), 0 deletions(-) diff --git a/daemons/ipa-kdb/ipa_kdb_principals.c b/daemons/ipa-kdb/ipa_kdb_principals.c index fdd834f355fd9e056058fa205b217e9e1f142e51..117eea86952ee4662930b80ba5e54c75aa587faf 100644 --- a/daemons/ipa-kdb/ipa_kdb_principals.c +++ b/daemons/ipa-kdb/ipa_kdb_principals.c @@ -1571,6 +1571,7 @@ static krb5_error_code ipadb_entry_to_mods(krb5_context kcontext, char **new_history; int nh_len; int ret; +int i; ied = (struct ipadb_e_data *)entry-e_data; if (ied-magic != IPA_E_DATA_MAGIC) { @@ -1619,6 +1620,12 @@ static krb5_error_code ipadb_entry_to_mods(krb5_context kcontext, kerr = ipadb_get_ldap_mod_str_list(imods, passwordHistory, new_history, nh_len, mod_op); + +for (i = 0; i nh_len; i++) { +free(new_history[i]); +} +free(new_history); + if (kerr) { goto done; } -- 1.7.6.4 From 5faf3e64d6d418be9daad1a2bb600817f29088dc Mon Sep 17 00:00:00 2001 From: Simo Sorce sso...@redhat.com Date: Thu, 3 Nov 2011 13:48:32 -0400 Subject: [PATCH 5/9] Fix CID 11023: Resource leak https://fedorahosted.org/freeipa/ticket/2037 --- daemons/ipa-kdb/ipa_kdb_principals.c |1 + 1 files changed, 1 insertions(+), 0