From 5497023c6857ae0fcf046e57744a91107d362c41 Mon Sep 17 00:00:00 2001

From: Thierry Bordaz <tbordaz@redhat.com>

Date: Mon, 25 Nov 2019 10:59:44 +0100

Subject: [PATCH] Ticket 50736 - RetroCL trimming may crash at shutdown if

 trimming configuration is invalid

Bug Description:

	If config of retroCL trimming contains invalid value for trim-interval

        and/or maxage, then the trimming initialization is skipped.

        In such case the trimming structures are not allocated and if they

        are freed at shutdown it triggers a crash

Fix Description:

        When trimming mechanism is stopped (at shutdown) check that

        it was successfully initialized before freeing the structs

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

Reviewed by: Mark Reynolds

Platforms tested: F30

Flag Day: no

Doc impact: no

---

 .../suites/replication/changelog_test.py      | 185 ++++++++++++++++++

 ldap/servers/plugins/retrocl/retrocl_trim.c   |  17 +-

 2 files changed, 196 insertions(+), 6 deletions(-)

diff --git a/dirsrvtests/tests/suites/replication/changelog_test.py b/dirsrvtests/tests/suites/replication/changelog_test.py

index 0b6b886f3..0d3e85bb2 100755

--- a/dirsrvtests/tests/suites/replication/changelog_test.py

+++ b/dirsrvtests/tests/suites/replication/changelog_test.py

@@ -16,6 +16,12 @@ from lib389.replica import Replicas

 from lib389.idm.user import UserAccounts

 from lib389.topologies import topology_m2 as topo

 from lib389._constants import *

+from lib389.plugins import RetroChangelogPlugin

+from lib389.dseldif import DSEldif

+from lib389.tasks import *

+from lib389.utils import *

+

+pytestmark = pytest.mark.tier1

 TEST_ENTRY_NAME = 'replusr'

 NEW_RDN_NAME = 'cl5usr'

@@ -235,6 +241,185 @@ def test_verify_changelog_offline_backup(topo):

     _check_changelog_ldif(topo, changelog_ldif)

+@pytest.mark.ds47669

+def test_changelog_maxage(topo, changelog_init):

+    """Check nsslapd-changelog max age values

+

+    :id: d284ff27-03b2-412c-ac74-ac4f2d2fae3b

+    :setup: Replication with two master, change nsslapd-changelogdir to

+    '/var/lib/dirsrv/slapd-master1/changelog' and

+    set cn=Retro Changelog Plugin,cn=plugins,cn=config to 'on'

+    :steps:

+        1. Set nsslapd-changelogmaxage in cn=changelog5,cn=config to values - '12345','10s','30M','12h','2D','4w'

+        2. Set nsslapd-changelogmaxage in cn=changelog5,cn=config to values - '-123','xyz'

+

+    :expectedresults:

+        1. Operation should be successful

+        2. Operation should be unsuccessful

+     """

+    log.info('1. Test nsslapd-changelogmaxage in cn=changelog5,cn=config')

+

+    # bind as directory manager

+    topo.ms["master1"].log.info("Bind as %s" % DN_DM)

+    topo.ms["master1"].simple_bind_s(DN_DM, PASSWORD)

+

+    add_and_check(topo, CHANGELOG, MAXAGE, '12345', True)

+    add_and_check(topo, CHANGELOG, MAXAGE, '10s', True)

+    add_and_check(topo, CHANGELOG, MAXAGE, '30M', True)

+    add_and_check(topo, CHANGELOG, MAXAGE, '12h', True)

+    add_and_check(topo, CHANGELOG, MAXAGE, '2D', True)

+    add_and_check(topo, CHANGELOG, MAXAGE, '4w', True)

+    add_and_check(topo, CHANGELOG, MAXAGE, '-123', False)

+    add_and_check(topo, CHANGELOG, MAXAGE, 'xyz', False)

+

+

+@pytest.mark.ds47669

+def test_ticket47669_changelog_triminterval(topo, changelog_init):

+    """Check nsslapd-changelog triminterval values

+

+    :id: 8f850c37-7e7c-49dd-a4e0-9344638616d6

+    :setup: Replication with two master, change nsslapd-changelogdir to

+    '/var/lib/dirsrv/slapd-master1/changelog' and

+    set cn=Retro Changelog Plugin,cn=plugins,cn=config to 'on'

+    :steps:

+        1. Set nsslapd-changelogtrim-interval in cn=changelog5,cn=config to values -

+           '12345','10s','30M','12h','2D','4w'

+        2. Set nsslapd-changelogtrim-interval in cn=changelog5,cn=config to values - '-123','xyz'

+

+    :expectedresults:

+        1. Operation should be successful

+        2. Operation should be unsuccessful

+     """

