#50340 structs for diabled plugins will not be freed
Opened 5 months ago by lkrispen. Modified 4 months ago

Asan shows leaks like this:

 Indirect leak of 239 byte(s) in 12 object(s) allocated from:
     #0 0x7fc5b6ab9c48 in malloc (/lib64/libasan.so.5+0xeec48)
     #1 0x7fc5b62b11f9 in slapi_ch_malloc /home/lkrispen/TEST/caa/ws/389-ds-base/ldap/servers/slapd/ch_malloc.c:95
     #2 0x7fc5b62f1d45 in slapi_entry_attr_get_charptr /home/lkrispen/TEST/caa/ws/389-ds-base/ldap/servers/slapd/entry.c:2751
     #3 0x7fc5b63c81be in plugin_setup /home/lkrispen/TEST/caa/ws/389-ds-base/ldap/servers/slapd/plugin.c:2896
     #4 0x7fc5b62b78f6 in load_plugin_entry /home/lkrispen/TEST/caa/ws/389-ds-base/ldap/servers/slapd/configdse.c:352
     #5 0x7fc5b62e41a5 in dse_call_callback /home/lkrispen/TEST/caa/ws/389-ds-base/ldap/servers/slapd/dse.c:2642
     #6 0x7fc5b62d8e65 in dse_read_one_file /home/lkrispen/TEST/caa/ws/389-ds-base/ldap/servers/slapd/dse.c:758
     #7 0x7fc5b62d95cf in dse_read_file /home/lkrispen/TEST/caa/ws/389-ds-base/ldap/servers/slapd/dse.c:829
     #8 0x440872 in init_dse_file /home/lkrispen/TEST/caa/ws/389-ds-base/ldap/servers/slapd/fedse.c:2556
     #9 0x442297 in setup_internal_backends /home/lkrispen/TEST/caa/ws/389-ds-base/ldap/servers/slapd/fedse.c:2796
     #10 0x447608 in main /home/lkrispen/TEST/caa/ws/389-ds-base/ldap/servers/slapd/main.c:858
     #11 0x7fc5b2d2e18a in __libc_start_main (/lib64/libc.so.6+0x2318a)

This affects plugins which are not enabled when read from dse.ldif. Enabled plugins are added to a plugin list and freed at shutdown

This is not a big deal from a memory usgae view, but makes it more difficult to analyze asan reports


IIRC I attempted to have these freed but found some circular memory issue (I think?). Anyway, ASAN has a way to "ignore" known warnings at run time, see "suppressions" in https://github.com/google/sanitizers/wiki/AddressSanitizerLeakSanitizer or "turning off instrumentation" here https://github.com/google/sanitizers/wiki/AddressSanitizer

Metadata Update from @firstyear:
- Custom field origin adjusted to None
- Custom field reviewstatus adjusted to None

5 months ago

well, I prefer to fix these leaks to supressing the report. The fix is committed already and if we find issues we will address them.
Or do you recall what the circular memory issues were ?

I honestly do not remember what they were, which is why I was asking for this to be tested with ASAN - if that's all clear, then I have no concerns :)

Metadata Update from @mreynolds:
- Issue set to the milestone: 1.4.1

4 months ago

Yes as @firstyear was concerned this fix causes a crash when doing a db2ldif -r. The plugin is freed because its not enabled in offline mode, and then we crash trying to compare the plugin name:

