From 1b7198a025898b36e90f953dda157d9699fa43c9 Mon Sep 17 00:00:00 2001 From: Mark Reynolds Date: May 30 2018 17:13:59 +0000 Subject: Ticket 49675 - Revise coverity fix Description: Fix issues with last coverity patch: missing unlock, and a return code was needed. Also fixed issue 17472 (memory leak in uid.c) https://pagure.io/389-ds-base/issue/49675 Reviewed by: tbordaz & lkrispenz(Thanks!!) --- diff --git a/ldap/servers/plugins/memberof/memberof_config.c b/ldap/servers/plugins/memberof/memberof_config.c index 8a27f52..f081391 100644 --- a/ldap/servers/plugins/memberof/memberof_config.c +++ b/ldap/servers/plugins/memberof/memberof_config.c @@ -550,7 +550,7 @@ memberof_apply_config(Slapi_PBlock *pb __attribute__((unused)), } /* Build the new list */ - for (i = 0; theConfig.groupattrs && theConfig.groupattrs[i]; i++) { + for (i = 0; theConfig.group_slapiattrs && theConfig.groupattrs && theConfig.groupattrs[i]; i++) { theConfig.group_slapiattrs[i] = slapi_attr_new(); slapi_attr_init(theConfig.group_slapiattrs[i], theConfig.groupattrs[i]); } @@ -731,7 +731,7 @@ memberof_copy_config(MemberOfConfig *dest, MemberOfConfig *src) } /* Copy the attributes. */ - for (i = 0; src->group_slapiattrs && src->group_slapiattrs[i]; i++) { + for (i = 0; dest->group_slapiattrs && src->group_slapiattrs && src->group_slapiattrs[i]; i++) { dest->group_slapiattrs[i] = slapi_attr_dup(src->group_slapiattrs[i]); } diff --git a/ldap/servers/plugins/replication/repl5_ruv.c b/ldap/servers/plugins/replication/repl5_ruv.c index ddb88b9..d021f0f 100644 --- a/ldap/servers/plugins/replication/repl5_ruv.c +++ b/ldap/servers/plugins/replication/repl5_ruv.c @@ -1673,9 +1673,10 @@ ruv_update_ruv(RUV *ruv, const CSN *csn, const char *replica_purl, void *replica if (local_rid != prim_rid) { repl_ruv = ruvGetReplica(ruv, prim_rid); if ((rc = ruv_update_ruv_element(ruv, repl_ruv, prim_csn, replica_purl, PR_FALSE))) { - slapi_log_err(SLAPI_LOG_REPL, repl_plugin_name, - "ruv_update_ruv - failed to update primary ruv, error (%d)", rc); - return rc; + slapi_rwlock_unlock(ruv->lock); + slapi_log_err(SLAPI_LOG_REPL, repl_plugin_name, + "ruv_update_ruv - failed to update primary ruv, error (%d)", rc); + return rc; } } repl_ruv = ruvGetReplica(ruv, local_rid); diff --git a/ldap/servers/plugins/uiduniq/uid.c b/ldap/servers/plugins/uiduniq/uid.c index 3a75f1f..3ca5985 100644 --- a/ldap/servers/plugins/uiduniq/uid.c +++ b/ldap/servers/plugins/uiduniq/uid.c @@ -454,10 +454,10 @@ uid_op_error(int internal_error) static char * create_filter(const char **attributes, const struct berval *value, const char *requiredObjectClass) { - char *filter; + char *filter = NULL; char *fp; char *max; - int *attrLen; + int *attrLen = NULL; int totalAttrLen = 0; int attrCount = 0; int valueLen; @@ -487,9 +487,10 @@ create_filter(const char **attributes, const struct berval *value, const char *r totalAttrLen += 3; } - if (ldap_quote_filter_value(value->bv_val, - value->bv_len, 0, 0, &valueLen)) - return 0; + if (ldap_quote_filter_value(value->bv_val, value->bv_len, 0, 0, &valueLen)) { + slapi_ch_free((void **)&attrLen); + return filter; + } if (requiredObjectClass) { classLen = strlen(requiredObjectClass); @@ -523,9 +524,9 @@ create_filter(const char **attributes, const struct berval *value, const char *r *fp++ = '='; /* Place value in filter */ - if (ldap_quote_filter_value(value->bv_val, value->bv_len, - fp, max - fp, &valueLen)) { - slapi_ch_free((void **)&filter); + if (ldap_quote_filter_value(value->bv_val, value->bv_len, fp, max - fp, &valueLen)) { + slapi_ch_free_string(&filter); + slapi_ch_free((void **)&attrLen); return 0; } fp += valueLen; @@ -545,9 +546,8 @@ create_filter(const char **attributes, const struct berval *value, const char *r *fp++ = '='; /* Place value in filter */ - if (ldap_quote_filter_value(value->bv_val, value->bv_len, - fp, max - fp, &valueLen)) { - slapi_ch_free((void **)&filter); + if (ldap_quote_filter_value(value->bv_val, value->bv_len, fp, max - fp, &valueLen)) { + slapi_ch_free_string(&filter); slapi_ch_free((void **)&attrLen); return 0; } diff --git a/ldap/servers/slapd/back-ldbm/dblayer.c b/ldap/servers/slapd/back-ldbm/dblayer.c index 18dd944..4b8754d 100644 --- a/ldap/servers/slapd/back-ldbm/dblayer.c +++ b/ldap/servers/slapd/back-ldbm/dblayer.c @@ -5656,6 +5656,7 @@ dblayer_copy_directory(struct ldbminfo *li, inst_dir, MAXPATHLEN); if (!inst_dirp || !*inst_dirp) { slapi_log_err(SLAPI_LOG_ERR, "dblayer_copy_directory", "Instance dir is NULL.\n"); + slapi_ch_free_string(&inst_dirp); return return_value; } len = strlen(inst_dirp); @@ -5969,6 +5970,7 @@ dblayer_backup(struct ldbminfo *li, char *dest_dir, Slapi_Task *task) slapi_task_log_notice(task, "Backup: Instance dir is empty\n"); } + slapi_ch_free_string(&inst_dirp); return_value = -1; goto bail; } @@ -7090,6 +7092,7 @@ dblayer_in_import(ldbm_instance *inst) inst_dirp = dblayer_get_full_inst_dir(inst->inst_li, inst, inst_dir, MAXPATHLEN); if (!inst_dirp || !*inst_dirp) { + slapi_ch_free_string(&inst_dirp); rval = -1; goto done; } @@ -7141,6 +7144,7 @@ dblayer_update_db_ext(ldbm_instance *inst, char *oldext, char *newext) if (NULL == inst_dirp || '\0' == *inst_dirp) { slapi_log_err(SLAPI_LOG_ERR, "dblayer_update_db_ext", "Instance dir is NULL\n"); + slapi_ch_free_string(&inst_dirp); return -1; /* non zero */ } for (a = (struct attrinfo *)avl_getfirst(inst->inst_attrs); diff --git a/ldap/servers/slapd/back-ldbm/ldif2ldbm.c b/ldap/servers/slapd/back-ldbm/ldif2ldbm.c index 16b87ee..ab794a1 100644 --- a/ldap/servers/slapd/back-ldbm/ldif2ldbm.c +++ b/ldap/servers/slapd/back-ldbm/ldif2ldbm.c @@ -1639,7 +1639,7 @@ bye: dblayer_release_id2entry(be, db); - if (fd > STDERR_FILENO) { + if (fd >= 0) { close(fd); } diff --git a/ldap/servers/slapd/backend_manager.c b/ldap/servers/slapd/backend_manager.c index 401ab5b..d547505 100644 --- a/ldap/servers/slapd/backend_manager.c +++ b/ldap/servers/slapd/backend_manager.c @@ -42,18 +42,18 @@ slapi_be_new(const char *type, const char *name, int isprivate, int logchanges) } } - /* Find the first open slot */ - for (i = 0; ((i < maxbackends) && (backends[i])); i++) - ; - - PR_ASSERT(i < maxbackends); be = (Slapi_Backend *)slapi_ch_calloc(1, sizeof(Slapi_Backend)); be->be_lock = slapi_new_rwlock(); be_init(be, type, name, isprivate, logchanges, defsize, deftime); - backends[i] = be; - nbackends++; + for (size_t i = 0; i < maxbackends; i++) { + if (backends[i] == NULL) { + backends[i] = be; + nbackends++; + break; + } + } slapi_log_err(SLAPI_LOG_TRACE, "slapi_be_new", "Added new backend name [%s] type [%s] nbackends [%d]\n", diff --git a/ldap/servers/slapd/pw.c b/ldap/servers/slapd/pw.c index e2cd165..6b5684a 100644 --- a/ldap/servers/slapd/pw.c +++ b/ldap/servers/slapd/pw.c @@ -1164,6 +1164,7 @@ update_pw_history(Slapi_PBlock *pb, const Slapi_DN *sdn, char *old_pw) if (old_pw) { /* we have a password to replace with the oldest one in the history. */ if (!values_replace || !vacnt) { /* This is the first one to store */ + slapi_ch_array_free(values_replace); values_replace = (char **)slapi_ch_calloc(2, sizeof(char *)); } } else { diff --git a/ldap/servers/slapd/ssl.c b/ldap/servers/slapd/ssl.c index a36c6bd..0e6d15b 100644 --- a/ldap/servers/slapd/ssl.c +++ b/ldap/servers/slapd/ssl.c @@ -2376,7 +2376,7 @@ slapd_SSL_client_auth(LDAP *ld) /* Free config data */ - if (!svrcore_setup() && token != NULL) { + if (token && !svrcore_setup()) { #ifdef WITH_SYSTEMD slapd_SSL_warn("Sending pin request to SVRCore. You may need to run " "systemd-tty-ask-password-agent to provide the password."); @@ -2460,6 +2460,11 @@ slapd_SSL_client_auth(LDAP *ld) } } } + } else { + if (token == NULL) { + slapd_SSL_warn("slapd_SSL_client_auth - certificate token was not found\n"); + } + rc = -1; } slapi_ch_free_string(&token);