389-ds-base

Clone
Source Code
GIT

#51075 Issue 50610 - memory leak in dbscan and changelog encryption
Closed 3 years ago by spichugi. Opened 3 years ago by spichugi.
spichugi/389-ds-base i50610  into  master

file modified
+1 
@@ -497,6 +497,7 @@ 
 

      _cl5Close();
 

  

      s_cl5Desc.dbState = CL5_STATE_CLOSED;
 

+     rc = clcrypt_destroy(s_cl5Desc.clcrypt_handle);
 

  

      slapi_rwlock_unlock(s_cl5Desc.stLock);

file modified
+3 
@@ -131,6 +131,9 @@ 
 

          /* slapi_ch_free_string accepts NULL pointer */
 

          slapi_ch_free_string(&config->maxAge);
 

          slapi_ch_free_string(&config->dir);
 

+         slapi_ch_free_string(&config->symmetricKey);
 

+         slapi_ch_free_string(&config->dbconfig.encryptionAlgorithm);
 

+         slapi_ch_free_string(&config->dbconfig.symmetricKey);
 

      }
 

  }

file modified
+43 
@@ -71,6 +71,49 @@ 
 

  

  /*
 

   * return values:  0 - success
 

+  *              : -1 - error
 

+  *
 

+  * output value: out: non-NULL - cl encryption state private freed
 

+  *                  :     NULL - failure
 

+  */
 

+ int
 

+ clcrypt_destroy(void *clcrypt_handle)
 

+ {
 

+     int rc = -1;
 

+     char *cookie = NULL;
 

+     Slapi_Backend *be = NULL;
 

+     back_info_crypt_destroy crypt_destroy = {0};
 

+ 

+     slapi_log_err(SLAPI_LOG_TRACE, repl_plugin_name,
 

+                   "-> clcrypt_destroy\n");
 

+     if (NULL == clcrypt_handle) {
 

+         goto bail;
 

+     }
 

+     crypt_destroy.state_priv = clcrypt_handle;
 

+ 

+     be = slapi_get_first_backend(&cookie);
 

+     while (be) {
 

+         rc = slapi_back_ctrl_info(be, BACK_INFO_CRYPT_DESTROY,
 

+                                   (void *)&crypt_destroy);
 

+         if (LDAP_SUCCESS == rc) {
 

+             break; /* Successfully freed */
 

+         }
 

+         be = slapi_get_next_backend(cookie);
 

+     }
 

+     slapi_ch_free((void **)&cookie);
 

+     if (LDAP_SUCCESS == rc) {
 

+         rc = 0;
 

+     } else {
 

+         rc = -1;
 

+     }
 

+ bail:
 

+     slapi_log_err(SLAPI_LOG_TRACE, repl_plugin_name,
 

+                   "<- clcrypt_destroy (returning %d)\n", rc);
 

+     return rc;
 

+ }
 

+ 

+ /*
 

+  * return values:  0 - success
 

   *              :  1 - no encryption
 

   *              : -1 - error
 

   *

file modified
+1 
@@ -19,6 +19,7 @@ 
 

  #include "cert.h"
 

  

  int clcrypt_init(const CL5DBConfig *config, void **clcrypt_handle);
 

+ int clcrypt_destroy(void *clcrypt_handle);
 

  int clcrypt_encrypt_value(void *clcrypt_handle, struct berval *in, struct berval **out);
 

  int clcrypt_decrypt_value(void *state_priv, struct berval *in, struct berval **out);
 

  #endif /* _CLCRYPT_H_ */

file modified
+5 
@@ -6074,6 +6074,11 @@ 
 

                               &(crypt_init->state_priv));
 

          break;
 

      }
 

