From d45d8bd0f474dac181c58525c8a11dea0ff1a721 Mon Sep 17 00:00:00 2001 From: Simon Pichugin Date: May 12 2020 13:26:37 +0000 Subject: Issue 50610 - memory leaks in dbscan and changelog encryption 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: lkrispen (Thanks!) --- diff --git a/ldap/servers/plugins/replication/cl5_api.c b/ldap/servers/plugins/replication/cl5_api.c index a518983..d7e4749 100644 --- a/ldap/servers/plugins/replication/cl5_api.c +++ b/ldap/servers/plugins/replication/cl5_api.c @@ -497,6 +497,7 @@ cl5Close() _cl5Close(); s_cl5Desc.dbState = CL5_STATE_CLOSED; + rc = clcrypt_destroy(s_cl5Desc.clcrypt_handle); slapi_rwlock_unlock(s_cl5Desc.stLock); diff --git a/ldap/servers/plugins/replication/cl5_config.c b/ldap/servers/plugins/replication/cl5_config.c index 9187e2f..e0530be 100644 --- a/ldap/servers/plugins/replication/cl5_config.c +++ b/ldap/servers/plugins/replication/cl5_config.c @@ -131,6 +131,9 @@ changelog5_config_done(changelog5Config *config) /* 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); } } diff --git a/ldap/servers/plugins/replication/cl_crypt.c b/ldap/servers/plugins/replication/cl_crypt.c index 0cac088..d57075b 100644 --- a/ldap/servers/plugins/replication/cl_crypt.c +++ b/ldap/servers/plugins/replication/cl_crypt.c @@ -71,6 +71,49 @@ bail: /* * 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 * diff --git a/ldap/servers/plugins/replication/cl_crypt.h b/ldap/servers/plugins/replication/cl_crypt.h index f3ed069..9b69052 100644 --- a/ldap/servers/plugins/replication/cl_crypt.h +++ b/ldap/servers/plugins/replication/cl_crypt.h @@ -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_ */ diff --git a/ldap/servers/slapd/back-ldbm/db-bdb/bdb_layer.c b/ldap/servers/slapd/back-ldbm/db-bdb/bdb_layer.c index 51a2892..464f89f 100644 --- a/ldap/servers/slapd/back-ldbm/db-bdb/bdb_layer.c +++ b/ldap/servers/slapd/back-ldbm/db-bdb/bdb_layer.c @@ -6074,6 +6074,11 @@ bdb_back_ctrl(Slapi_Backend *be, int cmd, void *info) &(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, diff --git a/ldap/servers/slapd/back-ldbm/ldbm_attrcrypt.c b/ldap/servers/slapd/back-ldbm/ldbm_attrcrypt.c index b42ec9e..374cea0 100644 --- a/ldap/servers/slapd/back-ldbm/ldbm_attrcrypt.c +++ b/ldap/servers/slapd/back-ldbm/ldbm_attrcrypt.c @@ -1118,6 +1118,14 @@ bail: 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 diff --git a/ldap/servers/slapd/back-ldbm/proto-back-ldbm.h b/ldap/servers/slapd/back-ldbm/proto-back-ldbm.h index a07acee..5b277b9 100644 --- a/ldap/servers/slapd/back-ldbm/proto-back-ldbm.h +++ b/ldap/servers/slapd/back-ldbm/proto-back-ldbm.h @@ -610,6 +610,7 @@ int ldbm_instance_attrcrypt_config_delete_callback(Slapi_PBlock *pb, Slapi_Entry 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); diff --git a/ldap/servers/slapd/slapi-plugin.h b/ldap/servers/slapd/slapi-plugin.h index 0e38570..9889100 100644 --- a/ldap/servers/slapd/slapi-plugin.h +++ b/ldap/servers/slapd/slapi-plugin.h @@ -7743,6 +7743,7 @@ int slapi_back_set_info(Slapi_Backend *be, int cmd, void *info); * * \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 @@ enum 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 @@ struct _back_info_crypt_init }; 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 */ diff --git a/ldap/servers/slapd/tools/dbscan.c b/ldap/servers/slapd/tools/dbscan.c index f0fc548..32eea07 100644 --- a/ldap/servers/slapd/tools/dbscan.c +++ b/ldap/servers/slapd/tools/dbscan.c @@ -1133,7 +1133,7 @@ main(int argc, char **argv) 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 @@ main(int argc, char **argv) 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 @@ main(int argc, char **argv) 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 @@ main(int argc, char **argv) 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 @@ main(int argc, char **argv) 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 @@ main(int argc, char **argv) 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 @@ main(int argc, char **argv) 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 @@ main(int argc, char **argv) } } - 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; }