From 855d78b5af6a97efa31f6986d27566c990da0826 Mon Sep 17 00:00:00 2001 From: William Brown Date: Jul 05 2018 19:59:48 +0000 Subject: 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: mreynolds (Thanks!) (cherry picked from commit 5c89dd8f9c8eb77c967574412d049d55565bb364) --- diff --git a/Makefile.am b/Makefile.am index a4af4e1..c10b428 100644 --- a/Makefile.am +++ b/Makefile.am @@ -2004,6 +2004,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 87ec0de..46a2266 100644 --- a/ldap/servers/slapd/filter.c +++ b/ldap/servers/slapd/filter.c @@ -1561,22 +1561,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);