From b1053b0e0c49dfadf791e0303ac5c5f30a47af8f Mon Sep 17 00:00:00 2001 From: William Brown Date: Mon, 30 Oct 2017 14:19:56 +1000 Subject: [PATCH] Ticket 49432 - filter optimise crash Bug Description: In a certain condition with a filter, when we removed the equality candidate to optimise it, with a nested and, during the merge process we would segfault Fix Description: Fix the merge subfilter process to be cleaner and work in all conditions. Merge the set of filter tests to cmocka in addition to the python tests to help catch this earlier https://pagure.io/389-ds-base/issue/49432 Author: wibrown Review by: ??? --- Makefile.am | 1 + ldap/servers/slapd/filter.c | 30 +++++++++------ test/libslapd/filter/optimise.c | 83 +++++++++++++++++++++++++++++++++++++++++ test/libslapd/test.c | 1 + test/test_slapd.h | 3 ++ 5 files changed, 106 insertions(+), 12 deletions(-) create mode 100644 test/libslapd/filter/optimise.c diff --git a/Makefile.am b/Makefile.am index 8e24957..c7f7b8a 100644 --- a/Makefile.am +++ b/Makefile.am @@ -1994,6 +1994,7 @@ TESTS = test_slapd \ test_slapd_SOURCES = test/main.c \ test/libslapd/test.c \ test/libslapd/counters/atomic.c \ + test/libslapd/filter/optimise.c \ test/libslapd/pblock/analytics.c \ test/libslapd/pblock/v3_compat.c \ test/libslapd/operation/v3_compat.c \ diff --git a/ldap/servers/slapd/filter.c b/ldap/servers/slapd/filter.c index 5e75871..8830939 100644 --- a/ldap/servers/slapd/filter.c +++ b/ldap/servers/slapd/filter.c @@ -1555,22 +1555,28 @@ filter_prioritise_element(Slapi_Filter **list, Slapi_Filter **head, Slapi_Filter static void filter_merge_subfilter(Slapi_Filter **list, Slapi_Filter **f_prev, Slapi_Filter **f_cur, Slapi_Filter **f_next) { - /* Cut our current AND/OR out */ - if (*f_prev != NULL) { - (*f_prev)->f_next = (*f_cur)->f_next; - } else if (*list == *f_cur) { - *list = (*f_cur)->f_next; - } - (*f_next) = (*f_cur)->f_next; - /* Look ahead to the end of our list, without the f_cur. */ - Slapi_Filter *f_cur_tail = *list; + /* First, graft in the new item between f_cur and f_cur -> f_next */ + Slapi_Filter *remainder = (*f_cur)->f_next; + (*f_cur)->f_next = (*f_cur)->f_list; + /* Go to the end of the newly grafted list, and put in our remainder. */ + Slapi_Filter *f_cur_tail = *f_cur; while (f_cur_tail->f_next != NULL) { f_cur_tail = f_cur_tail->f_next; } - /* Now append our descendant into the tail */ - f_cur_tail->f_next = (*f_cur)->f_list; - /* Finally free the remainder */ + f_cur_tail->f_next = remainder; + + /* Now indicate to the caller what the next element is. */ + *f_next = (*f_cur)->f_next; + + /* Now that we have grafted our list in, cut out f_cur */ + if (*f_prev != NULL) { + (*f_prev)->f_next = *f_next; + } else if (*list == *f_cur) { + *list = *f_next; + } + + /* Finally free the f_cur (and/or) */ slapi_filter_free(*f_cur, 0); } diff --git a/test/libslapd/filter/optimise.c b/test/libslapd/filter/optimise.c new file mode 100644 index 0000000..bcf4ccd --- /dev/null +++ b/test/libslapd/filter/optimise.c @@ -0,0 +1,83 @@ +/** BEGIN COPYRIGHT BLOCK + * Copyright (C) 2017 Red Hat, Inc. + * All rights reserved. + * + * License: GPL (version 3 or any later version). + * See LICENSE for details. + * END COPYRIGHT BLOCK **/ + +#include "../../test_slapd.h" + +/* To access filter optimise */ +#include + +void +test_libslapd_filter_optimise(void **state __attribute__((unused))) +{ + char *test_filters[] = { + "(&(uid=uid1)(sn=last1)(givenname=first1))", + "(&(uid=uid1)(&(sn=last1)(givenname=first1)))", + "(&(uid=uid1)(&(&(sn=last1))(&(givenname=first1))))", + "(&(uid=*)(sn=last3)(givenname=*))", + "(&(uid=*)(&(sn=last3)(givenname=*)))", + "(&(uid=uid5)(&(&(sn=*))(&(givenname=*))))", + "(&(objectclass=*)(uid=*)(sn=last*))", + "(&(objectclass=*)(uid=*)(sn=last1))", + + "(|(uid=uid1)(sn=last1)(givenname=first1))", + "(|(uid=uid1)(|(sn=last1)(givenname=first1)))", + "(|(uid=uid1)(|(|(sn=last1))(|(givenname=first1))))", + "(|(objectclass=*)(sn=last1)(|(givenname=first1)))", + "(|(&(objectclass=*)(sn=last1))(|(givenname=first1)))", + "(|(&(objectclass=*)(sn=last))(|(givenname=first1)))", + + "(&(uid=uid1)(!(cn=NULL)))", + "(&(!(cn=NULL))(uid=uid1))", + "(&(uid=*)(&(!(uid=1))(!(givenname=first1))))", + + "(&(|(uid=uid1)(uid=NULL))(sn=last1))", + "(&(|(uid=uid1)(uid=NULL))(!(sn=NULL)))", + "(&(|(uid=uid1)(sn=last2))(givenname=first1))", + "(|(&(uid=uid1)(!(uid=NULL)))(sn=last2))", + "(|(&(uid=uid1)(uid=NULL))(sn=last2))", + "(&(uid=uid5)(sn=*)(cn=*)(givenname=*)(uid=u*)(sn=la*)(cn=full*)(givenname=f*)(uid>=u)(!(givenname=NULL)))", + "(|(&(objectclass=*)(sn=last))(&(givenname=first1)))", + + "(&(uid=uid1)(sn=last1)(givenname=NULL))", + "(&(uid=uid1)(&(sn=last1)(givenname=NULL)))", + "(&(uid=uid1)(&(&(sn=last1))(&(givenname=NULL))))", + "(&(uid=uid1)(&(&(sn=last1))(&(givenname=NULL)(sn=*)))(|(sn=NULL)))", + "(&(uid=uid1)(&(&(sn=last*))(&(givenname=first*)))(&(sn=NULL)))", + + "(|(uid=NULL)(sn=NULL)(givenname=NULL))", + "(|(uid=NULL)(|(sn=NULL)(givenname=NULL)))", + "(|(uid=NULL)(|(|(sn=NULL))(|(givenname=NULL))))", + + "(uid>=uid3)", + "(&(uid=*)(uid>=uid3))", + "(|(uid>=uid3)(uid<=uid5))", + "(&(uid>=uid3)(uid<=uid5))", + "(|(&(uid>=uid3)(uid<=uid5))(uid=*))", + + "(|(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)" + "(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)" + "(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)" + "(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)" + "(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)" + "(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)" + "(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)" + "(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)(uid=*)" + "(uid=*))", + NULL + }; + + for (size_t i = 0; test_filters[i] != NULL; i++) { + char *filter_str = slapi_ch_strdup(test_filters[i]); + + struct slapi_filter *filter = slapi_str2filter(filter_str); + slapi_filter_optimise(filter); + slapi_filter_free(filter, 1); + slapi_ch_free_string(&filter_str); + } +} + diff --git a/test/libslapd/test.c b/test/libslapd/test.c index ffa650d..02c6c03 100644 --- a/test/libslapd/test.c +++ b/test/libslapd/test.c @@ -28,6 +28,7 @@ run_libslapd_tests(void) cmocka_unit_test(test_libslapd_operation_v3c_target_spec), cmocka_unit_test(test_libslapd_counters_atomic_usage), cmocka_unit_test(test_libslapd_counters_atomic_overflow), + cmocka_unit_test(test_libslapd_filter_optimise), cmocka_unit_test(test_libslapd_pal_meminfo), cmocka_unit_test(test_libslapd_util_cachesane), }; diff --git a/test/test_slapd.h b/test/test_slapd.h index ad4f73f..efccaea 100644 --- a/test/test_slapd.h +++ b/test/test_slapd.h @@ -26,6 +26,9 @@ int run_plugin_tests(void); /* libslapd */ void test_libslapd_hello(void **state); +/* libslapd-filter-optimise */ +void test_libslapd_filter_optimise(void **state); + /* libslapd-pblock-analytics */ void test_libslapd_pblock_analytics(void **state); -- 1.8.3.1