Issue 6442 - Fix latest covscan memory leaks

Description: Fixed a majority of the memory leaks (except ACL leaks). Fixes: 389ds#6442 Reviewed by: progier(Thanks!)
tbordaz · Dec 18, 2024 · fcd1cb7 · fcd1cb7
1 parent 9683aa5
commit fcd1cb7
Show file tree

Hide file tree

Showing 38 changed files with 157 additions and 106 deletions.
diff --git a/dirsrvtests/tests/suites/basic/basic_test.py b/dirsrvtests/tests/suites/basic/basic_test.py
@@ -540,8 +540,7 @@ def test_basic_import_export(topology_st, import_example_ldif):
     time.sleep(1)
 
     # Good as place as any to quick test the task has some expected attributes
-    if ds_is_newer('1.4.1.2'):
-        assert import_task.present('nstaskcreated')
+    assert import_task.present('nstaskcreated')
     assert import_task.present('nstasklog')
     assert import_task.present('nstaskcurrentitem')
     assert import_task.present('nstasktotalitems')

diff --git a/ldap/servers/plugins/acctpolicy/acct_plugin.c b/ldap/servers/plugins/acctpolicy/acct_plugin.c
@@ -272,7 +272,6 @@ acct_update_login_history(const char *dn, char *timestr)
         }
         /* first time round */
     } else if (cfg->login_history_size > 0) {
-        login_hist = (char **)slapi_ch_calloc(2, sizeof(char *));
         /* alloc new array and append latest value */
         login_hist = (char **)slapi_ch_realloc((char *)login_hist, sizeof(char *) * (num_entries + 2));
         login_hist[num_entries] = slapi_ch_smprintf("%s", timestr);

diff --git a/ldap/servers/plugins/acl/aclgroup.c b/ldap/servers/plugins/acl/aclgroup.c
@@ -263,11 +263,13 @@ aclg_get_usersGroup(struct acl_pblock *aclpb, char *n_dn)
         return NULL;
     }
 
-    if (aclpb->aclpb_groupinfo)
-        return aclpb->aclpb_groupinfo;
-
     ACLG_LOCK_GROUPCACHE_WRITE();
 
+    if (aclpb->aclpb_groupinfo) {
+        ACLG_ULOCK_GROUPCACHE_WRITE();
+        return aclpb->aclpb_groupinfo;
+    }
+
     /* try it one more time. We might have one in the meantime */
     aclg_init_userGroup(aclpb, n_dn, 1 /* got the lock */);
     if (aclpb->aclpb_groupinfo) {
@@ -337,11 +339,12 @@ aclg_get_usersGroup(struct acl_pblock *aclpb, char *n_dn)
 
     aclUserGroups->aclg_num_userGroups++;
 
+    /* Now hang on to it */
+    aclpb->aclpb_groupinfo = u_group;
+
     /* Put it in the queue */
     ACLG_ULOCK_GROUPCACHE_WRITE();
 
-    /* Now hang on to it */
-    aclpb->aclpb_groupinfo = u_group;
     return u_group;
 }
 

diff --git a/ldap/servers/plugins/acl/acllas.c b/ldap/servers/plugins/acl/acllas.c
@@ -4308,6 +4308,8 @@ acllas_replace_attr_macro(char *rule, lasInfo *lasinfo)
              * list which replaces the old one.
              */
             l = acl_strstr(&str[0], ")");
+            /* coverity false positive: l + 2 will always be positive */
+            /* coverity[integer_overflow] */
             macro_str = slapi_ch_malloc(l + 2);
             strncpy(macro_str, &str[0], l + 1);
             macro_str[l + 1] = '\0';
@@ -4323,18 +4325,19 @@ acllas_replace_attr_macro(char *rule, lasInfo *lasinfo)
 
             str++; /* skip the . */
             l = acl_strstr(&str[0], ")");
