Browse Source

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!!)
Mark Reynolds 7 years ago
parent
commit
7a8b5ace5e

+ 19 - 7
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);

+ 1 - 1
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];
 

+ 3 - 3
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

+ 7 - 7
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);
         }

+ 1 - 1
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 <type>="<value>" ? */
                 if ((rdnp != endrdn) && ('"' == *rdnp) && ('"' == *endrdn)) {

+ 4 - 3
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. <P.R> <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");
     }
 

+ 2 - 2
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);

+ 5 - 1
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++)
         ;
 

+ 4 - 2
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)

+ 23 - 18
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);
     }
 

+ 1 - 1
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.");

+ 1 - 0
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;
     }

+ 3 - 3
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;

+ 0 - 13
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 */
-
 }
 
 /*