+    log.info('2. Test nsslapd-changelogtrim-interval in cn=changelog5,cn=config')

+

+    # bind as directory manager

+    topo.ms["master1"].log.info("Bind as %s" % DN_DM)

+    topo.ms["master1"].simple_bind_s(DN_DM, PASSWORD)

+

+    add_and_check(topo, CHANGELOG, TRIMINTERVAL, '12345', True)

+    add_and_check(topo, CHANGELOG, TRIMINTERVAL, '10s', True)

+    add_and_check(topo, CHANGELOG, TRIMINTERVAL, '30M', True)

+    add_and_check(topo, CHANGELOG, TRIMINTERVAL, '12h', True)

+    add_and_check(topo, CHANGELOG, TRIMINTERVAL, '2D', True)

+    add_and_check(topo, CHANGELOG, TRIMINTERVAL, '4w', True)

+    add_and_check(topo, CHANGELOG, TRIMINTERVAL, '-123', False)

+    add_and_check(topo, CHANGELOG, TRIMINTERVAL, 'xyz', False)

+

+

+@pytest.mark.ds47669

+def test_changelog_compactdbinterval(topo, changelog_init):

+    """Check nsslapd-changelog compactdbinterval values

+

+    :id: 0f4b3118-9dfa-4c2a-945c-72847b42a48c

+    :setup: Replication with two master, change nsslapd-changelogdir to

+    '/var/lib/dirsrv/slapd-master1/changelog' and

+    set cn=Retro Changelog Plugin,cn=plugins,cn=config to 'on'

+    :steps:

+        1. Set nsslapd-changelogcompactdb-interval in cn=changelog5,cn=config to values -

+           '12345','10s','30M','12h','2D','4w'

+        2. Set nsslapd-changelogcompactdb-interval in cn=changelog5,cn=config to values -

+           '-123','xyz'

+

+    :expectedresults:

+        1. Operation should be successful

+        2. Operation should be unsuccessful

+     """

+    log.info('3. Test nsslapd-changelogcompactdb-interval in cn=changelog5,cn=config')

+

+    # bind as directory manager

+    topo.ms["master1"].log.info("Bind as %s" % DN_DM)

+    topo.ms["master1"].simple_bind_s(DN_DM, PASSWORD)

+

+    add_and_check(topo, CHANGELOG, COMPACTDBINTERVAL, '12345', True)

+    add_and_check(topo, CHANGELOG, COMPACTDBINTERVAL, '10s', True)

+    add_and_check(topo, CHANGELOG, COMPACTDBINTERVAL, '30M', True)

+    add_and_check(topo, CHANGELOG, COMPACTDBINTERVAL, '12h', True)

+    add_and_check(topo, CHANGELOG, COMPACTDBINTERVAL, '2D', True)

+    add_and_check(topo, CHANGELOG, COMPACTDBINTERVAL, '4w', True)

+    add_and_check(topo, CHANGELOG, COMPACTDBINTERVAL, '-123', False)

+    add_and_check(topo, CHANGELOG, COMPACTDBINTERVAL, 'xyz', False)

+

+

+@pytest.mark.ds47669

+def test_retrochangelog_maxage(topo, changelog_init):

+    """Check nsslapd-retrochangelog max age values

+

+    :id: 0cb84d81-3e86-4dbf-84a2-66aefd8281db

+    :setup: Replication with two master, change nsslapd-changelogdir to

+    '/var/lib/dirsrv/slapd-master1/changelog' and

+    set cn=Retro Changelog Plugin,cn=plugins,cn=config to 'on'

+    :steps:

+        1. Set nsslapd-changelogmaxage in cn=Retro Changelog Plugin,cn=plugins,cn=config to values -

+           '12345','10s','30M','12h','2D','4w'

+        2. Set nsslapd-changelogmaxage in cn=Retro Changelog Plugin,cn=plugins,cn=config to values -

+           '-123','xyz'

+

+    :expectedresults:

+        1. Operation should be successful

+        2. Operation should be unsuccessful

+     """

+    log.info('4. Test nsslapd-changelogmaxage in cn=Retro Changelog Plugin,cn=plugins,cn=config')

+

+    # bind as directory manager

