Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

structs for diabled plugins will not be freed #3399

Closed
389-ds-bot opened this issue Sep 13, 2020 · 15 comments
Closed

structs for diabled plugins will not be freed #3399

389-ds-bot opened this issue Sep 13, 2020 · 15 comments
Labels
closed: fixed Migration flag - Issue
Milestone
1.4.1

Comments

@389-ds-bot
Copy link

Cloned from Pagure issue: https://pagure.io/389-ds-base/issue/50340

  • Created at 2019-04-24 15:19:00 by lkrispen (@elkris)
  • Closed at 2020-02-26 16:24:21 as fixed
  • Assigned to nobody

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/elkris/TEST/caa/ws/389-ds-base/ldap/servers/slapd/ch_malloc.c:95
     2 0x7fc5b62f1d45 in slapi_entry_attr_get_charptr /home/elkris/TEST/caa/ws/389-ds-base/ldap/servers/slapd/entry.c:2751
     3 0x7fc5b63c81be in plugin_setup /home/elkris/TEST/caa/ws/389-ds-base/ldap/servers/slapd/plugin.c:2896
     4 0x7fc5b62b78f6 in load_plugin_entry /home/elkris/TEST/caa/ws/389-ds-base/ldap/servers/slapd/configdse.c:352
     5 0x7fc5b62e41a5 in dse_call_callback /home/elkris/TEST/caa/ws/389-ds-base/ldap/servers/slapd/dse.c:2642
     6 0x7fc5b62d8e65 in dse_read_one_file /home/elkris/TEST/caa/ws/389-ds-base/ldap/servers/slapd/dse.c:758
     7 0x7fc5b62d95cf in dse_read_file /home/elkris/TEST/caa/ws/389-ds-base/ldap/servers/slapd/dse.c:829
     8 0x440872 in init_dse_file /home/elkris/TEST/caa/ws/389-ds-base/ldap/servers/slapd/fedse.c:2556
     9 0x442297 in setup_internal_backends /home/elkris/TEST/caa/ws/389-ds-base/ldap/servers/slapd/fedse.c:2796
     10 0x447608 in main /home/elkris/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
@389-ds-bot 389-ds-bot added the closed: fixed Migration flag - Issue label Sep 13, 2020
@389-ds-bot 389-ds-bot added this to the 1.4.1 milestone Sep 13, 2020
@389-ds-bot
Copy link
Author

Comment from firstyear (@Firstyear) at 2019-04-29 00:54:20

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

@389-ds-bot
Copy link
Author

Comment from firstyear (@Firstyear) at 2019-04-29 00:54:21

Metadata Update from @Firstyear:

  • Custom field origin adjusted to None
  • Custom field reviewstatus adjusted to None

@389-ds-bot
Copy link
Author

Comment from lkrispen (@elkris) at 2019-05-07 15:38:47

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 ?

@389-ds-bot
Copy link
Author

Comment from firstyear (@Firstyear) at 2019-05-08 02:34:49

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 :)

@389-ds-bot
Copy link
Author

Comment from mreynolds (@mreynolds389) at 2019-05-09 17:54:22

Metadata Update from @mreynolds389:

  • Issue set to the milestone: 1.4.1

@389-ds-bot
Copy link
Author

Comment from mreynolds (@mreynolds389) at 2019-05-10 22:46:14

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"

@389-ds-bot
Copy link
Author

Comment from lkrispen (@elkris) at 2019-05-14 14:05:53

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

@389-ds-bot
Copy link
Author

Comment from vashirov (@vashirov) at 2019-05-17 12:31:32

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

@389-ds-bot
Copy link
Author

Comment from vashirov (@vashirov) at 2019-05-17 17:50:35

ns-slapd-standalone1.asan.30388

@389-ds-bot
Copy link
Author

Comment from lkrispen (@elkris) at 2019-05-21 09:56:36

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

@389-ds-bot
Copy link
Author

Comment from firstyear (@Firstyear) at 2019-05-22 00:33:14

@elkris 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

@389-ds-bot
Copy link
Author

Comment from lkrispen (@elkris) at 2019-05-22 16:49:27

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 :-(

@389-ds-bot
Copy link
Author

Comment from lkrispen (@elkris) at 2019-05-23 13:10:27

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

@389-ds-bot
Copy link
Author

389-ds-bot commented Sep 13, 2020 •
edited

Comment from lkrispen (@elkris) at 2020-02-26 16:24:12

#3459

@389-ds-bot
Copy link
Author

Comment from lkrispen (@elkris) at 2020-02-26 16:24:22

Metadata Update from @elkris:

  • Issue close_status updated to: fixed
  • Issue status updated to: Closed (was: Open)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
closed: fixed Migration flag - Issue
Projects
None yet
Milestone
1.4.1
Development

No branches or pull requests
1 participant
@389-ds-bot