Program received signal SIGSEGV, Segmentation fault.
strcmpi_fast (src=0x5555555b1b28 "Multimaster Replication Plugin", dst=0x1 <error: Cannot access memory at address 0x1>) at ldap/servers/slapd/intrinsics.h:30
30          if (((f = (unsigned char)(*(dst++))) >= 'A') && (f <= 'Z'))
Missing separate debuginfos, use: dnf debuginfo-install audit-libs-2.8.5-2.fc28.x86_64 cracklib-2.9.6-13.fc28.x86_64 cyrus-sasl-gs2-2.1.27-0.2rc7.fc28.x86_64 cyrus-sasl-gssapi-2.1.27-0.2rc7.fc28.x86_64 cyrus-sasl-ldap-2.1.27-0.2rc7.fc28.x86_64 cyrus-sasl-lib-2.1.27-0.2rc7.fc28.x86_64 cyrus-sasl-md5-2.1.27-0.2rc7.fc28.x86_64 cyrus-sasl-ntlm-2.1.27-0.2rc7.fc28.x86_64 cyrus-sasl-plain-2.1.27-0.2rc7.fc28.x86_64 cyrus-sasl-sql-2.1.27-0.2rc7.fc28.x86_64 gssproxy-0.8.0-4.fc28.x86_64 keyutils-libs-1.5.10-6.fc28.x86_64 krb5-libs-1.16.1-25.fc28.x86_64 libblkid-2.32.1-1.fc28.x86_64 libcap-2.25-9.fc28.x86_64 libcap-ng-0.7.9-4.fc28.x86_64 libcom_err-1.44.2-0.fc28.x86_64 libdb-5.3.28-30.fc28.x86_64 libevent-2.1.8-2.fc28.x86_64 libgcc-8.3.1-2.fc28.x86_64 libgcrypt-1.8.4-1.fc28.x86_64 libgpg-error-1.33-1.fc28.x86_64 libicu-60.2-2.fc28.x86_64 libmount-2.32.1-1.fc28.x86_64 libselinux-2.8-1.fc28.x86_64 libstdc++-8.3.1-2.fc28.x86_64 libuuid-2.32.1-1.fc28.x86_64 libxcrypt-4.4.4-2.fc28.x86_64 lz4-libs-1.8.1.2-4.fc28.x86_64 mariadb-connector-c-3.0.9-2.fc28.x86_64 nspr-4.21.0-1.fc28.x86_64 nss-3.43.0-1.fc28.x86_64 nss-softokn-3.43.0-1.fc28.x86_64 nss-softokn-freebl-3.43.0-1.fc28.x86_64 nss-util-3.43.0-1.fc28.x86_64 openldap-2.4.46-5.fc28.x86_64 openssl-libs-1.1.0i-1.fc28.x86_64 pam-1.3.1-15.fc28.x86_64 pcre-8.43-1.fc28.x86_64 pcre2-10.32-8.fc28.x86_64 postgresql-libs-10.7-1.fc28.x86_64 sqlite-libs-3.22.0-5.fc28.x86_64 sssd-client-1.16.3-2.fc28.x86_64 systemd-libs-238-12.git07f8cd5.fc28.x86_64 xz-libs-5.2.4-2.fc28.x86_64 zlib-1.2.11-8.fc28.x86_64
(gdb) where
#0  0x00007ffff792e412 in strcmpi_fast (src=0x5555555b1b28 "Multimaster Replication Plugin", dst=0x1 <error: Cannot access memory at address 0x1>)
    at ldap/servers/slapd/intrinsics.h:30
