From 7a8b5ace5e4a2abdffdfaf737bab80ede4e18eaf Mon Sep 17 00:00:00 2001 From: Mark Reynolds Date: May 25 2018 16:26:55 +0000 Subject: Ticket 49675 - Fix coverity issues Description: Fixed these coverity issues. Some of these fixes are just to quiet convscan: 16852 Unsigned compared - entrycache_add_int 16848 Unsigned compared - dncache_add_int 16704 Explicit null dereferenced s- lapd_SSL_client_auth 15953 Resource leak - new_task 15583 Out-of-bounds read - create_filter 15445 Unused value - ruv_update_ruv 15442 Argument cannot be negative - dse_write_file_nolock 15223 Double unlock - ruv_get_referrals 15170 Explicit null dereferenced - passwd_apply_mods 15581 Wrong sizeof argument - slapi_be_new 15144 Constant expression result - upgradedn_producer Also fixed a few compiler warnings https://pagure.io/389-ds-base/issue/49675 Reviewed by: spichugi & lkrispenz(Thanks!!) --- diff --git a/ldap/servers/plugins/replication/repl5_ruv.c b/ldap/servers/plugins/replication/repl5_ruv.c index 7bfdc34..ddb88b9 100644 --- a/ldap/servers/plugins/replication/repl5_ruv.c +++ b/ldap/servers/plugins/replication/repl5_ruv.c @@ -73,7 +73,7 @@ static RUVElement *ruvAddIndexReplicaNoCSN(RUV *ruv, ReplicaId rid, const char * static int ruvReplicaCompare(const void *el1, const void *el2); static RUVElement *get_ruvelement_from_berval(const struct berval *bval); static char *get_replgen_from_berval(const struct berval *bval); - +static PRInt32 ruv_replica_count_nolock(const RUV *ruv, int lock); static const char *const prefix_replicageneration = "{replicageneration}"; static const char *const prefix_ruvcsn = "{replica "; /* intentionally missing '}' */ @@ -1366,22 +1366,30 @@ ruv_compare_ruv(const RUV *ruv1, const char *ruv1name, const RUV *ruv2, const ch return rc; } -PRInt32 -ruv_replica_count(const RUV *ruv) +static PRInt32 +ruv_replica_count_nolock(const RUV *ruv, int lock) { if (ruv == NULL) return 0; else { int count; - slapi_rwlock_rdlock(ruv->lock); + if (lock) + slapi_rwlock_rdlock(ruv->lock); count = dl_get_count(ruv->elements); - slapi_rwlock_unlock(ruv->lock); + if (lock) + slapi_rwlock_unlock(ruv->lock); return count; } } +PRInt32 +ruv_replica_count(const RUV *ruv) +{ + return ruv_replica_count_nolock(ruv, 1); +} + /* * Extract all the referral URL's from the RUV (but self URL), * returning them in an array of strings, that @@ -1406,7 +1414,7 @@ ruv_get_referrals(const RUV *ruv) slapi_rwlock_rdlock(ruv->lock); - n = ruv_replica_count(ruv); + n = ruv_replica_count_nolock(ruv, 0); if (n > 0) { RUVElement *replica; int cookie; @@ -1664,7 +1672,11 @@ ruv_update_ruv(RUV *ruv, const CSN *csn, const char *replica_purl, void *replica slapi_rwlock_wrlock(ruv->lock); if (local_rid != prim_rid) { repl_ruv = ruvGetReplica(ruv, prim_rid); - rc = ruv_update_ruv_element(ruv, repl_ruv, prim_csn, replica_purl, PR_FALSE); + 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; + } } repl_ruv = ruvGetReplica(ruv, local_rid); rc = ruv_update_ruv_element(ruv, repl_ruv, prim_csn, replica_purl, PR_TRUE); diff --git a/ldap/servers/plugins/uiduniq/uid.c b/ldap/servers/plugins/uiduniq/uid.c index 7f60955..3a75f1f 100644 --- a/ldap/servers/plugins/uiduniq/uid.c +++ b/ldap/servers/plugins/uiduniq/uid.c @@ -500,7 +500,7 @@ create_filter(const char **attributes, const struct berval *value, const char *r } /* Allocate the buffer */ - filter = slapi_ch_malloc(filterLen); + filter = (char *)slapi_ch_calloc(1, filterLen + 1); fp = filter; max = &filter[filterLen]; diff --git a/ldap/servers/slapd/back-ldbm/back-ldbm.h b/ldap/servers/slapd/back-ldbm/back-ldbm.h index ffc803e..79115fe 100644 --- a/ldap/servers/slapd/back-ldbm/back-ldbm.h +++ b/ldap/servers/slapd/back-ldbm/back-ldbm.h @@ -359,10 +359,10 @@ struct backdn /* for the in-core cache of entries */ struct cache { - uint64_t c_maxsize; /* max size in bytes */ + uint64_t c_maxsize; /* max size in bytes */ Slapi_Counter *c_cursize; /* size in bytes */ - uint64_t c_maxentries; /* max entries allowed (-1: no limit) */ - uint64_t c_curentries; /* current # entries in cache */ + int64_t c_maxentries; /* max entries allowed (-1: no limit) */ + uint64_t c_curentries; /* current # entries in cache */ Hashtable *c_dntable; Hashtable *c_idtable; #ifdef UUIDCACHE_ON diff --git a/ldap/servers/slapd/back-ldbm/cache.c b/ldap/servers/slapd/back-ldbm/cache.c index 77757c2..827cd91 100644 --- a/ldap/servers/slapd/back-ldbm/cache.c +++ b/ldap/servers/slapd/back-ldbm/cache.c @@ -494,7 +494,7 @@ cache_make_hashes(struct cache *cache, int type) /* initialize the cache */ int -cache_init(struct cache *cache, uint64_t maxsize, uint64_t maxentries, int type) +cache_init(struct cache *cache, uint64_t maxsize, int64_t maxentries, int type) { slapi_log_err(SLAPI_LOG_TRACE, "cache_init", "-->\n"); cache->c_maxsize = maxsize; @@ -703,7 +703,7 @@ entrycache_set_max_size(struct cache *cache, uint64_t bytes) } void -cache_set_max_entries(struct cache *cache, long entries) +cache_set_max_entries(struct cache *cache, int64_t entries) { struct backentry *eflush = NULL; struct backentry *eflushtemp = NULL; @@ -742,10 +742,10 @@ cache_get_max_size(struct cache *cache) return n; } -long +int64_t cache_get_max_entries(struct cache *cache) { - long n; + int64_t n; cache_lock(cache); n = cache->c_maxentries; @@ -773,7 +773,7 @@ cache_entry_size(struct backentry *e) * these u_long *'s to a struct */ void -cache_get_stats(struct cache *cache, PRUint64 *hits, PRUint64 *tries, uint64_t *nentries, uint64_t *maxentries, uint64_t *size, uint64_t *maxsize) +cache_get_stats(struct cache *cache, PRUint64 *hits, PRUint64 *tries, uint64_t *nentries, int64_t *maxentries, uint64_t *size, uint64_t *maxsize) { cache_lock(cache); if (hits) @@ -1431,7 +1431,7 @@ entrycache_add_int(struct cache *cache, struct backentry *e, int state, struct b /* don't add to lru since refcnt = 1 */ LOG("added entry of size %lu -> total now %lu out of max %lu\n", e->ep_size, slapi_counter_get_value(cache->c_cursize), cache->c_maxsize); - if (cache->c_maxentries >= 0) { + if (cache->c_maxentries > 0) { LOG(" total entries %ld out of %ld\n", cache->c_curentries, cache->c_maxentries); } @@ -1838,7 +1838,7 @@ dncache_add_int(struct cache *cache, struct backdn *bdn, int state, struct backd LOG("added entry of size %lu -> total now %lu out of max %lu\n", bdn->ep_size, slapi_counter_get_value(cache->c_cursize), cache->c_maxsize); - if (cache->c_maxentries >= 0) { + if (cache->c_maxentries > 0) { LOG(" total entries %ld out of %ld\n", cache->c_curentries, cache->c_maxentries); } diff --git a/ldap/servers/slapd/back-ldbm/import-threads.c b/ldap/servers/slapd/back-ldbm/import-threads.c index 1be15a4..7c2921c 100644 --- a/ldap/servers/slapd/back-ldbm/import-threads.c +++ b/ldap/servers/slapd/back-ldbm/import-threads.c @@ -1862,7 +1862,7 @@ upgradedn_producer(void *param) if (PL_strchr(rdnp, '\\')) { do_dn_norm = 1; } else { - while ((++rdnp <= endrdn) && (*rdnp == ' ') && (*rdnp == '\t')) + while ((++rdnp <= endrdn) && (*rdnp == ' ' || *rdnp == '\t')) ; /* DN contains an RDN ="" ? */ if ((rdnp != endrdn) && ('"' == *rdnp) && ('"' == *endrdn)) { diff --git a/ldap/servers/slapd/back-ldbm/monitor.c b/ldap/servers/slapd/back-ldbm/monitor.c index 91abeea..126715b 100644 --- a/ldap/servers/slapd/back-ldbm/monitor.c +++ b/ldap/servers/slapd/back-ldbm/monitor.c @@ -48,7 +48,8 @@ ldbm_back_monitor_instance_search(Slapi_PBlock *pb __attribute__((unused)), struct berval *vals[2]; char buf[BUFSIZ]; uint64_t hits, tries; - uint64_t nentries, maxentries; + uint64_t nentries; + int64_t maxentries; uint64_t size, maxsize; /* NPCTE fix for bugid 544365, esc 0. <04-Jul-2001> */ struct stat astat; @@ -100,7 +101,7 @@ ldbm_back_monitor_instance_search(Slapi_PBlock *pb __attribute__((unused)), MSET("maxEntryCacheSize"); sprintf(buf, "%" PRIu64, nentries); MSET("currentEntryCacheCount"); - sprintf(buf, "%" PRIu64, maxentries); + sprintf(buf, "%" PRId64, maxentries); MSET("maxEntryCacheCount"); if (entryrdn_get_switch()) { @@ -119,7 +120,7 @@ ldbm_back_monitor_instance_search(Slapi_PBlock *pb __attribute__((unused)), MSET("maxDnCacheSize"); sprintf(buf, "%" PRIu64, nentries); MSET("currentDnCacheCount"); - sprintf(buf, "%" PRIu64, maxentries); + sprintf(buf, "%" PRId64, maxentries); MSET("maxDnCacheCount"); } diff --git a/ldap/servers/slapd/back-ldbm/proto-back-ldbm.h b/ldap/servers/slapd/back-ldbm/proto-back-ldbm.h index 2898a35..793150b 100644 --- a/ldap/servers/slapd/back-ldbm/proto-back-ldbm.h +++ b/ldap/servers/slapd/back-ldbm/proto-back-ldbm.h @@ -32,14 +32,14 @@ void attr_create_empty(backend *be, char *type, struct attrinfo **ai); /* * cache.c */ -int cache_init(struct cache *cache, uint64_t maxsize, uint64_t maxentries, int type); +int cache_init(struct cache *cache, uint64_t maxsize, int64_t maxentries, int type); void cache_clear(struct cache *cache, int type); void cache_destroy_please(struct cache *cache, int type); void cache_set_max_size(struct cache *cache, uint64_t bytes, int type); void cache_set_max_entries(struct cache *cache, long entries); size_t cache_get_max_size(struct cache *cache); long cache_get_max_entries(struct cache *cache); -void cache_get_stats(struct cache *cache, uint64_t *hits, uint64_t *tries, uint64_t *entries, uint64_t *maxentries, uint64_t *size, uint64_t *maxsize); +void cache_get_stats(struct cache *cache, uint64_t *hits, uint64_t *tries, uint64_t *entries, int64_t *maxentries, uint64_t *size, uint64_t *maxsize); void cache_debug_hash(struct cache *cache, char **out); int cache_remove(struct cache *cache, void *e); void cache_return(struct cache *cache, void **bep); diff --git a/ldap/servers/slapd/backend_manager.c b/ldap/servers/slapd/backend_manager.c index 8ea433c..401ab5b 100644 --- a/ldap/servers/slapd/backend_manager.c +++ b/ldap/servers/slapd/backend_manager.c @@ -36,9 +36,13 @@ slapi_be_new(const char *type, const char *name, int isprivate, int logchanges) int oldsize = maxbackends; maxbackends += BACKEND_GRAB_SIZE; backends = (Slapi_Backend **)slapi_ch_realloc((char *)backends, maxbackends * sizeof(Slapi_Backend *)); - memset(&backends[oldsize], '\0', BACKEND_GRAB_SIZE * sizeof(Slapi_Backend *)); + for (i = oldsize; i < maxbackends; i++){ + /* init the new be pointers so we can track empty slots */ + backends[i] = NULL; + } } + /* Find the first open slot */ for (i = 0; ((i < maxbackends) && (backends[i])); i++) ; diff --git a/ldap/servers/slapd/dse.c b/ldap/servers/slapd/dse.c index f93398e..e61e9d9 100644 --- a/ldap/servers/slapd/dse.c +++ b/ldap/servers/slapd/dse.c @@ -1005,8 +1005,10 @@ dse_write_file_nolock(struct dse *pdse) * we need to open and fsync the dir to make the rename stick. */ int fp_configdir = open(pdse->dse_configdir, O_PATH | O_DIRECTORY); - fsync(fp_configdir); - close(fp_configdir); + if (fp_configdir != -1) { + fsync(fp_configdir); + close(fp_configdir); + } } } if (fpw.fpw_prfd) diff --git a/ldap/servers/slapd/passwd_extop.c b/ldap/servers/slapd/passwd_extop.c index 5f21f2f..df9fea7 100644 --- a/ldap/servers/slapd/passwd_extop.c +++ b/ldap/servers/slapd/passwd_extop.c @@ -146,29 +146,34 @@ passwd_apply_mods(Slapi_PBlock *pb_orig, const Slapi_DN *sdn, Slapi_Mods *mods, * that it was done by the root DN. */ Connection *pb_conn = NULL; slapi_pblock_get(pb_orig, SLAPI_CONNECTION, &pb_conn); - slapi_pblock_set(pb, SLAPI_CONNECTION, pb_conn); + if (pb_conn){ + slapi_pblock_set(pb, SLAPI_CONNECTION, pb_conn); + ret = slapi_modify_internal_pb(pb); - ret = slapi_modify_internal_pb(pb); + /* We now clean up the connection that we copied into the + * new pblock. We want to leave it untouched. */ + slapi_pblock_set(pb, SLAPI_CONNECTION, NULL); - /* We now clean up the connection that we copied into the - * new pblock. We want to leave it untouched. */ - slapi_pblock_set(pb, SLAPI_CONNECTION, NULL); + slapi_pblock_get(pb, SLAPI_PLUGIN_INTOP_RESULT, &ret); - slapi_pblock_get(pb, SLAPI_PLUGIN_INTOP_RESULT, &ret); - - /* Retrieve and duplicate the response controls since they will be - * destroyed along with the pblock used for the internal operation. */ - slapi_pblock_get(pb, SLAPI_RESCONTROLS, &pb_resp_controls); - if (pb_resp_controls) { - slapi_add_controls(resp_controls, pb_resp_controls, 1); - } + /* Retrieve and duplicate the response controls since they will be + * destroyed along with the pblock used for the internal operation. */ + slapi_pblock_get(pb, SLAPI_RESCONTROLS, &pb_resp_controls); + if (pb_resp_controls) { + slapi_add_controls(resp_controls, pb_resp_controls, 1); + } - if (ret != LDAP_SUCCESS) { - slapi_log_err(SLAPI_LOG_TRACE, "passwd_apply_mods", - "WARNING: passwordPolicy modify error %d on entry '%s'\n", - ret, slapi_sdn_get_dn(sdn)); + if (ret != LDAP_SUCCESS) { + slapi_log_err(SLAPI_LOG_TRACE, "passwd_apply_mods", + "WARNING: passwordPolicy modify error %d on entry '%s'\n", + ret, slapi_sdn_get_dn(sdn)); + } + } else { + ret = -1; + slapi_log_err(SLAPI_LOG_ERR, "passwd_apply_mods", + "(%s) Original connection is NULL\n", + slapi_sdn_get_dn(sdn)); } - slapi_pblock_destroy(pb); } diff --git a/ldap/servers/slapd/ssl.c b/ldap/servers/slapd/ssl.c index 36b09fd..288bae9 100644 --- a/ldap/servers/slapd/ssl.c +++ b/ldap/servers/slapd/ssl.c @@ -2448,7 +2448,7 @@ slapd_SSL_client_auth(LDAP *ld) /* Free config data */ - if (!svrcore_setup()) { + if (!svrcore_setup() && token != NULL) { #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."); diff --git a/ldap/servers/slapd/task.c b/ldap/servers/slapd/task.c index 23a9f9e..c839c1f 100644 --- a/ldap/servers/slapd/task.c +++ b/ldap/servers/slapd/task.c @@ -600,6 +600,7 @@ new_task(const char *rawdn, void *plugin) if (task->task_log_lock == NULL) { /* Failed to allocate! Uh Oh! */ slapi_ch_free((void **)&task); + slapi_ch_free_string(&dn); slapi_log_err(SLAPI_LOG_ERR, "new_task", "Unable to allocate task lock for: %s\n", rawdn); return NULL; } diff --git a/src/libsds/test/benchmark_par.c b/src/libsds/test/benchmark_par.c index 587f7c5..37ef763 100644 --- a/src/libsds/test/benchmark_par.c +++ b/src/libsds/test/benchmark_par.c @@ -115,7 +115,7 @@ batch_random(struct thread_info *info) size_t step = 0; size_t current_step = 0; size_t max_factors = info->iter / 2048; - size_t baseid; + size_t baseid = 0; void *output; /* Give ourselves a unique base id */ for (size_t i = 0; i < info->tid; i++) { @@ -188,7 +188,7 @@ batch_insert(struct thread_info *info) size_t cf = 0; size_t step = 0; size_t max_factors = info->iter / 2048; - size_t baseid; + size_t baseid = 0; /* Give ourselves a unique base id */ for (size_t i = 0; i < info->tid; i++) { baseid += 50000; @@ -215,7 +215,7 @@ batch_delete(struct thread_info *info) size_t cf = 0; size_t step = 0; size_t max_factors = info->iter / 2048; - size_t baseid; + size_t baseid = 0; /* Give ourselves a unique base id */ for (size_t i = 0; i < info->tid; i++) { baseid += 50000; diff --git a/src/svrcore/src/user.c b/src/svrcore/src/user.c index 3da668d..f3fd963 100644 --- a/src/svrcore/src/user.c +++ b/src/svrcore/src/user.c @@ -29,8 +29,6 @@ static const char retryWarning[] = "Warning: Incorrect PIN may result in disabling the token"; static const char prompt[] = "Enter PIN for"; -static const char nt_retryWarning[] = -"Warning: You entered an incorrect PIN. Incorrect PIN may result in disabling the token"; struct SVRCOREUserPinObj { @@ -122,14 +120,6 @@ static char *getPin(SVRCOREPinObj *obj, const char *tokenName, PRBool retry) /* If the program is not interactive then return no result */ if (!tty->interactive) return 0; -#ifdef _WIN32 - if (retry) { - MessageBox(GetDesktopWindow(), nt_retryWarning, - "Netscape Server", MB_ICONEXCLAMATION | MB_OK); - } - return NT_PromptForPin(tokenName); -#else - if (retry) fprintf(stdout, "%s\n", retryWarning); @@ -168,9 +158,6 @@ static char *getPin(SVRCOREPinObj *obj, const char *tokenName, PRBool retry) if (line[0] == 0) return 0; return strdup(line); - -#endif /* _WIN32 */ - } /*