Re: [Freeipa-devel] [PATCHES] #2037 Coverity issues

2011-11-07 Thread Alexander Bokovoy
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

2011-11-07 Thread Simo Sorce
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

2011-11-03 Thread Simo Sorce
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