+    topo.ms["master1"].log.info("Bind as %s" % DN_DM)

+    topo.ms["master1"].simple_bind_s(DN_DM, PASSWORD)

+

+    add_and_check(topo, RETROCHANGELOG, MAXAGE, '12345', True)

+    add_and_check(topo, RETROCHANGELOG, MAXAGE, '10s', True)

+    add_and_check(topo, RETROCHANGELOG, MAXAGE, '30M', True)

+    add_and_check(topo, RETROCHANGELOG, MAXAGE, '12h', True)

+    add_and_check(topo, RETROCHANGELOG, MAXAGE, '2D', True)

+    add_and_check(topo, RETROCHANGELOG, MAXAGE, '4w', True)

+    add_and_check(topo, RETROCHANGELOG, MAXAGE, '-123', False)

+    add_and_check(topo, RETROCHANGELOG, MAXAGE, 'xyz', False)

+

+    topo.ms["master1"].log.info("ticket47669 was successfully verified.")

+

+@pytest.mark.ds50736

+def test_retrochangelog_trimming_crash(topo, changelog_init):

+    """Check that when retroCL nsslapd-retrocthangelog contains invalid

+    value, then the instance does not crash at shutdown

+

+    :id: 5d9bd7ca-e9bf-4be9-8fc8-902aa5513052

+    :setup: Replication with two master, change nsslapd-changelogdir to

+    '/var/lib/dirsrv/slapd-master1/changelog' and

+    set cn=Retro Changelog Plugin,cn=plugins,cn=config to 'on'

+    :steps:

+        1. Set nsslapd-changelogmaxage in cn=Retro Changelog Plugin,cn=plugins,cn=config to value '-1'

+           This value is invalid. To disable retroCL trimming it should be set to 0

+        2. Do several restart

+        3. check there is no 'Detected Disorderly Shutdown' message (crash)

+        4. restore valid value for nsslapd-changelogmaxage '1w'

+

+    :expectedresults:

+        1. Operation should be successful

+        2. Operation should be successful

+        3. Operation should be successful

+        4. Operation should be successful

+     """

+    log.info('1. Test retroCL trimming crash in cn=Retro Changelog Plugin,cn=plugins,cn=config')

+

+    # set the nsslapd-changelogmaxage directly on dse.ldif

+    # because the set value is invalid

+    topo.ms["master1"].log.info("ticket50736 start verification")

+    topo.ms["master1"].stop()

+    retroPlugin = RetroChangelogPlugin(topo.ms["master1"])

+    dse_ldif = DSEldif(topo.ms["master1"])

+    dse_ldif.replace(retroPlugin.dn, 'nsslapd-changelogmaxage', '-1')

+    topo.ms["master1"].start()

+

+    # The crash should be systematic, but just in case do several restart

+    # with a delay to let all plugin init

+    for i in range(5):

+        time.sleep(1)

+        topo.ms["master1"].stop()

+        topo.ms["master1"].start()

+

+    assert not topo.ms["master1"].detectDisorderlyShutdown()

+

+    topo.ms["master1"].log.info("ticket 50736 was successfully verified.")

+

+

 if __name__ == '__main__':

     # Run isolated

     # -s for DEBUG mode

diff --git a/ldap/servers/plugins/retrocl/retrocl_trim.c b/ldap/servers/plugins/retrocl/retrocl_trim.c

index a46534984..0378eb7f6 100644

--- a/ldap/servers/plugins/retrocl/retrocl_trim.c

+++ b/ldap/servers/plugins/retrocl/retrocl_trim.c

@@ -481,11 +481,16 @@ retrocl_init_trimming(void)

 void

 retrocl_stop_trimming(void)

 {

-    retrocl_trimming = 0;

-    if (retrocl_trim_ctx) {

-        slapi_eq_cancel(retrocl_trim_ctx);

-        retrocl_trim_ctx = NULL;

+    if (retrocl_trimming) {

+        /* RetroCL trimming config was valid and trimming struct allocated

+         * Let's free them

+         */

+        retrocl_trimming = 0;

+        if (retrocl_trim_ctx) {

+            slapi_eq_cancel(retrocl_trim_ctx);

+            retrocl_trim_ctx = NULL;

+        }

+        PR_DestroyLock(ts.ts_s_trim_mutex);

+        ts.ts_s_trim_mutex = NULL;

     }

-    PR_DestroyLock(ts.ts_s_trim_mutex);

-    ts.ts_s_trim_mutex = NULL;

 }

-- 

2.24.1