From: Alan T. DeKok <aland@freeradius.org>
Date: Fri, 26 Jan 2024 13:54:24 +0000 (-0500)
Subject: correctly handle inter-dictionary dependencies.
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=70ef31ac1d93a45e13def3f679e2106c96dc880e;p=thirdparty%2Ffreeradius-server.git

correctly handle inter-dictionary dependencies.

when adding them, make the "next" one depend on the one which
loaded it.  Then add the "next" one to the autofree list for the
current dictionary.

When freeing the global context, walk through all of the
dictionaries and free the autoref references first. Then walk through
them again, freeing the externally loaded references.
---

diff --git a/src/lib/util/dict_fixup.c b/src/lib/util/dict_fixup.c
index 03bff81355f..408d78f0b33 100644
--- a/src/lib/util/dict_fixup.c
+++ b/src/lib/util/dict_fixup.c
@@ -256,12 +256,20 @@ static fr_dict_attr_t const *dict_find_or_load_reference(fr_dict_t **dict_def, c
 		p = strchr(name, '.');
 		if (p) *p = '\0';
 
-		if (fr_dict_protocol_afrom_file(&dict, name, NULL, filename) < 0) {
+		/*
+		 *	Load the new dictionary, and mark it as loaded from our dictionary.
+		 */
+		if (fr_dict_protocol_afrom_file(&dict, name, NULL, (*dict_def)->root->name) < 0) {
 			fr_strerror_printf("Unknown protocol '%s' at %s[%d]", name,
 					   fr_cwd_strip(filename), line);
 			return NULL;
 		}
 
+		if (!fr_hash_table_insert((*dict_def)->autoref, dict)) {
+			fr_strerror_const("Failed inserting into internal autoref table");
+			return NULL;
+		}
+
 		/*
 		 *	The reference is to the root of the foreign protocol, we're done.
 		 */
diff --git a/src/lib/util/dict_util.c b/src/lib/util/dict_util.c
index d4a9eb806cf..76321326a99 100644
--- a/src/lib/util/dict_util.c
+++ b/src/lib/util/dict_util.c
@@ -3296,6 +3296,30 @@ bool dict_has_dependents(fr_dict_t *dict)
 	return (fr_rb_num_elements(dict->dependents) > 0);
 }
 
+static int dict_autoref_free(fr_dict_t *dict)
+{
+	fr_dict_t **refd_list;
+	unsigned int i;
+
+	if (!dict->autoref) return 0;
+
+	if (fr_hash_table_flatten(NULL, (void ***)&refd_list, dict->autoref) < 0) {
+		fr_strerror_const("failed flattening autoref hash table");
+		return -1;
+	}
+
+	for (i = 0; i < talloc_array_length(refd_list); i++) {
+		if (fr_dict_free(&refd_list[i], dict->root->name) < 0) {
+			fr_strerror_printf("failed freeing autoloaded protocol %s", refd_list[i]->root->name);
+			return -1;
+		}
+	}
+
+	TALLOC_FREE(dict->autoref);
+
+	return 0;
+}
+
 static int _dict_free(fr_dict_t *dict)
 {
 	/*
@@ -3312,7 +3336,10 @@ static int _dict_free(fr_dict_t *dict)
 	}
 
 #ifdef STATIC_ANALYZER
-	if (!dict->root) return -1;
+	if (!dict->root) {
+		fr_strerror_const("dict root is missing");
+		return -1;
+	}
 #endif
 
 	if (!fr_cond_assert(!dict->in_protocol_by_name || fr_hash_table_delete(dict->gctx->protocol_by_name, dict))) {
@@ -3338,9 +3365,12 @@ static int _dict_free(fr_dict_t *dict)
 		     dep = fr_rb_iter_next_inorder(&iter)) {
 			fr_strerror_printf_push("%s (%u)", dep->dependent, dep->count);
 		}
+
 		return -1;
 	}
 
+	if (dict_autoref_free(dict) < 0) return -1;
+
 	/*
 	 *	Free the hash tables with free functions first
 	 *	so that the things the hash tables reference
@@ -3348,19 +3378,6 @@ static int _dict_free(fr_dict_t *dict)
 	 */
 	talloc_free(dict->vendors_by_name);
 
-	if (dict->autoref) {
-		fr_dict_t **refd_list;
-		unsigned int i;
-
-		if (fr_hash_table_flatten(NULL, (void ***)&refd_list, dict->autoref) < 0) return -1;
-
-		for (i = 0; i < talloc_array_length(refd_list); i++) {
-		     if (fr_dict_free(&refd_list[i], dict->root->name) < 0) return -1;
-		}
-
-		talloc_free(refd_list);
-	}
-
 	/*
 	 *	Decrease the reference count on the validation
 	 *	library we loaded.
@@ -3893,13 +3910,29 @@ static int _dict_global_free(fr_dict_gctx_t *gctx)
 	    	if (talloc_free(gctx->internal) < 0) still_loaded = true;
 	}
 
+	/*
+	 *	Free up autorefs first, which will free up inter-dictionary dependencies.
+	 */
+	for (dict = fr_hash_table_iter_init(gctx->protocol_by_name, &iter);
+	     dict;
+	     dict = fr_hash_table_iter_next(gctx->protocol_by_name, &iter)) {
+		(void)talloc_get_type_abort(dict, fr_dict_t);
+
+		if (dict_autoref_free(dict) < 0) return -1;
+	}
+
 	for (dict = fr_hash_table_iter_init(gctx->protocol_by_name, &iter);
 	     dict;
 	     dict = fr_hash_table_iter_next(gctx->protocol_by_name, &iter)) {
 	     	(void)talloc_get_type_abort(dict, fr_dict_t);
 	     	dict_dependent_remove(dict, "global");			/* remove our dependency */
 
-	    	if (talloc_free(dict) < 0) still_loaded = true;
+		if (talloc_free(dict) < 0) {
+#ifndef NDEBUG
+			FR_FAULT_LOG("gctx failed to free dictionary %s - %s", dict->root->name, fr_strerror());
+#endif
+			still_loaded = true;
+		}
 	}
 
 	if (still_loaded) {