#1  0x00007ffff792e412 in plugin_get_plugin_dependencies (plugin_name=0x5555555b1b28 "Multimaster Replication Plugin", names=0x7fffffffd0f0) at ldap/servers/slapd/plugin.c:1422
#2  0x00005555555652b3 in slapd_exemode_db2ldif (mcfg=0x7fffffffd150, argv=0x7fffffffd528, argc=11) at ldap/servers/slapd/main.c:2323
#3  0x00005555555652b3 in main (argc=11, argv=0x7fffffffd528) at ldap/servers/slapd/main.c:970
(gdb) up
#1  plugin_get_plugin_dependencies (plugin_name=0x5555555b1b28 "Multimaster Replication Plugin", names=0x7fffffffd0f0) at ldap/servers/slapd/plugin.c:1422
1422                if (strcasecmp(ep->plugin->plg_name, plugin_name) == 0) {
(gdb) p ep->plugin
$1 = (struct slapdplugin *) 0x55555587bdf0
(gdb) p *ep->plugin
$2 = {plg_private = 0x555555840f70, plg_version = 0x5555557bb010 "\006\006\a\005\a\006\005\a\001\a\a\a\006\a\a", plg_argc = 0, plg_argv = 0x0, plg_libpath = 0x0, 
  plg_initfunc = 0x0, plg_close = 0x0, plg_desc = {spd_id = 0x0, spd_vendor = 0x0, spd_version = 0x0, spd_description = 0x0}, plg_name = 0x0, plg_next = 0x0, plg_type = 6, 
  plg_dn = 0x0, plg_id = 0x0, plg_identity = 0x555555878f00, plg_precedence = 50, plg_group = 0x0, plg_conf = {plgc_target_subtrees = {subtrees = {elements = 0x0, 
        element_count = 0, alloc_count = 0}, special_data = {0, 0, 0, 0}}, plgc_excluded_target_subtrees = {subtrees = {elements = 0x0, element_count = 0, alloc_count = 0}, 
      special_data = {0, 0, 0, 0}}, plgc_bind_subtrees = {subtrees = {elements = 0x0, element_count = 0, alloc_count = 0}, special_data = {0, 0, 0, 0}}, 
    plgc_excluded_bind_subtrees = {subtrees = {elements = 0x0, element_count = 0, alloc_count = 0}, special_data = {0, 0, 0, 0}}, plgc_schema_check = 1, plgc_log_change = 2, 
    plgc_log_access = 0, plgc_log_audit = 0, plgc_invoke_for_replop = 1}, plg_cleanup = 0x0, plg_start = 0x0, plg_poststart = 0x0, plg_closed = 0, plg_removed = 0, 
  plg_started = 0, plg_stopped = 0, plg_op_counter = 0x0, plg_un = {plg_un_db = {plg_un_db_bind = 0x0, plg_un_db_unbind = 0x0, plg_un_db_search = 0x0, 
      plg_un_db_next_search_entry = 0x0, plg_un_db_next_search_entry_ext = 0x0, plg_un_db_search_results_release = 0x0, plg_un_db_prev_search_results = 0x0, 
      plg_un_db_entry_release = 0x0, plg_un_db_compare = 0x0, plg_un_db_modify = 0x0, plg_un_db_modrdn = 0x0, plg_un_db_add = 0x0, plg_un_db_delete = 0x0, 
      plg_un_db_abandon = 0x0, plg_un_db_config = 0x0, plg_un_db_seq = 0x0, plg_un_db_entry = 0x0, plg_un_db_referral = 0x0, plg_un_db_result = 0x0, plg_un_db_ldif2db = 0x0, 
      plg_un_db_db2ldif = 0x0, plg_un_db_db2index = 0x0, plg_un_db_archive2db = 0x0, plg_un_db_db2archive = 0x0, plg_un_db_upgradedb = 0x0, plg_un_db_upgradednformat = 0x0, 
      plg_un_db_begin = 0x0, plg_un_db_commit = 0x0, plg_un_db_abort = 0x0, plg_un_db_dbsize = 0x0, plg_un_db_dbtest = 0x0, plg_un_db_rmdb = 0x0, 
      plg_un_db_register_dn_callback = 0x0, plg_un_db_register_oc_callback = 0x0, plg_un_db_init_instance = 0x0, plg_un_db_wire_import = 0x0, plg_un_db_verify = 0x0, 
      plg_un_db_add_schema = 0x0, plg_un_db_get_info = 0x0, plg_un_db_set_info = 0x0, plg_un_db_ctrl_info = 0x0}, plg_un_pe = {plg_un_pe_exoids = 0x0, plg_un_pe_exnames = 0x0, 
      plg_un_pe_exhandler = 0x0, plg_un_pe_pre_exhandler = 0x0, plg_un_pe_post_exhandler = 0x0, plg_un_pe_be_exhandler = 0x0}, plg_un_pre = {plg_un_pre_bind = 0x0, 
      plg_un_pre_unbind = 0x0, plg_un_pre_search = 0x0, plg_un_pre_compare = 0x0, plg_un_pre_modify = 0x0, plg_un_pre_modrdn = 0x0, plg_un_pre_add = 0x0, plg_un_pre_delete = 0x0, 
      plg_un_pre_abandon = 0x0, plg_un_pre_entry = 0x0, plg_un_pre_referral = 0x0, plg_un_pre_result = 0x0}, plg_un_post = {plg_un_post_bind = 0x0, plg_un_post_unbind = 0x0, 
      plg_un_post_search = 0x0, plg_un_post_searchfail = 0x0, plg_un_post_compare = 0x0, plg_un_post_modify = 0x0, plg_un_post_modrdn = 0x0, plg_un_post_add = 0x0, 
      plg_un_post_delete = 0x0, plg_un_post_abandon = 0x0, plg_un_post_entry = 0x0, plg_un_post_referral = 0x0, plg_un_post_result = 0x0}, plg_un_bepre = {
      plg_un_bepre_modify = 0x0, plg_un_bepre_modrdn = 0x0, plg_un_bepre_add = 0x0, plg_un_bepre_delete = 0x0, plg_un_bepre_delete_tombstone = 0x0, plg_un_bepre_close = 0x0, 
      plg_un_bepre_backup = 0x0}, plg_un_bepost = {plg_un_bepost_modify = 0x0, plg_un_bepost_modrdn = 0x0, plg_un_bepost_add = 0x0, plg_un_bepost_delete = 0x0, 
      plg_un_bepost_open = 0x0, plg_un_bepost_backup = 0x0}, plg_un_internal_pre = {plg_un_internal_pre_modify = 0x0, plg_un_internal_pre_modrdn = 0x0, 
      plg_un_internal_pre_add = 0x0, plg_un_internal_pre_delete = 0x0, plg_un_internal_pre_bind = 0x0}, plg_un_internal_post = {plg_un_internal_post_modify = 0x0, 
      plg_un_internal_post_modrdn = 0x0, plg_un_internal_post_add = 0x0, plg_un_internal_post_delete = 0x0}, plg_un_mr = {plg_un_mr_filter_create = 0x0, 
      plg_un_mr_indexer_create = 0x0, plg_un_mr_filter_ava = 0x0, plg_un_mr_filter_sub = 0x0, plg_un_mr_values2keys = 0x0, plg_un_mr_assertion2keys_ava = 0x0, 
      plg_un_mr_assertion2keys_sub = 0x0, plg_un_mr_flags = 0, plg_un_mr_names = 0x0, plg_un_mr_compare = 0x0, plg_un_mr_normalize = 0x0}, plg_un_syntax = {
      plg_un_syntax_filter_ava = 0x0, plg_un_syntax_filter_ava_sv = 0x0, plg_un_syntax_filter_sub = 0x0, plg_un_syntax_filter_sub_sv = 0x0, plg_un_syntax_values2keys = 0x0, 
      plg_un_syntax_values2keys_sv = 0x0, plg_un_syntax_assertion2keys_ava = 0x0, plg_un_syntax_assertion2keys_sub = 0x0, plg_un_syntax_flags = 0, plg_un_syntax_names = 0x0, 
      plg_un_syntax_oid = 0x0, plg_un_syntax_compare = 0x0, plg_un_syntax_validate = 0x0, plg_un_syntax_normalize = 0x0}, plg_un_acl = {plg_un_acl_init = 0x0, 
      plg_un_acl_syntax_check = 0x0, plg_un_acl_access_allowed = 0x0, plg_un_acl_mods_allowed = 0x0, plg_un_acl_mods_update = 0x0}, plg_un_mmr = {plg_un_mmr_betxn_preop = 0x0, 
      plg_un_mmr_betxn_postop = 0x0}, plg_un_pwd_storage_scheme = {plg_un_pwd_storage_scheme_name = 0x0, plg_un_pwd_storage_scheme_enc = 0x0, plg_un_pwd_storage_scheme_dec = 0x0, 
      plg_un_pwd_storage_scheme_cmp = 0x0}, plg_un_entry_fetch_store = {plg_un_entry_fetch_func = 0x0, plg_un_entry_store_func = 0x0}, plg_un_betxnpre = {
      plg_un_betxnpre_modify = 0x0, plg_un_betxnpre_modrdn = 0x0, plg_un_betxnpre_add = 0x0, plg_un_betxnpre_delete = 0x0, plg_un_betxnpre_delete_tombstone = 0x0}, 
    plg_un_betxnpost = {plg_un_betxnpost_modify = 0x0, plg_un_betxnpost_modrdn = 0x0, plg_un_betxnpost_add = 0x0, plg_un_betxnpost_delete = 0x0}}}
(gdb) p *ep->plugin->plg_name
Cannot access memory at address 0x0
(gdb) p plugin_name
$3 = 0x5555555b1b28 "Multimaster Replication Plugin"

Yes as @firstyear was concerned this fix causes a crash when doing a db2ldif -r. The plugin is freed because its not enabled in offline mode, and then we crash trying to compare the plugin name:

no, they are enabled, but only the plugins MMR depends on should be started. The problem is that not enabled plugins are now freed but remain in the plugin list and when searching the list for the plugins MMR depends on we access a freed plugin.

The db2ldif -r crash is fixed by this change:

  @@ -3031,7 +3031,7 @@ plugin_setup(Slapi_Entry *plugin_entry, struct slapi_componentid *group, slapi_p
          add_plugin_entry_dn(dn_copy);
      }

 -    if (add_entry) {
 +    if (add_entry && enabled) {
          /* make a copy of the plugin entry for our own use because it will
             be freed later by the caller */
          Slapi_Entry *e_copy = slapi_entry_dup(plugin_entry);

I will create a new PR

Now the server crashes in suites/plugins/rootdn_plugin_test.py test:

#0  0x00007fdc581740f1 in free () at /lib64/libc.so.6
#1  0x00007fdc588c7085 in slapi_ch_free () at /usr/lib64/dirsrv/libslapd.so.0
#2  0x00007fdc5891b721 in ??? () at /usr/lib64/dirsrv/libslapd.so.0
#3  0x00007fdc589219c2 in plugin_start () at /usr/lib64/dirsrv/libslapd.so.0
#4  0x00007fdc58921e34 in plugin_add () at /usr/lib64/dirsrv/libslapd.so.0
#5  0x00007fdc588d6745 in dse_modify () at /usr/lib64/dirsrv/libslapd.so.0
#6  0x00007fdc5890aa40 in ??? () at /usr/lib64/dirsrv/libslapd.so.0
#7  0x00007fdc5890c160 in do_modify () at /usr/lib64/dirsrv/libslapd.so.0
#8  0x0000562cc64576fb in connection_dispatch_operation (pb=0x7fdc0c000b20, op=0x562cc81d5ac0, conn=0x7fdc40f92810) at ldap/servers/slapd/connection.c:626
#9  0x0000562cc64576fb in connection_threadmain () at ldap/servers/slapd/connection.c:1765
#10 0x00007fdc58426b98 in _pt_root (arg=0x562cc857f9f0) at ../../.././nspr/pr/src/pthreads/ptthread.c:201
#11 0x00007fdc583bd58e in start_thread () at /lib64/libpthread.so.0
#12 0x00007fdc581ea6a3 in clone () at /lib64/libc.so.6

sorry for creating so much trouble with this patch, will look into it and if a fix is not obvious we could decide to remove the patch (and live with some memory leaks).

A first look at the asan report shows that the free was not the one introduced in the patch, will investigate

@lkrispen It's okay, I remember this leak drove me insane trying to fix it also ... I think it's why I chose to leave it, and opted for the plugin framework rewrite, with an intent to lift-and-shift all the v3 plugins to the v4 execution system which is much safer and better definitions of plugin metadata lifetimes.

If you decide to keep digging ... good luck :D

If you decide to keep digging ... good luck :D

thanks for the encouragement :-)

I will not spend too much time on it, but knowing that my patch causes a crash and not understanding why is not so satisfactory :-(

ok, I didn't understand why it fails, but I do understand why we have memory leaks for not enabled plugins. It was introduced with the patch for #49186, in plugins_dependency_freeall() the freeing of plugins was commented out and move to plugin_freeall(). But this only frees enabled plugins.

A new PR where the free in plugin_dependecy_freeall is handling not enabled plugins is coming

Login to comment on this ticket.

Metadata
Attachments 1