From becc8ef12c971d35146dde6d18cea3010b43f123 Mon Sep 17 00:00:00 2001 From: Thierry Bordaz Date: Tue, 21 Jun 2016 10:28:03 +0200 Subject: [PATCH] Ticket 48891 ns-slapd crashes during the shutdown after adding attribute with a matching rule Bug Description: in https://fedorahosted.org/389/ticket/48872, plugin free is moved from plugin_closeall to be_cleanup. The problem is that in be_cleanup, it frees be_database that can be a common reference to the same be plugin (ldbm). It triggers a double free. Fix Description: Fix //fedorahosted.org/389/ticket/48872, frees plugin->plg_pwdstorageschemename only if the plugin is typed PWD_STORAGE_SCHEME. This part of the fix is ok. Reverting the plugin_free to be called in plugin_closeall. https://fedorahosted.org/389/ticket/48891 Reviewed by: ? Platforms tested: F23 Flag Day: no Doc impact: no --- dirsrvtests/tests/tickets/ticket48891_test.py | 175 ++++++++++++++++++++++++++ ldap/servers/slapd/backend_manager.c | 1 - ldap/servers/slapd/daemon.c | 2 +- ldap/servers/slapd/plugin.c | 7 +- ldap/servers/slapd/proto-slap.h | 1 - 5 files changed, 178 insertions(+), 8 deletions(-) create mode 100644 dirsrvtests/tests/tickets/ticket48891_test.py diff --git a/dirsrvtests/tests/tickets/ticket48891_test.py b/dirsrvtests/tests/tickets/ticket48891_test.py new file mode 100644 index 0000000..bf2d1dd --- /dev/null +++ b/dirsrvtests/tests/tickets/ticket48891_test.py @@ -0,0 +1,175 @@ +# --- BEGIN COPYRIGHT BLOCK --- +# Copyright (C) 2015 Red Hat, Inc. +# All rights reserved. +# +# License: GPL (version 3 or any later version). +# See LICENSE for details. +# --- END COPYRIGHT BLOCK --- +# +import os +import sys +import time +import ldap +import logging +import pytest +from lib389 import DirSrv, Entry, tools, tasks +from lib389.tools import DirSrvTools +from lib389._constants import * +from lib389.properties import * +from lib389.tasks import * +from ldap.controls import SimplePagedResultsControl +from ldap.controls.simple import GetEffectiveRightsControl +import fnmatch + +log = logging.getLogger(__name__) + +installation_prefix = None + +CONFIG_DN = 'cn=config' +RDN_VAL_SUFFIX = 'ticket48891.org' +MYSUFFIX = 'dc=%s' % RDN_VAL_SUFFIX +MYSUFFIXBE = 'ticket48891' + +SEARCHFILTER = '(objectclass=person)' + +OTHER_NAME = 'other_entry' +MAX_OTHERS = 10 + +class TopologyStandalone(object): + def __init__(self, standalone): + standalone.open() + self.standalone = standalone + + +@pytest.fixture(scope="module") +def topology(request): + ''' + This fixture is used to standalone topology for the 'module'. + ''' + global installation_prefix + + if installation_prefix: + args_instance[SER_DEPLOYED_DIR] = installation_prefix + + standalone = DirSrv(verbose=False) + + # Args for the standalone instance + args_instance[SER_HOST] = HOST_STANDALONE + args_instance[SER_PORT] = PORT_STANDALONE + args_instance[SER_SERVERID_PROP] = SERVERID_STANDALONE + args_standalone = args_instance.copy() + standalone.allocate(args_standalone) + + # Get the status of the instance and restart it if it exists + instance_standalone = standalone.exists() + + # Remove the instance + if instance_standalone: + standalone.delete() + + # Create the instance + standalone.create() + + # Used to retrieve configuration information (dbdir, confdir...) + standalone.open() + + # clear the tmp directory + standalone.clearTmpDir(__file__) + + # Here we have standalone instance up and running + return TopologyStandalone(standalone) + + +def test_ticket48891_setup(topology): + """ + Check there is no core + Create a second backend + stop DS (that should trigger the core) + check there is no core + """ + log.info('Testing Ticket 48891 - ns-slapd crashes during the shutdown after adding attribute with a matching rule') + + # bind as directory manager + topology.standalone.log.info("Bind as %s" % DN_DM) + topology.standalone.simple_bind_s(DN_DM, PASSWORD) + + # check there is no core + entry = topology.standalone.search_s(CONFIG_DN, ldap.SCOPE_BASE, "(cn=config)",['nsslapd-workingdir']) + assert entry + assert entry[0] + assert entry[0].hasAttr('nsslapd-workingdir') + path = entry[0].getValue('nsslapd-workingdir') + cores = fnmatch.filter(os.listdir(path), 'core.*') + assert len(cores) == 0 + + topology.standalone.log.info("\n\n######################### SETUP SUFFIX o=ticket48891.org ######################\n") + + topology.standalone.backend.create(MYSUFFIX, {BACKEND_NAME: MYSUFFIXBE}) + topology.standalone.mappingtree.create(MYSUFFIX, bename=MYSUFFIXBE) + topology.standalone.add_s(Entry((MYSUFFIX, { + 'objectclass': "top domain".split(), + 'dc': RDN_VAL_SUFFIX}))) + + topology.standalone.log.info("\n\n######################### Generate Test data ######################\n") + + # add dummy entries on both backends + for cpt in range(MAX_OTHERS): + name = "%s%d" % (OTHER_NAME, cpt) + topology.standalone.add_s(Entry(("cn=%s,%s" % (name, SUFFIX), { + 'objectclass': "top person".split(), + 'sn': name, + 'cn': name}))) + for cpt in range(MAX_OTHERS): + name = "%s%d" % (OTHER_NAME, cpt) + topology.standalone.add_s(Entry(("cn=%s,%s" % (name, MYSUFFIX), { + 'objectclass': "top person".split(), + 'sn': name, + 'cn': name}))) + + topology.standalone.log.info("\n\n######################### SEARCH ALL ######################\n") + topology.standalone.log.info("Bind as %s and add the READ/SEARCH SELFDN aci" % DN_DM) + topology.standalone.simple_bind_s(DN_DM, PASSWORD) + + entries = topology.standalone.search_s(MYSUFFIX, ldap.SCOPE_SUBTREE, SEARCHFILTER) + topology.standalone.log.info("Returned %d entries.\n", len(entries)) + + assert MAX_OTHERS == len(entries) + + topology.standalone.log.info('%d person entries are successfully created under %s.' % (len(entries), MYSUFFIX)) + + + topology.standalone.stop(timeout=1) + + + cores = fnmatch.filter(os.listdir(path), 'core.*') + for core in cores: + core = os.path.join(path, core) + topology.standalone.log.info('cores are %s' % core) + assert not os.path.isfile(core) + + + +def test_ticket48891_final(topology): + #topology.standalone.delete() + log.info('Testcase PASSED') + + +def run_isolated(): + ''' + run_isolated is used to run these test cases independently of a test scheduler (xunit, py.test..) + To run isolated without py.test, you need to + - edit this file and comment '@pytest.fixture' line before 'topology' function. + - set the installation prefix + - run this program + ''' + global installation_prefix + installation_prefix = None + + topo = topology(True) + test_ticket48891_setup(topo) + test_ticket48891_final(topo) + + +if __name__ == '__main__': + run_isolated() + diff --git a/ldap/servers/slapd/backend_manager.c b/ldap/servers/slapd/backend_manager.c index 979144a..9084d95 100644 --- a/ldap/servers/slapd/backend_manager.c +++ b/ldap/servers/slapd/backend_manager.c @@ -323,7 +323,6 @@ be_cleanupall() slapi_pblock_set( &pb, SLAPI_BACKEND, backends[i] ); (*backends[i]->be_cleanup)( &pb ); - plugin_free(backends[i]->be_database); slapi_be_free(&backends[i]); } } diff --git a/ldap/servers/slapd/daemon.c b/ldap/servers/slapd/daemon.c index 90d1f7d..862f6e7 100644 --- a/ldap/servers/slapd/daemon.c +++ b/ldap/servers/slapd/daemon.c @@ -1318,7 +1318,7 @@ void slapd_daemon( daemon_ports_t *ports ) uniqueIDGenCleanup (); } - plugin_closeall( 0 /* Close Backends */, 1 /* Close Globals */); + plugin_closeall( 1 /* Close Backends */, 1 /* Close Globals */); if ( ! in_referral_mode ) { /* Close SNMP collator after the plugins closed... diff --git a/ldap/servers/slapd/plugin.c b/ldap/servers/slapd/plugin.c index de907d8..9634c82 100644 --- a/ldap/servers/slapd/plugin.c +++ b/ldap/servers/slapd/plugin.c @@ -71,6 +71,7 @@ static char *plugin_get_type_str( int type ); static void plugin_cleanup_list(); static int plugin_remove_plugins(struct slapdplugin *plugin_entry, char *plugin_type); static void plugin_remove_from_shutdown(struct slapdplugin *plugin_entry); +static void plugin_free(struct slapdplugin *plugin); static PLHashTable *global_plugin_dns = NULL; @@ -1962,10 +1963,6 @@ plugin_closeall(int close_backends, int close_globals) iterp = dep_plugin_entries; while (iterp) { nextp = iterp->next; - if (close_backends == 0 && iterp->plugin->plg_type == SLAPI_PLUGIN_DATABASE) { - iterp = nextp; - continue; - } slapi_entry_free(iterp->e); plugin_free(iterp->plugin); slapi_ch_free((void **)&iterp); @@ -2724,7 +2721,7 @@ plugin_add_descriptive_attributes( Slapi_Entry *e, struct slapdplugin *plugin ) /* clean up the memory associated with the plugin */ -void +static void plugin_free(struct slapdplugin *plugin) { slapi_log_error(SLAPI_LOG_TRACE, "plugin_free", "Freeing %s \n", plugin->plg_name ); diff --git a/ldap/servers/slapd/proto-slap.h b/ldap/servers/slapd/proto-slap.h index 7e0e632..555d15d 100644 --- a/ldap/servers/slapd/proto-slap.h +++ b/ldap/servers/slapd/proto-slap.h @@ -879,7 +879,6 @@ void global_plugin_init(); int plugin_call_plugins( Slapi_PBlock *, int ); int plugin_setup(Slapi_Entry *plugin_entry, struct slapi_componentid *group, slapi_plugin_init_fnptr initfunc, int add_to_dit, char *returntext); -void plugin_free(struct slapdplugin *plugin); int plugin_determine_exop_plugins( const char *oid, struct slapdplugin **plugin ); int plugin_call_exop_plugins( Slapi_PBlock *pb, struct slapdplugin *p ); Slapi_Backend * plugin_extended_op_getbackend( Slapi_PBlock *pb, struct slapdplugin *p); -- 2.5.0