-            if (l == -1){
+            if (l == -1) {
                 slapi_log_err(SLAPI_LOG_ERR, plugin_name,
                               "acllas_replace_attr_macro - Invalid macro str \"%s\".", str);
                 slapi_ch_free_string(&macro_str);
                 charray_free(working_list);
                 return NULL;
             }
+            /* coverity false positive: l + 1 will always be positive */
+            /* coverity[integer_overflow] */
             macro_attr_name = slapi_ch_malloc(l + 1);
             strncpy(macro_attr_name, &str[0], l);
             macro_attr_name[l] = '\0';
 
-
             slapi_entry_attr_find(e, macro_attr_name, &attr);
             if (NULL == attr) {
 

diff --git a/ldap/servers/plugins/chainingdb/cb_conn_stateless.c b/ldap/servers/plugins/chainingdb/cb_conn_stateless.c
@@ -943,10 +943,10 @@ cb_update_failed_conn_cpt(cb_backend_instance *cb)
 {
     /* if the chaining BE is already unavailable, we do nothing*/
     time_t now;
+
+    slapi_lock_mutex(cb->monitor_availability.cpt_lock);
     if (cb->monitor_availability.farmserver_state == FARMSERVER_AVAILABLE) {
-        slapi_lock_mutex(cb->monitor_availability.cpt_lock);
         cb->monitor_availability.cpt++;
-        slapi_unlock_mutex(cb->monitor_availability.cpt_lock);
         if (cb->monitor_availability.cpt >= CB_NUM_CONN_BEFORE_UNAVAILABILITY) {
             /* we reach the limit of authorized failed connections => we setup the chaining BE state to unavailable */
             now = slapi_current_rel_time_t();
@@ -958,21 +958,22 @@ cb_update_failed_conn_cpt(cb_backend_instance *cb)
                           "cb_update_failed_conn_cpt - Farm server unavailable");
         }
     }
+    slapi_unlock_mutex(cb->monitor_availability.cpt_lock);
 }
 
 void
 cb_reset_conn_cpt(cb_backend_instance *cb)
 {
+    slapi_lock_mutex(cb->monitor_availability.cpt_lock);
     if (cb->monitor_availability.cpt > 0) {
-        slapi_lock_mutex(cb->monitor_availability.cpt_lock);
         cb->monitor_availability.cpt = 0;
         if (cb->monitor_availability.farmserver_state == FARMSERVER_UNAVAILABLE) {
             cb->monitor_availability.farmserver_state = FARMSERVER_AVAILABLE;
             slapi_log_err(SLAPI_LOG_PLUGIN, CB_PLUGIN_SUBSYSTEM,
                           "cb_reset_conn_cpt - Farm server is back");
         }
-        slapi_unlock_mutex(cb->monitor_availability.cpt_lock);
     }
+    slapi_unlock_mutex(cb->monitor_availability.cpt_lock);
 }
 
 int

diff --git a/ldap/servers/plugins/memberof/memberof.c b/ldap/servers/plugins/memberof/memberof.c
@@ -887,6 +887,7 @@ perform_needed_fixup(void)
     if (config.memberof_attr == NULL) {
         slapi_log_err(SLAPI_LOG_ALERT, MEMBEROF_PLUGIN_SUBSYSTEM,
                       "Failed to perform memberof fixup task: The memberof attribute is not configured.\n");
+        memberof_free_config(&config);
         return -1;
     }
     slapi_log_err(SLAPI_LOG_INFO, MEMBEROF_PLUGIN_SUBSYSTEM,
@@ -897,6 +898,7 @@ perform_needed_fixup(void)
         slapi_log_err(SLAPI_LOG_ALERT, MEMBEROF_PLUGIN_SUBSYSTEM,
                       "Failed to perform memberof fixup task because no objectclass contains the %s attribute.\n",
                       config.memberof_attr);
+        memberof_free_config(&config);
         return -1;
     }
     filter_size = 4; /* For "(|...)\0" */
@@ -930,6 +932,7 @@ perform_needed_fixup(void)
     slapi_ch_free_string(&cookie);
     slapi_ch_free_string(&td.bind_dn);
     slapi_ch_free_string(&td.filter_str);
+    memberof_free_config(&config);
     slapi_log_err(SLAPI_LOG_INFO, MEMBEROF_PLUGIN_SUBSYSTEM,
                   "Memberof plugin finished the global fixup task for attribute %s\n", config.memberof_attr);
     return rc;

diff --git a/ldap/servers/plugins/replication/cl5_api.c b/ldap/servers/plugins/replication/cl5_api.c
@@ -558,7 +558,15 @@ cl5ImportLDIF(const char *clDir, const char *ldifFile, Replica *replica)
 
     /* Set changelog state to import */
     pthread_mutex_lock(&(cldb->stLock));
+
+    if (cldb->dbState == CL5_STATE_IMPORT) {
+        pthread_mutex_unlock(&(cldb->stLock));
+        slapi_log_err(SLAPI_LOG_ERR, repl_plugin_name_cl,
+                      "cl5ImportLDIF - changelog import already in progress\n");
+        return CL5_IGNORE_OP;
+    }
     cldb->dbState = CL5_STATE_IMPORT;
+
     pthread_mutex_unlock(&(cldb->stLock));
 
     /* Wait for all the threads to stop */
@@ -1363,6 +1371,7 @@ cldb_SetReplicaDB(Replica *replica, void *arg)
     if (rc != CL5_SUCCESS) {
         slapi_log_err(SLAPI_LOG_ERR, repl_plugin_name_cl,
                       "cldb_SetReplicaDB - failed to configure changelog trimming\n");
+        changelog5_config_done(&config);
         return CL5_BAD_DATA;
     }
 
@@ -1578,7 +1587,12 @@ _cl5Entry2DBData(const CL5Entry *entry, char **data, PRUint32 *len, void *clcryp
     /* write change type */
     (*pos) = (unsigned char)op->operation_type;
     pos++;
-    /* write time */
+    /*
+     * write time
+     * --> Y2K38 - will hopefully be fixed by a future C standard htonll()
+     * function. Then we can move "t" and "entry->time" to be uint64_t
+     * to fix the issue.
+     */
     t = PR_htonl((PRUint32)entry->time);
     memcpy(pos, &t, sizeof(t));
     pos += sizeof(t);

diff --git a/ldap/servers/plugins/replication/cl5_init.c b/ldap/servers/plugins/replication/cl5_init.c
@@ -42,7 +42,7 @@ changelog5_upgrade(void)
 
     if (config.dir == NULL) {
         /* we do not have a valid legacy config, nothing to upgrade */
-        return rc;
+        goto done;
     }
 
     replica_enumerate_replicas(_cl5_upgrade_replica, (void *)&config);
@@ -51,6 +51,7 @@ changelog5_upgrade(void)
 
     rc |= _cl5_upgrade_removeconfig();
 
+done:
     changelog5_config_done(&config);
 
     return rc;
@@ -122,7 +123,7 @@ _cl5_upgrade_replica_config(Replica *replica, changelog5Config *config)
         slapi_entry_add_string(config_entry, CONFIG_CHANGELOG_TRIM_ATTRIBUTE, gen_duration(config->trimInterval));
     }
 
-    /* if changelog encryption is enabled then in the upgrade mode all backends will have 
+    /* if changelog encryption is enabled then in the upgrade mode all backends will have
      * an encrypted changelog, store the encryption attrs */
     if (config->encryptionAlgorithm) {
         slapi_entry_add_string(config_entry, CONFIG_CHANGELOG_ENCRYPTION_ALGORITHM, config->encryptionAlgorithm);

diff --git a/ldap/servers/plugins/replication/repl5_connection.c b/ldap/servers/plugins/replication/repl5_connection.c
@@ -679,8 +679,8 @@ check_flow_control_tot_init(Repl_Connection *conn, int optype, const char *extop
 static ConnResult
 conn_is_available(Repl_Connection *conn)
 {
-    time_t poll_timeout_sec = 1;   /* Polling for 1sec */
-    time_t yield_delay_msec = 100; /* Delay to wait */
+    int32_t poll_timeout_sec = 1;   /* Polling for 1sec */
+    int32_t yield_delay_msec = 100; /* Delay to wait */
     time_t start_time = slapi_current_rel_time_t();
     time_t time_now;
     ConnResult return_value = CONN_OPERATION_SUCCESS;

diff --git a/ldap/servers/plugins/replication/repl5_replica.c b/ldap/servers/plugins/replication/repl5_replica.c
@@ -35,7 +35,7 @@ struct replica
     ReplicaUpdateDNList updatedn_list; /* list of dns with which a supplier should bind to update this replica */
     Slapi_ValueSet *updatedn_groups;   /* set of groups whose memebers are allowed to update replica */
     ReplicaUpdateDNList groupdn_list;  /* exploded listof dns from update group */
-    uint32_t updatedn_group_last_check;    /* the time of the last group check */
+    time_t updatedn_group_last_check;  /* the time of the last group check */
     int64_t updatedn_group_check_interval; /* the group check interval */
     ReplicaType repl_type;             /* is this replica read-only ? */
     ReplicaId repl_rid;                /* replicaID */
@@ -219,7 +219,7 @@ replica_new_from_entry(Slapi_Entry *e, char *errortext, PRBool is_add_operation,
          * during replica initialization
          */
         rc = _replica_update_entry(r, e, errortext);
-        /* add changelog config entry to config 
+        /* add changelog config entry to config
          * this is only needed for replicas logging changes,
          * but for now let it exist for all replicas. Makes handling
          * of changing replica flags easier
@@ -2544,7 +2544,7 @@ _replica_get_config_dn(const Slapi_DN *root)
     return dn;
 }
 /* when a replica is added the changelog config entry is created
- * it will only the container entry, specifications for trimming 
+ * it will only the container entry, specifications for trimming
  * or encyrption need to be added separately
  */
 static int
@@ -2876,6 +2876,7 @@ replica_update_state(time_t when __attribute__((unused)), void *arg)
                       "replica_update_state - Failed to get the config dn for %s\n",
                       slapi_sdn_get_dn(r->repl_root));
         replica_unlock(r->repl_lock);
+        slapi_mod_done(&smod);
         return;
     }
     pb = slapi_pblock_new();

diff --git a/ldap/servers/plugins/replication/repl5_tot_protocol.c b/ldap/servers/plugins/replication/repl5_tot_protocol.c
@@ -43,7 +43,7 @@ typedef struct callback_data
     Private_Repl_Protocol *prp;
     int rc;
     unsigned long num_entries;
-    time_t sleep_on_busy;
+    uint32_t sleep_on_busy;
     time_t last_busy;
     pthread_mutex_t lock;                    /* Lock to protect access to this structure, the message id list and to force memory barriers */
     PRThread *result_tid;                    /* The async result thread */
@@ -481,7 +481,7 @@ repl5_tot_run(Private_Repl_Protocol *prp)
         cb_data.prp = prp;
         cb_data.rc = 0;
         cb_data.num_entries = 1UL;
-        cb_data.sleep_on_busy = 0UL;
+        cb_data.sleep_on_busy = 0;
         cb_data.last_busy = slapi_current_rel_time_t();
         cb_data.flowcontrol_detection = 0;
         pthread_mutex_init(&(cb_data.lock), NULL);
@@ -540,7 +540,7 @@ repl5_tot_run(Private_Repl_Protocol *prp)
         cb_data.prp = prp;
         cb_data.rc = 0;
         cb_data.num_entries = 0UL;
-        cb_data.sleep_on_busy = 0UL;
+        cb_data.sleep_on_busy = 0;
         cb_data.last_busy = slapi_current_rel_time_t();
         cb_data.flowcontrol_detection = 0;
         pthread_mutex_init(&(cb_data.lock), NULL);
@@ -803,7 +803,7 @@ send_entry(Slapi_Entry *e, void *cb_data)
     BerElement *bere;
     struct berval *bv;
     unsigned long *num_entriesp;
-    time_t *sleep_on_busyp;
+    uint32_t *sleep_on_busyp;
     time_t *last_busyp;
     int message_id = 0;
     int retval = 0;
@@ -899,7 +899,7 @@ send_entry(Slapi_Entry *e, void *cb_data)
             *last_busyp = now;
 
             slapi_log_err(SLAPI_LOG_ERR, repl_plugin_name,
-                          "send_entry - Replica \"%s\" is busy. Waiting %lds while"
+                          "send_entry - Replica \"%s\" is busy. Waiting %ds while"
                           " it finishes processing its current import queue\n",
                           agmt_get_long_name(prp->agmt), *sleep_on_busyp);
             DS_Sleep(PR_SecondsToInterval(*sleep_on_busyp));

diff --git a/ldap/servers/plugins/replication/repl_cleanallruv.c b/ldap/servers/plugins/replication/repl_cleanallruv.c
@@ -2174,19 +2174,26 @@ replica_cleanallruv_thread(void *arg)
                  "Waiting to process all the updates from the deleted replica...");
     ruv_obj = replica_get_ruv(data->replica);
     ruv = object_get_data(ruv_obj);
-    while (data->maxcsn && !is_task_aborted(data->rid) && !is_cleaned_rid(data->rid) && !slapi_is_shutting_down()) {
-        struct timespec current_time = {0};
-        if (csn_get_replicaid(data->maxcsn) == 0 ||
-            ruv_covers_csn_cleanallruv(ruv, data->maxcsn) ||
-            strcasecmp(data->force, "yes") == 0) {
-            /* We are caught up, now we can clean the ruv's */
-            break;
+    if (data->maxcsn) {
+        while (!is_task_aborted(data->rid) &&
+               !is_cleaned_rid(data->rid) &&
+               !slapi_is_shutting_down())
+        {
+            struct timespec current_time = {0};
+
+            if (csn_get_replicaid(data->maxcsn) == 0 ||
+                ruv_covers_csn_cleanallruv(ruv, data->maxcsn) ||
+                strcasecmp(data->force, "yes") == 0)
+            {
+                /* We are caught up, now we can clean the ruv's */
+                break;
+            }
+            clock_gettime(CLOCK_MONOTONIC, &current_time);
+            current_time.tv_sec += CLEANALLRUV_SLEEP;
+            pthread_mutex_lock(&notify_lock);
+            pthread_cond_timedwait(&notify_cvar, &notify_lock, &current_time);
+            pthread_mutex_unlock(&notify_lock);
         }
-        clock_gettime(CLOCK_MONOTONIC, &current_time);
-        current_time.tv_sec += CLEANALLRUV_SLEEP;
-        pthread_mutex_lock(&notify_lock);
-        pthread_cond_timedwait(&notify_cvar, &notify_lock, &current_time);
-        pthread_mutex_unlock(&notify_lock);
     }
     object_release(ruv_obj);
     /*

diff --git a/ldap/servers/plugins/replication/repl_controls.c b/ldap/servers/plugins/replication/repl_controls.c
@@ -161,7 +161,7 @@ decode_NSDS50ReplUpdateInfoControl(LDAPControl **controlsp,
     struct berval superior_uuid_val = {0};
     struct berval csn_val = {0};
     BerElement *tmp_bere = NULL;
-    Slapi_Mods modrdn_smods;
+    Slapi_Mods modrdn_smods = {0};
     PRBool got_modrdn_mods = PR_FALSE;
     ber_len_t len;
 
@@ -232,11 +232,13 @@ decode_NSDS50ReplUpdateInfoControl(LDAPControl **controlsp,
 
         if (NULL != modrdn_mods && got_modrdn_mods) {
             *modrdn_mods = slapi_mods_get_ldapmods_passout(&modrdn_smods);
+        } else {
+            slapi_mods_done(&modrdn_smods);
         }
-        slapi_mods_done(&modrdn_smods);
 
         rc = 1;
     } else {
+        /* Control not present */
         rc = 0;
     }
 loser:
@@ -258,6 +260,10 @@ decode_NSDS50ReplUpdateInfoControl(LDAPControl **controlsp,
         ldap_memfree(csn_val.bv_val);
         csn_val.bv_val = NULL;
     }
+    if (rc < 1) {
+        /* no control or error, free smods */
+        slapi_mods_done(&modrdn_smods);
+    }
     return rc;
 }
 

diff --git a/ldap/servers/plugins/retrocl/retrocl_trim.c b/ldap/servers/plugins/retrocl/retrocl_trim.c
@@ -239,7 +239,7 @@ trim_changelog(void)
     time_t now_maxage; /* used for checking if the changelog entry can be trimmed */
     changeNumber first_in_log = 0, last_in_log = 0;
     int num_deleted = 0;
-    int max_age, last_trim, trim_interval;
+    time_t max_age, last_trim, trim_interval;
 
     now_interval = slapi_current_rel_time_t(); /* monotonic time for interval */
 
@@ -297,7 +297,7 @@ trim_changelog(void)
             }
         }
     } else {
-        slapi_log_err(SLAPI_LOG_PLUGIN, RETROCL_PLUGIN_NAME, "Not yet time to trim: %ld < (%d+%d)\n",
+        slapi_log_err(SLAPI_LOG_PLUGIN, RETROCL_PLUGIN_NAME, "Not yet time to trim: %ld < (%ld+%ld)\n",
                       now_interval, last_trim, trim_interval);
     }
     PR_Lock(ts.ts_s_trim_mutex);