From efd8801c3702eb53c0b6753dcb57b6872abb218d Mon Sep 17 00:00:00 2001
From: Mark Reynolds <mreynolds@redhat.com>
Date: Tue, 30 Aug 2016 10:32:45 -0400
Subject: [PATCH 402/404] Ticket 48970 - Serverside sorting crashes the server
Bug Description: When using a matching rule and server side sorting
the server does a double-free on the matching rule
keys which crashes the server.
Fix Description: Set the pblock pointer to NULL after the keys are
freed. This prevents the double free.
Also fixed some complier warnings/indentation.
Valgrind: passed
https://fedorahosted.org/389/ticket/48970
Reviewed by: nhosoi(Thanks!)
(cherry picked from commit 43997fa8782ca93e20595ae10e303d85e5b765f4)
(cherry picked from commit dba6ff0c2fc12881179beaeb1d62c97c8d487c5b)
---
ldap/servers/plugins/collation/collate.c | 14 ++++----
ldap/servers/plugins/collation/orfilter.c | 55 ++++++++++++++++++-------------
ldap/servers/slapd/back-ldbm/sort.c | 12 +++----
3 files changed, 43 insertions(+), 38 deletions(-)
diff --git a/ldap/servers/plugins/collation/collate.c b/ldap/servers/plugins/collation/collate.c
index 2a73ee1..6d293f7 100644
--- a/ldap/servers/plugins/collation/collate.c
+++ b/ldap/servers/plugins/collation/collate.c
@@ -376,23 +376,23 @@ collation_index (indexer_t* ix, struct berval** bvec, struct berval** prefixes)
return keys;
}
+/* The destructor function for a collation-based indexer. */
static void
collation_indexer_destroy (indexer_t* ix)
- /* The destructor function for a collation-based indexer. */
{
collation_indexer_t* etc = (collation_indexer_t*) ix->ix_etc;
if (etc->converter) {
- ucnv_close(etc->converter);
- etc->converter = NULL;
+ ucnv_close(etc->converter);
+ etc->converter = NULL;
}
if (etc->collator) {
- ucol_close(etc->collator);
- etc->collator = NULL;
+ ucol_close(etc->collator);
+ etc->collator = NULL;
}
if (etc->ix_keys != NULL) {
- ber_bvecfree (etc->ix_keys);
- etc->ix_keys = NULL;
+ ber_bvecfree (etc->ix_keys);
+ etc->ix_keys = NULL;
}
slapi_ch_free((void**)&ix->ix_etc);
ix->ix_etc = NULL; /* just for hygiene */
diff --git a/ldap/servers/plugins/collation/orfilter.c b/ldap/servers/plugins/collation/orfilter.c
index 2293baf..bf1ccf8 100644
--- a/ldap/servers/plugins/collation/orfilter.c
+++ b/ldap/servers/plugins/collation/orfilter.c
@@ -63,7 +63,7 @@ static void
indexer_free (indexer_t* ix)
{
if (ix->ix_destroy != NULL) {
- ix->ix_destroy (ix);
+ ix->ix_destroy (ix);
}
slapi_ch_free((void**)&ix);
}
@@ -250,23 +250,28 @@ op_filter_match (or_filter_t* or, struct berval** vals)
auto indexer_t* ix = or->or_indexer;
auto struct berval** v = ix->ix_index (ix, vals, NULL);
if (v != NULL) for (; *v; ++v) {
- auto struct berval** k = or->or_match_keys;
- if (k != NULL) for (; *k; ++k) {
- switch (or->or_op) {
- case SLAPI_OP_LESS:
- if (slapi_berval_cmp (*v, *k) < 0) return 0; break;
- case SLAPI_OP_LESS_OR_EQUAL:
- if (slapi_berval_cmp (*v, *k) <= 0) return 0; break;
- case SLAPI_OP_EQUAL:
- if (SLAPI_BERVAL_EQ (*v, *k)) return 0; break;
- case SLAPI_OP_GREATER_OR_EQUAL:
- if (slapi_berval_cmp (*v, *k) >= 0) return 0; break;
- case SLAPI_OP_GREATER:
- if (slapi_berval_cmp (*v, *k) > 0) return 0; break;
- default:
- break;
- }
- }
+ auto struct berval** k = or->or_match_keys;
+ if (k != NULL) for (; *k; ++k) {
+ switch (or->or_op) {
+ case SLAPI_OP_LESS:
+ if (slapi_berval_cmp (*v, *k) < 0) return 0;
+ break;
+ case SLAPI_OP_LESS_OR_EQUAL:
+ if (slapi_berval_cmp (*v, *k) <= 0) return 0;
+ break;
+ case SLAPI_OP_EQUAL:
+ if (SLAPI_BERVAL_EQ (*v, *k)) return 0;
+ break;
+ case SLAPI_OP_GREATER_OR_EQUAL:
+ if (slapi_berval_cmp (*v, *k) >= 0) return 0;
+ break;
+ case SLAPI_OP_GREATER:
+ if (slapi_berval_cmp (*v, *k) > 0) return 0;
+ break;
+ default:
+ break;
+ }
+ }
}
return -1;
}
@@ -599,7 +604,9 @@ op_indexer_destroy (Slapi_PBlock* pb)
auto indexer_t* ix = op_indexer_get (pb);
LDAPDebug (LDAP_DEBUG_FILTER, "op_indexer_destroy(%p)\n", (void*)ix, 0, 0);
if (ix != NULL) {
- indexer_free (ix);
+ indexer_free (ix);
+ /* The keys were freed, but we need to reset the pblock pointer */
+ slapi_pblock_set(pb, SLAPI_PLUGIN_MR_KEYS, NULL);
}
return 0;
}
@@ -652,10 +659,10 @@ typedef struct ss_indexer_t {
static void
ss_indexer_free (ss_indexer_t* ss)
{
- slapi_ch_free((void**)&ss->ss_oid);
+ slapi_ch_free_string(&ss->ss_oid);
if (ss->ss_indexer != NULL) {
- indexer_free (ss->ss_indexer);
- ss->ss_indexer = NULL;
+ indexer_free (ss->ss_indexer);
+ ss->ss_indexer = NULL;
}
slapi_ch_free((void**)&ss);
}
@@ -676,7 +683,9 @@ ss_indexer_destroy (Slapi_PBlock* pb)
auto ss_indexer_t* ss = ss_indexer_get (pb);
LDAPDebug (LDAP_DEBUG_FILTER, "ss_indexer_destroy(%p)\n", (void*)ss, 0, 0);
if (ss) {
- ss_indexer_free (ss);
+ ss_indexer_free(ss);
+ /* The keys were freed, but we need to reset the pblock pointer */
+ slapi_pblock_set(pb, SLAPI_PLUGIN_MR_KEYS, NULL);
}
}
diff --git a/ldap/servers/slapd/back-ldbm/sort.c b/ldap/servers/slapd/back-ldbm/sort.c
index 4164147..004b45c 100644
--- a/ldap/servers/slapd/back-ldbm/sort.c
+++ b/ldap/servers/slapd/back-ldbm/sort.c
@@ -61,15 +61,11 @@ static int print_out_sort_spec(char* buffer,sort_spec *s,int *size);
static void sort_spec_thing_free(sort_spec_thing *s)
{
- if (NULL != s->type) {
- slapi_ch_free((void **)&s->type);
- }
- if (NULL != s->matchrule) {
- slapi_ch_free( (void**)&s->matchrule);
- }
+ slapi_ch_free_string(&s->type);
+ slapi_ch_free_string(&s->matchrule);
if (NULL != s->mr_pb) {
destroy_matchrule_indexer(s->mr_pb);
- slapi_pblock_destroy (s->mr_pb);
+ slapi_pblock_destroy (s->mr_pb);
}
attr_done(&s->sattr);
slapi_ch_free( (void**)&s);
@@ -145,7 +141,7 @@ void sort_log_access(Slapi_PBlock *pb,sort_spec_thing *s,IDList *candidates)
/* Now output it */
ldbm_log_access_message(pb,buffer);
if (buffer != stack_buffer) {
- slapi_ch_free( (void**)&buffer);
+ slapi_ch_free_string(&buffer);
}
}
--
2.4.11