+     case BACK_INFO_CRYPT_DESTROY: {
 

+         back_info_crypt_destroy *crypt_init = (back_info_crypt_destroy *)info;
 

+         rc = back_crypt_destroy(crypt_init->state_priv);
 

+         break;
 

+     }
 

      case BACK_INFO_CRYPT_ENCRYPT_VALUE: {
 

          back_info_crypt_value *crypt_value = (back_info_crypt_value *)info;
 

          rc = back_crypt_encrypt_value(crypt_value->state_priv, crypt_value->in,

file modified
+8 
@@ -1118,6 +1118,14 @@ 
 

      return ret;
 

  }
 

  

+ int
 

+ back_crypt_destroy(void *handle)
 

+ {
 

+     attrcrypt_state_private *state_priv = (attrcrypt_state_private *)handle;
 

+     _back_crypt_cleanup_private(&state_priv);
 

+     return 0;
 

+ }
 

+ 

  /*
 

   * return values:  0 - success
 

   *              : -1 - error

file modified
+1 
@@ -610,6 +610,7 @@ 
 

  int ldbm_instance_attrcrypt_config_modify_callback(Slapi_PBlock *pb, Slapi_Entry *e, Slapi_Entry *entryAfter, int *returncode, char *returntext, void *arg);
 

  

  int back_crypt_init(Slapi_Backend *be, const char *dn, const char *encAlgorithm, void **handle);
 

+ int back_crypt_destroy(void *handle);
 

  int back_crypt_encrypt_value(void *handle, struct berval *in, struct berval **out);
 

  int
 

  back_crypt_decrypt_value(void *handle, struct berval *in, struct berval **out);

file modified
+8 
@@ -7743,6 +7743,7 @@ 
 

   *
 

   * \note Implemented cmd:
 

   * BACK_INFO_CRYPT_INIT - Initialize cipher (info: back_info_crypt_init)
 

+  * BACK_INFO_CRYPT_DESTROY - Free allocated during init data (info: back_info_crypt_destroy)
 

   * BACK_INFO_CRYPT_ENCRYPT_VALUE - Encrypt the given value (info: back_info_crypt_value)
 

   * BACK_INFO_CRYPT_DECRYPT_VALUE - Decrypt the given value (info: back_info_crypt_value)
 

   */
 
@@ -7756,6 +7757,7 @@ 
 

      BACK_INFO_INDEXPAGESIZE,       /* Get the index page size */
 

      BACK_INFO_DBENV_OPENFLAGS,     /* Get the dbenv openflags */
 

      BACK_INFO_CRYPT_INIT,          /* Ctrl: clcrypt_init */
 

+     BACK_INFO_CRYPT_DESTROY,       /* Ctrl: clcrypt_destroy */
 

      BACK_INFO_CRYPT_ENCRYPT_VALUE, /* Ctrl: clcrypt_encrypt_value */
 

      BACK_INFO_CRYPT_DECRYPT_VALUE, /* Ctrl: clcrypt_decrypt_value */
 

      BACK_INFO_DIRECTORY,           /* Get the directory path */
 
@@ -7782,6 +7784,12 @@ 
 

  };
 

  typedef struct _back_info_crypt_init back_info_crypt_init;
 

  

+ struct _back_info_crypt_destroy
 

+ {
 

+     void *state_priv;          /* a structure to free */
 

+ };
 

+ typedef struct _back_info_crypt_destroy back_info_crypt_destroy;
 

+ 

  struct _back_info_crypt_value
 

  {
 

      void *state_priv;   /* input */

file modified
+50 -37 
@@ -1133,7 +1133,7 @@ 
 

      DBC *cursor = NULL;
 

      char *filename = NULL;
 

      DBT key = {0}, data = {0};
 

-     int ret;
 

+     int ret = 0;
 

      char *find_key = NULL;
 

      uint32_t entry_id = 0xffffffff;
 

      int c;
 
@@ -1210,23 +1210,27 @@ 
 

      ret = db_env_create(&env, 0);
 

      if (ret != 0) {
 

          printf("Can't create dbenv: %s\n", db_strerror(ret));
 

-         exit(1);
 

+         ret = 1;
 

+         goto done;
 

      }
 

      ret = env->open(env, NULL, DB_CREATE | DB_INIT_MPOOL | DB_PRIVATE, 0);
 

      if (ret != 0) {
 

          printf("Can't open dbenv: %s\n", db_strerror(ret));
 

-         exit(1);
 

+         ret = 1;
 

+         goto done;
 

      }
 

  

      ret = db_create(&db, env, 0);
 

      if (ret != 0) {
 

          printf("Can't create db handle: %d\n", ret);
 

-         exit(1);
 

+         ret = 1;
 

+         goto done;
 

      }
 

      ret = db->open(db, NULL, filename, NULL, DB_UNKNOWN, DB_RDONLY, 0);
 

      if (ret != 0) {
 

          printf("Can't open db file '%s': %s\n", filename, db_strerror(ret));
 

-         exit(1);
 

+         ret = 1;
 

+         goto done;
 

      }
 

  

      /* cursor through the db */
 
@@ -1234,16 +1238,19 @@ 
 

      ret = db->cursor(db, NULL, &cursor, 0);
 

      if (ret != 0) {
 

          printf("Can't create db cursor: %s\n", db_strerror(ret));
 

-         exit(1);
 

+         ret = 1;
 

+         goto done;
 

      }
 

      ret = cursor->c_get(cursor, &key, &data, DB_FIRST);
 

      if (ret == DB_NOTFOUND) {
 

          printf("Empty database!\n");
 

-         exit(0);
 

+         ret = 0;
 

+         goto done;
 

      }
 

      if (ret != 0) {
 

          printf("Can't get first cursor: %s\n", db_strerror(ret));
 

-         exit(1);
 

+         ret = 1;
 

+         goto done;
 

      }
 

  

      if (find_key) {
 
@@ -1256,7 +1263,8 @@ 
 

              ret = db->get(db, NULL, &key, &data, 0);
 

              if (ret != 0) {
 

                  printf("Can't find key '%s'\n", find_key);
 

-                 exit(1);
 

+                 ret = 1;
 

+                 goto done;
 

              }
 

          }
 

          if (file_type & ENTRYRDNINDEXTYPE) {
 
@@ -1266,7 +1274,8 @@ 
 

              if (ret != 0) {
 

                  printf("Can't set cursor to returned item: %s\n",
 

                         db_strerror(ret));
 

-                 exit(1);
 

+                 ret = 1;
 

+                 goto done;
 

              }
 

              do {
 

                  display_item(cursor, &key, &data);
 
@@ -1282,7 +1291,8 @@ 
 

          if (ret != 0) {
 

              printf("Can't set cursor to returned item: %s\n",
 

                     db_strerror(ret));
 

-             exit(1);
 

+             ret = 1;
 

+             goto done;
 

          }
 

          display_item(cursor, &key, &data);
 

          key.size = 0;
 
@@ -1299,31 +1309,15 @@ 
 

                  ret = cursor->c_get(cursor, &key, &data, DB_NEXT);
 

                  if ((ret != 0) && (ret != DB_NOTFOUND)) {
 

                      printf("Bizarre error: %s\n", db_strerror(ret));
 

-                     exit(1);
 

+                     ret = 1;
 

+                     goto done;
 

                  }
 

              }
 

+             /* Success! Setting the return code to 0 */
 

+             ret = 0;
 

          }
 

      }
 

  

