From dd7974b1ecfe7ef53eae9f99d6d0e5e8ab28900f Mon Sep 17 00:00:00 2001 From: Thierry Bordaz Date: Wed, 2 Mar 2016 18:13:41 +0100 Subject: [PATCH] Ticket 48746: Crash when indexing an attribute with a matching rule Bug Description: When creating a mr_indexer, we first look at the registered MR (global_mr_oids). If the indexer is not registered or have no indexer create function, we try - to find a plugin (possibly a custom one) that could provide such indexer create. - use a default indexer The problem is that going through the matching rule plugins, we pick the first one with an indexer even if it applies to a different syntax. For example, searching for an caseExactIA5Match MR plugin we select caseIgnoreIA5Match because it appears first in the plugins list. The consequence is that we may index with the wrong index function (and trigger https://fedorahosted.org/389/ticket/48745) and picking a wrong indexer_create (or filter_create) function the MR private in the pblock (SLAPI_PLUGIN_OBJECT) can be NULL (https://fedorahosted.org/389/ticket/48746). Also we can imagine an incorrect custom MR plugin that forgets to set this MR private object and could trigger a crash. So use of MR private object should check if it was set. Fix Description: The fix is: slapi_mr_indexer_create: plugin_mr_filter_create: If there is no register MR for a specific oid, we search a plugin that can handle that oid (plugin_mr_find). mr_wrap_mr_index_sv_fn: mr_wrap_mr_index_fn: default_mr_filter_match: default_mr_filter_index: hardening if a custom plugin index_create function did not set SLAPI_PLUGIN_OBJECT https://fedorahosted.org/389/ticket/48746 Reviewed by: ? Platforms tested: F17 Flag Day: no Doc impact: no --- ldap/servers/slapd/plugin_mr.c | 108 ++++++++++++++++++++++++----------------- 1 file changed, 64 insertions(+), 44 deletions(-) diff --git a/ldap/servers/slapd/plugin_mr.c b/ldap/servers/slapd/plugin_mr.c index d25fca9..90175ae 100644 --- a/ldap/servers/slapd/plugin_mr.c +++ b/ldap/servers/slapd/plugin_mr.c @@ -165,44 +165,43 @@ slapi_mr_indexer_create (Slapi_PBlock* opb) } else { - /* call each plugin, until one is able to handle this request. */ - rc = LDAP_UNAVAILABLE_CRITICAL_EXTENSION; - for (mrp = get_plugin_list(PLUGIN_LIST_MATCHINGRULE); mrp != NULL; mrp = mrp->plg_next) - { + /* look for a new syntax-style mr plugin */ + struct slapdplugin* pi = plugin_mr_find(oid); + rc = LDAP_UNAVAILABLE_CRITICAL_EXTENSION; + + /* register that plugin at the condition it has a createFn/index/indexSvFn */ + if (pi) { IFP indexFn = NULL; IFP indexSvFn = NULL; Slapi_PBlock pb; - memcpy (&pb, opb, sizeof(Slapi_PBlock)); - slapi_pblock_set(&pb, SLAPI_PLUGIN, mrp); - if (slapi_pblock_get(&pb, SLAPI_PLUGIN_MR_INDEXER_CREATE_FN, &createFn)) { - /* plugin not a matchingrule type */ - continue; - } + memcpy(&pb, opb, sizeof (Slapi_PBlock)); + slapi_pblock_set(&pb, SLAPI_PLUGIN, pi); + slapi_pblock_get(&pb, SLAPI_PLUGIN_MR_INDEXER_CREATE_FN, &createFn); if (createFn && !createFn(&pb)) { + /* we need to call the createFn before testing the indexFn/indexSvFn + * because it sets the index callbacks + */ slapi_pblock_get(&pb, SLAPI_PLUGIN_MR_INDEX_FN, &indexFn); slapi_pblock_get(&pb, SLAPI_PLUGIN_MR_INDEX_SV_FN, &indexSvFn); if (indexFn || indexSvFn) { - /* Success: this plugin can handle it. */ + /* Use the defined indexer_create function if it exists */ memcpy(opb, &pb, sizeof (Slapi_PBlock)); - plugin_mr_bind(oid, mrp); /* for future reference */ + plugin_mr_bind(oid, pi); /* for future reference */ rc = 0; /* success */ - break; } - } - } - if (rc != 0) { - /* look for a new syntax-style mr plugin */ - struct slapdplugin *pi = plugin_mr_find(oid); - if (pi) { - Slapi_PBlock pb; - memcpy (&pb, opb, sizeof(Slapi_PBlock)); - slapi_pblock_set(&pb, SLAPI_PLUGIN, pi); - rc = default_mr_indexer_create(&pb); - if (!rc) { - memcpy (opb, &pb, sizeof(Slapi_PBlock)); - plugin_mr_bind (oid, pi); /* for future reference */ - } + } + if (pi && (rc != 0)) { + /* No defined indexer_create or it fails + * Let's use the default one + */ + Slapi_PBlock pb; + memcpy(&pb, opb, sizeof (Slapi_PBlock)); + slapi_pblock_set(&pb, SLAPI_PLUGIN, pi); + rc = default_mr_indexer_create(&pb); + if (!rc) { + memcpy(opb, &pb, sizeof (Slapi_PBlock)); + plugin_mr_bind(oid, pi); /* for future reference */ } } } @@ -292,8 +291,14 @@ mr_wrap_mr_index_sv_fn(Slapi_PBlock* pb) slapi_pblock_set(pb, SLAPI_PLUGIN_MR_KEYS, out_vals); /* we have to save out_vals to free next time or during destroy */ slapi_pblock_get(pb, SLAPI_PLUGIN_OBJECT, &mrpriv); - mr_private_indexer_done(mrpriv); /* free old vals, if any */ - mrpriv->sva = out_vals; /* save pointer for later */ + + /* In case SLAPI_PLUGIN_OBJECT is not set + * (e.g. custom index/filter create function did not initialize it + */ + if (mrpriv) { + mr_private_indexer_done(mrpriv); /* free old vals, if any */ + mrpriv->sva = out_vals; /* save pointer for later */ + } rc = 0; } return rc; @@ -329,8 +334,14 @@ mr_wrap_mr_index_fn(Slapi_PBlock* pb) get freed by mr_private_indexer_done() */ /* we have to save out_vals to free next time or during destroy */ slapi_pblock_get(pb, SLAPI_PLUGIN_OBJECT, &mrpriv); - mr_private_indexer_done(mrpriv); /* free old vals, if any */ - mrpriv->bva = out_vals; /* save pointer for later */ + + /* In case SLAPI_PLUGIN_OBJECT is not set + * (e.g. custom index/filter create function did not initialize it + */ + if (mrpriv) { + mr_private_indexer_done(mrpriv); /* free old vals, if any */ + mrpriv->bva = out_vals; /* save pointer for later */ + } /* set return value berval array for caller */ slapi_pblock_set(pb, SLAPI_PLUGIN_MR_KEYS, out_vals); @@ -359,6 +370,11 @@ default_mr_filter_match(void *obj, Slapi_Entry *e, Slapi_Attr *attr) */ int rc = -1; struct mr_private* mrpriv = (struct mr_private*)obj; + + /* In case SLAPI_PLUGIN_OBJECT is not set, mrpriv may be NULL */ + if (mrpriv == NULL) + return rc; + for (; (rc == -1) && (attr != NULL); slapi_entry_next_attr(e, attr, &attr)) { char* type = NULL; if (!slapi_attr_get_type (attr, &type) && type != NULL && @@ -385,6 +401,22 @@ default_mr_filter_index(Slapi_PBlock *pb) struct mr_private* mrpriv = NULL; slapi_pblock_get(pb, SLAPI_PLUGIN_OBJECT, &mrpriv); + + /* In case SLAPI_PLUGIN_OBJECT is not set + * (e.g. custom index/filter create function did not initialize it + */ + if (mrpriv == NULL) { + char* mrOID = NULL; + char* mrTYPE = NULL; + slapi_pblock_get(pb, SLAPI_PLUGIN_MR_OID, &mrOID); + slapi_pblock_get(pb, SLAPI_PLUGIN_MR_TYPE, &mrTYPE); + + slapi_log_error(SLAPI_LOG_FATAL, "default_mr_filter_index", + "Failure because mrpriv is NULL : %s %s\n", + mrOID ? "" : " oid", + mrTYPE ? "" : " attribute type"); + return -1; + } slapi_pblock_set(pb, SLAPI_PLUGIN, (void *)mrpriv->pi); slapi_pblock_set(pb, SLAPI_PLUGIN_MR_TYPE, (void *)mrpriv->type); @@ -547,19 +579,7 @@ plugin_mr_filter_create (mr_filter_t* f) { rc = attempt_mr_filter_create (f, mrp, &pb); } - else - { - /* call each plugin, until one is able to handle this request. */ - for (mrp = get_plugin_list(PLUGIN_LIST_MATCHINGRULE); mrp != NULL; mrp = mrp->plg_next) - { - if (!(rc = attempt_mr_filter_create (f, mrp, &pb))) - { - plugin_mr_bind (f->mrf_oid, mrp); /* for future reference */ - break; - } - } - } - if (rc) + if (!mrp || rc) { /* look for a new syntax-style mr plugin */ mrp = plugin_mr_find(f->mrf_oid); -- 1.7.11.7