On Tue, 27 Jan 2015, Martin Babinsky wrote:
The attached patch is related to
https://fedorahosted.org/freeipa/ticket/4795 and fixes (hopefully)
some of the defects reported by subsequent scans.
NACK overall. If you want to provide fixes, make them separate of each
other and explain each fix. See below for details.
There are also 21 defects reported in asn1/asn1c/*.c files (http://cov01.lab.eng.brq.redhat.com/covscanhub/task/16545/log/freeipa-4.1.99.201501261541GIT871f9bb-0.fc21/scan-results.html).
Since this code is (semi)-automatically generated by asn1c software,
we should decide what to do with them.
Should I try to fix them by hand and/or report them upstream?
No manual fixes, we'll need to find out a way to fix them upstream.
Martin^3
From 4732626ed0fb8ec0fb2074c55955ab570eac58fa Mon Sep 17 00:00:00 2001
From: Martin Babinsky <mbabi...@redhat.com>
Date: Fri, 16 Jan 2015 15:43:17 +0100
Subject: [PATCH] Fixed issues reported in FreeIPA code by covscan
This patch is related to https://fedorahosted.org/freeipa/ticket/4795.
Performed another scan and fixed/waived some defects reported by Coverity in
IPA C code.
---
daemons/ipa-kdb/ipa_kdb_audit_as.c | 5 +++++
daemons/ipa-kdb/ipa_kdb_mspac.c | 5 +----
daemons/ipa-kdb/ipa_kdb_principals.c | 11 ++++-------
daemons/ipa-sam/ipa_sam.c | 2 +-
.../ipa-slapi-plugins/ipa-otp-lasttoken/ipa_otp_lasttoken.c | 8 ++++++--
daemons/ipa-slapi-plugins/ipa-pwd-extop/prepost.c | 4 +++-
daemons/ipa-slapi-plugins/ipa-uuid/ipa_uuid.c | 3 ++-
daemons/ipa-slapi-plugins/libotp/Makefile.am | 4 +++-
daemons/ipa-slapi-plugins/libotp/otp_config.c | 9 ++++++++-
util/ipa_krb5.h | 2 ++
10 files changed, 35 insertions(+), 18 deletions(-)
diff --git a/daemons/ipa-kdb/ipa_kdb_audit_as.c
b/daemons/ipa-kdb/ipa_kdb_audit_as.c
index
52c165442bde61d3ce88843b122aae7fe0fae50b..81ccbc2de28681c9c90b932fb14831965e0b246c
100644
--- a/daemons/ipa-kdb/ipa_kdb_audit_as.c
+++ b/daemons/ipa-kdb/ipa_kdb_audit_as.c
@@ -20,6 +20,7 @@
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
+#include <syslog.h>
#include "ipa_kdb.h"
#include "ipa_pwd.h"
NACK, why do you need syslog.h here?
@@ -120,7 +121,11 @@ void ipadb_audit_as_req(krb5_context kcontext,
client->last_failed = authtime;
client->mask |= KMASK_LAST_FAILED;
break;
+ /*coverity[dead_error_begin]*/
default:
+ krb5_klog_syslog(LOG_ERR,
+ "Got an unexpected value of error_code: %d\n",
+ error_code);
return;
}
diff --git a/daemons/ipa-kdb/ipa_kdb_mspac.c b/daemons/ipa-kdb/ipa_kdb_mspac.c
index
a4500070760e83994c8155a12ee6414b5ebee9e0..0f47d1f4bd536e24b9d46a35232ad558b33b4b26
100644
--- a/daemons/ipa-kdb/ipa_kdb_mspac.c
+++ b/daemons/ipa-kdb/ipa_kdb_mspac.c
@@ -54,9 +54,6 @@ struct ipadb_mspac {
time_t last_update;
};
-
-int krb5_klog_syslog(int, const char *, ...);
-
static char *user_pac_attrs[] = {
"objectClass",
"uid",
What does this fix have to do with coverity?
Please separate patches and submit one by one.
@@ -2074,7 +2071,7 @@ krb5_error_code ipadb_sign_authdata(krb5_context context,
}
}
- kerr = ipadb_get_pac(context, client_entry ? client_entry : client,
&pac);
+ kerr = ipadb_get_pac(context, client, &pac);
if (kerr != 0 && kerr != ENOENT) {
goto done;
}
NACK, this change is wrong and breaks conditional delegation.
diff --git a/daemons/ipa-kdb/ipa_kdb_principals.c
b/daemons/ipa-kdb/ipa_kdb_principals.c
index
e158c236eab5c7c5a7c12664dbde5d51cc55406d..760faeef224701c3dc4ee69d16df043e9c622d9a
100644
--- a/daemons/ipa-kdb/ipa_kdb_principals.c
+++ b/daemons/ipa-kdb/ipa_kdb_principals.c
@@ -1894,19 +1894,16 @@ static krb5_error_code
ipadb_modify_principal(krb5_context kcontext,
if (!ipactx) {
return KRB5_KDB_DBNOTINITED;
}
-
+ kerr = krb5_unparse_name(kcontext, entry->princ, &principal);
+ if (kerr != 0) {
+ goto done;
+ }
ied = (struct ipadb_e_data *)entry->e_data;
if (!ied || !ied->entry_dn) {
- kerr = krb5_unparse_name(kcontext, entry->princ, &principal);
- if (kerr != 0) {
- goto done;
- }
-
kerr = ipadb_fetch_principals(ipactx, 0, principal, &res);
if (kerr != 0) {
goto done;
}
-
/* FIXME: no alias allowed for now, should we allow modifies
* by alias name ? */
kerr = ipadb_find_principal(kcontext, 0, res, &principal, &lentry);
NACK, we only want to touch entry->princ and fetch it if it does not
exist. The cost of these operations is sufficient enough to postpone.
diff --git a/daemons/ipa-sam/ipa_sam.c b/daemons/ipa-sam/ipa_sam.c
index
07249fd27b362ed6499e372d651192dfc31b5173..ea9c18888503f40bfc288703d985530a66539b7d
100644
--- a/daemons/ipa-sam/ipa_sam.c
+++ b/daemons/ipa-sam/ipa_sam.c
@@ -1487,7 +1487,7 @@ static bool ldapgroup2displayentry(struct
ldap_search_state *state,
return false;
}
break;
-
+ /*coverity[dead_error_begin]*/
default:
DEBUG(0,("unknown group type: %d\n", group_type));
talloc_free(sid);
diff --git a/daemons/ipa-slapi-plugins/ipa-otp-lasttoken/ipa_otp_lasttoken.c
b/daemons/ipa-slapi-plugins/ipa-otp-lasttoken/ipa_otp_lasttoken.c
index
233813745795344f31a7dcf1931cf74a09f1e552..2990fba51452fcbe1c67572b0d1a64d5565e6eba
100644
--- a/daemons/ipa-slapi-plugins/ipa-otp-lasttoken/ipa_otp_lasttoken.c
+++ b/daemons/ipa-slapi-plugins/ipa-otp-lasttoken/ipa_otp_lasttoken.c
@@ -111,13 +111,17 @@ static bool is_pwd_enabled(const char *user_dn)
Slapi_Entry *entry = NULL;
uint32_t authtypes;
Slapi_DN *sdn;
+ int search_result = 0;
sdn = slapi_sdn_new_dn_byval(user_dn);
if (sdn == NULL)
return false;
- slapi_search_internal_get_entry(sdn, attrs, &entry,
- otp_config_plugin_id(otp_config));
+ search_result = slapi_search_internal_get_entry(sdn, attrs, &entry,
+ otp_config_plugin_id(otp_config));
+ if (search_result != LDAP_SUCCESS) {
+ LOG_TRACE("Entry not found. Error code: %d\n", search_result);
+ }
slapi_sdn_free(&sdn);
if (entry == NULL)
return false;
What does this fix have to do with Coverity?
diff --git a/daemons/ipa-slapi-plugins/ipa-pwd-extop/prepost.c
b/daemons/ipa-slapi-plugins/ipa-pwd-extop/prepost.c
index
84eff17013d2742d1b5e5c4ea5f4e22ee290d785..b28e2d220a41628277dbdce84dfdbc5952476190
100644
--- a/daemons/ipa-slapi-plugins/ipa-pwd-extop/prepost.c
+++ b/daemons/ipa-slapi-plugins/ipa-pwd-extop/prepost.c
@@ -634,7 +634,9 @@ static int ipapwd_pre_mod(Slapi_PBlock *pb)
is_krb = 0;
is_smb = 0;
is_ipant = 0;
-
+ /* coverity[fallthrough]
+ * After examining the output of covscan, we think that this
+ * fallthrough is intentional.*/
case LDAP_MOD_ADD:
if (!lmod->mod_bvalues ||
!lmod->mod_bvalues[0]) {
diff --git a/daemons/ipa-slapi-plugins/ipa-uuid/ipa_uuid.c
b/daemons/ipa-slapi-plugins/ipa-uuid/ipa_uuid.c
index
2b07de45b63dab36a0b7167e3583e88ebd07f6f7..061fd12521f072498ecc72858dfe79ba46624a51
100644
--- a/daemons/ipa-slapi-plugins/ipa-uuid/ipa_uuid.c
+++ b/daemons/ipa-slapi-plugins/ipa-uuid/ipa_uuid.c
@@ -1027,9 +1027,10 @@ static int ipauuid_pre_op(Slapi_PBlock *pb, int modtype)
slapi_mod_free(&next_mod);
break;
-
+ /*coverity[dead_error_begin]*/
default:
/* never reached, just silence compiler */
+ LOG_TRACE("Got unexpected value of modtype: %d\n", modtype);
break;
}
diff --git a/daemons/ipa-slapi-plugins/libotp/Makefile.am
b/daemons/ipa-slapi-plugins/libotp/Makefile.am
index
4428f6bdc38a4e4ec224d1fa70744d8381f7e0b1..71b9c19f40379ba6c61858984f9de0253020e00d
100644
--- a/daemons/ipa-slapi-plugins/libotp/Makefile.am
+++ b/daemons/ipa-slapi-plugins/libotp/Makefile.am
@@ -1,5 +1,7 @@
MAINTAINERCLEANFILES = *~ Makefile.in
-AM_CPPFLAGS = -I/usr/include/dirsrv
+PLUGIN_COMMON_DIR = ../common
+AM_CPPFLAGS = -I/usr/include/dirsrv \
+ -I$(PLUGIN_COMMON_DIR)
noinst_LTLIBRARIES = libhotp.la libotp.la
libhotp_la_SOURCES = hotp.c hotp.h
diff --git a/daemons/ipa-slapi-plugins/libotp/otp_config.c
b/daemons/ipa-slapi-plugins/libotp/otp_config.c
index
ac2cfc72aa9f72af8eb5b5c565650325ac8bf714..0d87ac0cdf35fd0d457ee5f2ee22d6cf4b1297cd
100644
--- a/daemons/ipa-slapi-plugins/libotp/otp_config.c
+++ b/daemons/ipa-slapi-plugins/libotp/otp_config.c
@@ -38,6 +38,7 @@
* END COPYRIGHT BLOCK **/
#include "otp_config.h"
+#include "util.h"
#include <pratom.h>
#include <plstr.h>
@@ -214,6 +215,7 @@ struct otp_config *otp_config_init(Slapi_ComponentId
*plugin_id)
struct otp_config *cfg = NULL;
void *node = NULL;
+ int search_result = 0;
cfg = (typeof(cfg)) slapi_ch_calloc(1, sizeof(*cfg));
cfg->plugin_id = plugin_id;
@@ -236,7 +238,12 @@ struct otp_config *otp_config_init(Slapi_ComponentId
*plugin_id)
cfg->records = rec;
/* Load the specified entry. */
- slapi_search_internal_get_entry(rec->sdn, NULL, &entry, plugin_id);
+ search_result = slapi_search_internal_get_entry(rec->sdn,
+ NULL, &entry, plugin_id);
+ if (search_result != LDAP_SUCCESS) {
+ LOG_TRACE("Entry not found. Error code: %d\n",
+ search_result);
+ }
update(cfg, rec->sdn, entry);
slapi_entry_free(entry);
}
What does this fix have to do with coverity?
diff --git a/util/ipa_krb5.h b/util/ipa_krb5.h
index
7b877aa665dd6cb4e0c1cf9d8153319cc8f61a20..2153bd57142d1468031d0aa4b5d3f59ef5c890b5
100644
--- a/util/ipa_krb5.h
+++ b/util/ipa_krb5.h
@@ -30,6 +30,8 @@ struct keys_container {
#define KEYTAB_RET_OID "2.16.840.1.113730.3.8.10.2"
#define KEYTAB_GET_OID "2.16.840.1.113730.3.8.10.5"
+int krb5_klog_syslog(int, const char *, ...);
+
void
ipa_krb5_free_ktypes(krb5_context context, krb5_enctype *val);
--
2.1.0
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
--
/ Alexander Bokovoy
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