-     if (key.data) {
 

-         free(key.data);
 

-     }
 

-     if (data.data) {
 

-         free(data.data);
 

-     }
 

- 

-     ret = cursor->c_close(cursor);
 

-     if (ret != 0) {
 

-         printf("Can't close the cursor (?!): %s\n", db_strerror(ret));
 

-         exit(1);
 

-     }
 

- 

-     ret = db->close(db, 0);
 

-     if (ret != 0) {
 

-         printf("Unable to close db file: %s\n", db_strerror(ret));
 

-         exit(1);
 

-     }
 

- 

      if (display_mode & SHOWSUMMARY) {
 

  

          if (allids_cnt > 0) {
 
@@ -1359,11 +1353,30 @@ 
 

          }
 

      }
 

  

-     ret = env->close(env, 0);
 

-     if (ret != 0) {
 

-         printf("Unable to shutdown libdb: %s\n", db_strerror(ret));
 

-         exit(1);
 

+ done:
 

+     if (key.data) {
 

+         free(key.data);
 

      }
 

- 

-     return 0;
 

+     if (data.data) {
 

+         free(data.data);
 

+     }
 

+     if (cursor) {
 

+         if (cursor->c_close(cursor) != 0) {
 

+             printf("Can't close the cursor (?!): %s\n", db_strerror(1));
 

+             return 1;
 

+         }
 

+     }
 

+     if (db) {
 

+         if (db->close(db, 0) != 0) {
 

+             printf("Unable to close db file: %s\n", db_strerror(1));
 

+             return 1;
 

+         }
 

+     }
 

+     if (env) {
 

+         if (env->close(env, 0) != 0) {
 

+             printf("Unable to shutdown libdb: %s\n", db_strerror(1));
 

+             return 1;
 

+         }
 

+     }
 

+     return ret;
 

  }

Bug Description: More leaks are present that involve dbscan
execution (the issue happens on instance restart though).

Fix Description: dbscan - add 'done:' section to which we can
go to if something went worng and free the allocated data.

changelog encryption - add clcrypt_destroy function;
properly free the allocated memory when we go to shutdown.
When we do changelog5_config_done, additionally free
config->symmetricKey, config->dbconfig.encryptionAlgorithm,
and config->dbconfig.symmetricKey

https://pagure.io/389-ds-base/issue/50610

Reviewed by: ?

rebased onto 8c27c9027faec6d9b264d3eb6f0efef0c21c588c
3 years ago

It is not Initialize

looks good

rebased onto d45d8bd
3 years ago

Pull-Request has been merged by spichugi
3 years ago

389-ds-base is moving from Pagure to Github. This means that new issues and pull requests
will be accepted only in 389-ds-base's github repository.

This pull request has been cloned to Github as issue and is available here:
- https://github.com/389ds/389-ds-base/issues/4128

If you want to continue to work on the PR, please navigate to the github issue,
download the patch from the attachments and file a new pull request.

Thank you for understanding. We apologize for all inconvenience.

Pull-Request has been closed by spichugi
3 years ago
Metadata
None
No Tags
Changes Summary 9
+1 -0
file changed
ldap/servers/plugins/replication/cl5_api.c
+3 -0
file changed
ldap/servers/plugins/replication/cl5_config.c
+43 -0
file changed
ldap/servers/plugins/replication/cl_crypt.c
+1 -0
file changed
ldap/servers/plugins/replication/cl_crypt.h
+5 -0
file changed
ldap/servers/slapd/back-ldbm/db-bdb/bdb_layer.c
+8 -0
file changed
ldap/servers/slapd/back-ldbm/ldbm_attrcrypt.c
+1 -0
file changed
ldap/servers/slapd/back-ldbm/proto-back-ldbm.h
+8 -0
file changed
ldap/servers/slapd/slapi-plugin.h
+50 -37
file changed
ldap/servers/slapd/tools/dbscan.c
Powered by Pagure 5.13.3
Documentation • File an Issue • About • SSH Hostkey/Fingerprint
© Red Hat, Inc. and others.