add / clear verified flag to pair lists

so that we avoid slowdowns with repeated recursion
FreeRADIUS · Sep 10, 2023 · 9fb6392 · 9fb6392
1 parent 6dae134
commit 9fb6392
Show file tree

Hide file tree

Showing 4 changed files with 114 additions and 0 deletions.
diff --git a/src/lib/util/pair.c b/src/lib/util/pair.c
@@ -1227,6 +1227,11 @@ int fr_pair_prepend(fr_pair_list_t *list, fr_pair_t *to_add)
 {
 	PAIR_VERIFY(to_add);
 
+#ifdef WITH_VERIFY_PTR
+	fr_assert(!fr_pair_order_list_in_a_list(to_add));
+	list->verified = false;
+#endif
+
 	if (fr_pair_order_list_in_a_list(to_add)) {
 		fr_strerror_printf(IN_A_LIST_MSG, to_add);
 		return -1;
@@ -1253,6 +1258,11 @@ int fr_pair_append(fr_pair_list_t *list, fr_pair_t *to_add)
 {
 	PAIR_VERIFY(to_add);
 
+#ifdef WITH_VERIFY_PTR
+	fr_assert(!fr_pair_order_list_in_a_list(to_add));
+	list->verified = false;
+#endif
+
 	if (fr_pair_order_list_in_a_list(to_add)) {
 		fr_strerror_printf(IN_A_LIST_MSG, to_add);
 		return -1;
@@ -1276,6 +1286,11 @@ int fr_pair_insert_after(fr_pair_list_t *list, fr_pair_t *pos, fr_pair_t *to_add
 {
 	PAIR_VERIFY(to_add);
 
+#ifdef WITH_VERIFY_PTR
+	fr_assert(!fr_pair_order_list_in_a_list(to_add));
+	list->verified = false;
+#endif
+
 	if (fr_pair_order_list_in_a_list(to_add)) {
 		fr_strerror_printf(IN_A_LIST_MSG, to_add);
 		return -1;
@@ -1304,6 +1319,12 @@ int fr_pair_insert_before(fr_pair_list_t *list, fr_pair_t *pos, fr_pair_t *to_ad
 {
 	PAIR_VERIFY(to_add);
 
+#ifdef WITH_VERIFY_PTR
+	fr_assert(!fr_pair_order_list_in_a_list(to_add));
+	fr_assert(!pos || fr_pair_order_list_in_a_list(pos));
+	list->verified = false;
+#endif
+
 	if (fr_pair_order_list_in_a_list(to_add)) {
 		fr_strerror_printf(IN_A_LIST_MSG, to_add);
 		return -1;
@@ -1332,6 +1353,12 @@ void fr_pair_replace(fr_pair_list_t *list, fr_pair_t *to_replace, fr_pair_t *vp)
 	PAIR_VERIFY_WITH_LIST(list, to_replace);
 	PAIR_VERIFY(vp);
 
+#ifdef WITH_VERIFY_PTR
+	fr_assert(!fr_pair_order_list_in_a_list(vp));
+	fr_assert(fr_pair_order_list_in_a_list(to_replace));
+	list->verified = false;
+#endif
+
 	fr_pair_insert_after(list, to_replace, vp);
 	fr_pair_remove(list, to_replace);
 	talloc_free(to_replace);
@@ -3082,6 +3109,8 @@ void fr_pair_verify(char const *file, int line, fr_pair_list_t const *list, fr_p
 
        case FR_TYPE_STRUCTURAL:
        {
+	       if (vp->vp_group.verified) break;
+
 	       fr_pair_list_foreach(&vp->vp_group, child) {
 			TALLOC_CTX *parent = talloc_parent(child);
 
@@ -3101,6 +3130,8 @@ void fr_pair_verify(char const *file, int line, fr_pair_list_t const *list, fr_p
 //			fr_assert(fr_dict_attr_can_contain(vp->da, child->da));
 			fr_pair_verify(file, line, &vp->vp_group, child);
 		}
+
+	       UNCONST(fr_pair_t *, vp)->vp_group.verified = true;
 	}
 	       break;
 
@@ -3165,6 +3196,11 @@ void fr_pair_list_verify(char const *file, int line, TALLOC_CTX const *expected,
 
 	if (fr_pair_list_empty(list)) return;	/* Fast path */
 
+	/*
+	 *	Only verify the list if it has been modified.
+	 */
+	if (list->verified) return;
+
 	for (slow = fr_pair_list_head(list), fast = fr_pair_list_head(list);
 	     slow && fast;
 	     slow = fr_pair_list_next(list, slow), fast = fr_pair_list_next(list, fast)) {
@@ -3202,6 +3238,8 @@ void fr_pair_list_verify(char const *file, int line, TALLOC_CTX const *expected,
 		parent = talloc_parent(slow);
 		if (expected && (parent != expected)) goto bad_parent;
 	}
+
+	UNCONST(fr_pair_list_t *, list)->verified = true;
 }
 #endif
 

diff --git a/src/lib/util/pair.h b/src/lib/util/pair.h
@@ -53,6 +53,10 @@ typedef struct {
         FR_TLIST_HEAD(fr_pair_order_list)	order;			//!< Maintains the relative order of pairs in a list.
 
 	bool				 _CONST is_child;		//!< is a child of a VP
+
+#ifdef WITH_VERIFY_PTR
+	unsigned int		verified : 1;				//!< hack to avoid O(N^3) issues
+#endif
 } fr_pair_list_t;
 
 /** Stores an attribute, a value and various bits of other data

diff --git a/src/lib/util/pair_inline.c b/src/lib/util/pair_inline.c
@@ -93,6 +93,11 @@ _INLINE fr_pair_t *fr_pair_list_prev(fr_pair_list_t const *list, fr_pair_t const
  */
 _INLINE fr_pair_t *fr_pair_remove(fr_pair_list_t *list, fr_pair_t *vp)
 {
+#ifdef WITH_VERIFY_PTR
+	fr_assert(fr_pair_order_list_in_a_list(vp));
+	list->verified = false;
+#endif
+
 	return fr_pair_order_list_remove(&list->order, vp);
 }
 

diff --git a/src/tests/unit/protocols/dhcpv6/nested.txt b/src/tests/unit/protocols/dhcpv6/nested.txt
@@ -0,0 +1,67 @@
+#  -*- text -*-
+#  Copyright (C) 2023 Network RADIUS SARL ([email protected])
+#  This work is licensed under CC-BY version 4.0 https://creativecommons.org/licenses/by/4.0
+#
+#  Version $Id$
+#
+#  Tests for deeply nested pairs.
+#
+#  The PAIR_VERIFY macros were recursive, perhaps exponential.  So the tests here would take gradually more
+#  and more time, to more than 30+s.  That's bad.
+#
+
+proto dhcpv6
+proto-dictionary dhcpv6
+fuzzer-out dhcpv6
+
+decode-pair 00 5e 00 04 6a 2b 00 00
+match S46-MAP-Cont-E = { Options = { raw.27179 = 0x } }
+
+decode-pair ff 0f 00 01 fa 
+match raw.65295 = 0xfa
+
+decode-pair 00 00 00 02 00 04
+match raw.0 = 0x0004
+
+decode-pair 00 5f 00 0c 00 5e 00 08 00 5e 00 04 6a 2b 00 00 40 1a 00 00
+match S46-MAP-Cont-T = { Options = { S46-MAP-Cont-E = { Options = { S46-MAP-Cont-E = { Options = { raw.27179 = 0x } } } } } }, raw.16410 = 0x
+
+decode-pair 00 5e 00 10 00 5f 00 0c 00 5e 00 08 00 5e 00 04 6a 2b 00 00 40 1a 00 00
+match S46-MAP-Cont-E = { Options = { S46-MAP-Cont-T = { Options = { S46-MAP-Cont-E = { Options = { S46-MAP-Cont-E = { Options = { raw.27179 = 0x } } } } } } } }, raw.16410 = 0x
+
+decode-pair 00 5e 00 14 00 5e 00 10 00 5f 00 0c 00 5e 00 08 00 5e 00 04 6a 2b 00 00 40 1a 00 00
+match S46-MAP-Cont-E = { Options = { S46-MAP-Cont-E = { Options = { S46-MAP-Cont-T = { Options = { S46-MAP-Cont-E = { Options = { S46-MAP-Cont-E = { Options = { raw.27179 = 0x } } } } } } } } } }, raw.16410 = 0x
+
+decode-pair 00 5e 00 18 00 5e 00 14 00 5e 00 10 00 5f 00 0c 00 5e 00 08 00 5e 00 04 6a 2b 00 00 40 1a 00 00
+match S46-MAP-Cont-E = { Options = { S46-MAP-Cont-E = { Options = { S46-MAP-Cont-E = { Options = { S46-MAP-Cont-T = { Options = { S46-MAP-Cont-E = { Options = { S46-MAP-Cont-E = { Options = { raw.27179 = 0x } } } } } } } } } } } }, raw.16410 = 0x
+
+decode-pair 00 5e 00 1c 00 5e 00 18 00 5e 00 14 00 5e 00 10 00 5f 00 0c 00 5e 00 08 00 5e 00 04 6a 2b 00 00 40 1a 00 00
+match S46-MAP-Cont-E = { Options = { S46-MAP-Cont-E = { Options = { S46-MAP-Cont-E = { Options = { S46-MAP-Cont-E = { Options = { S46-MAP-Cont-T = { Options = { S46-MAP-Cont-E = { Options = { S46-MAP-Cont-E = { Options = { raw.27179 = 0x } } } } } } } } } } } } } }, raw.16410 = 0x
+
+decode-pair 00 5e 00 20 00 5e 00 1c 00 5e 00 18 00 5e 00 14 00 5e 00 10 00 5f 00 0c 00 5e 00 08 00 5e 00 04 6a 2b 00 00 40 1a 00 00
+match S46-MAP-Cont-E = { Options = { S46-MAP-Cont-E = { Options = { S46-MAP-Cont-E = { Options = { S46-MAP-Cont-E = { Options = { S46-MAP-Cont-E = { Options = { S46-MAP-Cont-T = { Options = { S46-MAP-Cont-E = { Options = { S46-MAP-Cont-E = { Options = { raw.27179 = 0x } } } } } } } } } } } } } } } }, raw.16410 = 0x
+
+decode-pair 00 5e 00 24 00 5e 00 20 00 5e 00 1c 00 5e 00 18 00 5e 00 14 00 5e 00 10 00 5f 00 0c 00 5e 00 08 00 5e 00 04 6a 2b 00 00 40 1a 00 00
+match S46-MAP-Cont-E = { Options = { S46-MAP-Cont-E = { Options = { S46-MAP-Cont-E = { Options = { S46-MAP-Cont-E = { Options = { S46-MAP-Cont-E = { Options = { S46-MAP-Cont-E = { Options = { S46-MAP-Cont-T = { Options = { S46-MAP-Cont-E = { Options = { S46-MAP-Cont-E = { Options = { raw.27179 = 0x } } } } } } } } } } } } } } } } } }, raw.16410 = 0x
+
+decode-pair 00 5e 00 28 00 5e 00 24 00 5e 00 20 00 5e 00 1c 00 5e 00 18 00 5e 00 14 00 5e 00 10 00 5f 00 0c 00 5e 00 08 00 5e 00 04 6a 2b 00 00 40 1a 00 00
+match S46-MAP-Cont-E = { Options = { S46-MAP-Cont-E = { Options = { S46-MAP-Cont-E = { Options = { S46-MAP-Cont-E = { Options = { S46-MAP-Cont-E = { Options = { S46-MAP-Cont-E = { Options = { S46-MAP-Cont-E = { Options = { S46-MAP-Cont-T = { Options = { S46-MAP-Cont-E = { Options = { S46-MAP-Cont-E = { Options = { raw.27179 = 0x } } } } } } } } } } } } } } } } } } } }, raw.16410 = 0x
+
+#  00 5e 00 30
+#    00 5e 00 2c
+#      00 5e 00 24
+#        00 5e 00 20
+#          00 5e 00 1c
+#            00 5e 00 18
+#              00 5f 00 14
+#                00 5e 00 10
+#                  00 5f 00 0c
+#                    00 5e 00 08
+#                      00 5e 00 04
+#                        6a 2b 00 00 40 1a 00 00
+
+decode-pair 00 5e 00 30 00 5e 00 2c 00 5e 00 24 00 5e 00 20 00 5e 00 1c 00 5e 00 18 00 5f 00 14 00 5e 00 10 00 5f 00 0c 00 5e 00 08 00 5e 00 04 6a 2b 00 00 40 1a 00 00
+match S46-MAP-Cont-E = { Options = { S46-MAP-Cont-E = { Options = { S46-MAP-Cont-E = { Options = { S46-MAP-Cont-E = { Options = { S46-MAP-Cont-E = { Options = { S46-MAP-Cont-E = { Options = { S46-MAP-Cont-T = { Options = { S46-MAP-Cont-E = { Options = { S46-MAP-Cont-T = { Options = { S46-MAP-Cont-E = { Options = { S46-MAP-Cont-E = { Options = { raw.27179 = 0x } } } } } } } } } } } } } } } } } }, raw.16410 = 0x } } } }
+
+count
+match 27