From: Arran Cudbard-Bell <a.cudbardb@freeradius.org>
Date: Tue, 29 Jun 2021 17:17:33 +0000 (-0500)
Subject: Add namesapce/hashtable validation
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=7d4253b5525cf5ce355d8afa1c487786ca8ddd5e;p=thirdparty%2Ffreeradius-server.git

Add namesapce/hashtable validation
---

diff --git a/src/lib/util/dict_util.c b/src/lib/util/dict_util.c
index e88028cfaec..b5853b6f330 100644
--- a/src/lib/util/dict_util.c
+++ b/src/lib/util/dict_util.c
@@ -484,7 +484,8 @@ static inline CC_HINT(always_inline) int dict_attr_namespace_init(fr_dict_attr_t
 	 *	namespace hash table.
 	 */
 	if (!ext->namespace) {
-		ext->namespace = fr_hash_table_alloc(*da_p, dict_attr_name_hash, dict_attr_name_cmp, NULL);
+		ext->namespace = fr_hash_table_talloc_alloc(*da_p, fr_dict_attr_t,
+							    dict_attr_name_hash, dict_attr_name_cmp, NULL);
 		if (!ext->namespace) {
 			fr_strerror_printf("Failed allocating \"namespace\" table");
 			return -1;
@@ -1261,13 +1262,15 @@ int dict_attr_enum_add_name(fr_dict_attr_t *da, char const *name,
 	 *	Initialise enumv hash tables
 	 */
 	if (!ext->value_by_name || !ext->name_by_value) {
-		ext->value_by_name = fr_hash_table_alloc(da, dict_enum_name_hash, dict_enum_name_cmp, hash_pool_free);
+		ext->value_by_name = fr_hash_table_talloc_alloc(da, fr_dict_enum_t, dict_enum_name_hash,
+								dict_enum_name_cmp, hash_pool_free);
 		if (!ext->value_by_name) {
 			fr_strerror_printf("Failed allocating \"value_by_name\" table");
 			return -1;
 		}
 
-		ext->name_by_value = fr_hash_table_alloc(da, dict_enum_value_hash, dict_enum_value_cmp, NULL);
+		ext->name_by_value = fr_hash_table_talloc_alloc(da, fr_dict_enum_t, dict_enum_value_hash,
+								dict_enum_value_cmp, NULL);
 		if (!ext->name_by_value) {
 			fr_strerror_printf("Failed allocating \"name_by_value\" table");
 			return -1;
@@ -3839,7 +3842,7 @@ void fr_dict_attr_verify(char const *file, int line, fr_dict_attr_t const *da)
 		/*
 		 *	Check the namespace hash table is ok
 		 */
-		(void)talloc_get_type_abort(dict_attr_namespace(da), fr_hash_table_t);
+		fr_hash_table_verify(dict_attr_namespace(da));
 	}
 		break;
 
diff --git a/src/lib/util/hash.c b/src/lib/util/hash.c
index 454e1fc1078..a90c707167e 100644
--- a/src/lib/util/hash.c
+++ b/src/lib/util/hash.c
@@ -49,18 +49,19 @@ struct fr_hash_entry_s {
 };
 
 struct fr_hash_table_s {
-	uint32_t		num_elements;
-	uint32_t		num_buckets; /* power of 2 */
+	uint32_t		num_elements;	//!< Number of elements in the hash table.
+	uint32_t		num_buckets;	//!< Number of buckets (how long the array is) - power of 2 */
 	uint32_t		next_grow;
 	uint32_t		mask;
 
-	fr_free_t		free;
-	fr_hash_t		hash;
-	fr_cmp_t		cmp;
+	fr_free_t		free;		//!< Data free function.
+	fr_hash_t		hash;		//!< Hashing function.
+	fr_cmp_t		cmp;		//!< Comparison function.
 
-	fr_hash_entry_t		null;
+	char const		*type;		//!< Talloc type to check elements against.
 
-	fr_hash_entry_t		**buckets;
+	fr_hash_entry_t		null;
+	fr_hash_entry_t		**buckets;	//!< Array of hash buckets.
 };
 
 #ifdef TESTING
@@ -277,32 +278,35 @@ static int _fr_hash_table_free(fr_hash_table_t *ht)
  *
  *	Memory usage in bytes is (20/3) * number of entries.
  */
-fr_hash_table_t *fr_hash_table_alloc(TALLOC_CTX *ctx,
-				     fr_hash_t hash_func,
-				     fr_cmp_t cmp_func,
-				     fr_free_t free_func)
+fr_hash_table_t *_fr_hash_table_alloc(TALLOC_CTX *ctx,
+				      char const *type,
+				      fr_hash_t hash_func,
+				      fr_cmp_t cmp_func,
+				      fr_free_t free_func)
 {
 	fr_hash_table_t *ht;
 
-	ht = talloc_zero(ctx, fr_hash_table_t);
+	ht = talloc(ctx, fr_hash_table_t);
 	if (!ht) return NULL;
 	talloc_set_destructor(ht, _fr_hash_table_free);
 
-	ht->free = free_func;
-	ht->hash = hash_func;
-	ht->cmp = cmp_func;
-	ht->num_buckets = FR_HASH_NUM_BUCKETS;
-	ht->mask = ht->num_buckets - 1;
-
-	/*
-	 *	Have a default load factor of 2.5.  In practice this
-	 *	means that the average load will hit 3 before the
-	 *	table grows.
-	 */
-	ht->next_grow = (ht->num_buckets << 1) + (ht->num_buckets >> 1);
+	*ht = (fr_hash_table_t){
+		.type = type,
+		.free = free_func,
+		.hash = hash_func,
+		.cmp = cmp_func,
+		.num_buckets = FR_HASH_NUM_BUCKETS,
+		.mask = FR_HASH_NUM_BUCKETS - 1,
 
-	ht->buckets = talloc_zero_array(ht, fr_hash_entry_t *, ht->num_buckets);
-	if (!ht->buckets) {
+		/*
+		 *	Have a default load factor of 2.5.  In practice this
+		 *	means that the average load will hit 3 before the
+		 *	table grows.
+		 */
+		.next_grow = (FR_HASH_NUM_BUCKETS << 1) + (FR_HASH_NUM_BUCKETS >> 1),
+		.buckets = talloc_zero_array(ht, fr_hash_entry_t *, FR_HASH_NUM_BUCKETS)
+	};
+	if (unlikely(!ht->buckets)) {
 		talloc_free(ht);
 		return NULL;
 	}
@@ -467,6 +471,10 @@ bool fr_hash_table_insert(fr_hash_table_t *ht, void const *data)
 	uint32_t		reversed;
 	fr_hash_entry_t		*node;
 
+#ifndef TALLOC_GET_TYPE_ABORT_NOOP
+	if (ht->type) (void)_talloc_get_type_abort(data, ht->type, __location__);
+#endif
+
 	key = ht->hash(data);
 	entry = key & ht->mask;
 	reversed = reverse(key);
@@ -876,6 +884,31 @@ uint32_t fr_hash_case_string(char const *p)
 	return hash;
 }
 
+/** Check hash table is sane
+ *
+ */
+void fr_hash_table_verify(fr_hash_table_t *ht)
+{
+	fr_hash_iter_t	iter;
+	void		*ptr;
+
+	(void)talloc_get_type_abort(ht, fr_hash_table_t);
+	(void)talloc_get_type_abort(ht->buckets, fr_hash_entry_t *);
+
+	fr_assert(talloc_array_length(ht->buckets) == ht->num_buckets);
+
+	/*
+	 *	Check talloc headers on all data
+	 */
+	if (ht->type) {
+		for (ptr = fr_hash_table_iter_init(ht, &iter);
+		     ptr;
+		     ptr = fr_hash_table_iter_next(ht, &iter)) {
+			(void)_talloc_get_type_abort(ptr, ht->type, __location__);
+		}
+	}
+}
+
 #ifdef TESTING
 /*
  *  cc -g -DTESTING -I ../include hash.c -o hash
diff --git a/src/lib/util/hash.h b/src/lib/util/hash.h
index 0b3d75c887a..d5c78ab4ef3 100644
--- a/src/lib/util/hash.h
+++ b/src/lib/util/hash.h
@@ -55,10 +55,17 @@ uint32_t fr_hash_case_string(char const *p);
 typedef struct fr_hash_table_s fr_hash_table_t;
 typedef int (*fr_hash_table_walk_t)(void *data, void *uctx);
 
-fr_hash_table_t *fr_hash_table_alloc(TALLOC_CTX *ctx,
-				     fr_hash_t hash_node,
-				     fr_cmp_t cmp_node,
-				     fr_free_t free_node) CC_HINT(nonnull(2,3));
+#define		fr_hash_table_alloc(_ctx, _hash_node, _cmp_node, _free_node) \
+		_fr_hash_table_alloc(_ctx, NULL, _hash_node, _cmp_node, _free_node)
+
+#define		fr_hash_table_talloc_alloc(_ctx, _type, _hash_node, _cmp_node, _free_node) \
+		_fr_hash_table_alloc(_ctx, #_type, _hash_node, _cmp_node, _free_node)
+
+fr_hash_table_t *_fr_hash_table_alloc(TALLOC_CTX *ctx,
+				      char const *type,
+				      fr_hash_t hash_node,
+				      fr_cmp_t cmp_node,
+				      fr_free_t free_node) CC_HINT(nonnull(3,4));
 
 void		*fr_hash_table_find(fr_hash_table_t *ht, void const *data) CC_HINT(nonnull);
 
@@ -82,6 +89,8 @@ int		fr_hash_table_flatten(TALLOC_CTX *ctx, void **out[], fr_hash_table_t *ht) C
 
 void		fr_hash_table_fill(fr_hash_table_t *ht) CC_HINT(nonnull);
 
+void		fr_hash_table_verify(fr_hash_table_t *ht);
+
 #ifdef __cplusplus
 }
 #endif