From 9a8ee0954699da05501ded2900a834584346ef85 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.21.1