From: Alan T. DeKok <aland@freeradius.org>
Date: Sat, 3 Feb 2024 14:05:14 +0000 (-0500)
Subject: allow for loops in foreign dictionaries
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=62716fa56dbd1fda4724cf4aed1da6041e7d810f;p=thirdparty%2Ffreeradius-server.git

allow for loops in foreign dictionaries

by adding two flags, "loading", which is set in begin-proto,
and cleared when the dictionary is done.  And "loaded" which is
set by fr_protocol_afrom_file(), to indicate that it has loaded
the dictionary file. and initialized the protocol library.

The dict routines now call proto->init() and proto->free(), so that
when a protocol library is loaded, everything it needs is also
loaded and initialized.
---

diff --git a/src/lib/util/dict_fixup.c b/src/lib/util/dict_fixup.c
index c8042b331c9..c88e559812a 100644
--- a/src/lib/util/dict_fixup.c
+++ b/src/lib/util/dict_fixup.c
@@ -273,20 +273,6 @@ static fr_dict_attr_t const *dict_protocol_reference(fr_dict_t **dict_def, char
 			return NULL;
 		}
 
-		/*
-		 *	If we call init(), make sure that we call free();
-		 */
-		if (dict->proto && dict->proto->init) {
-			if (dict->proto->init() < 0) return NULL;
-
-			/*
-			 *	Mark the *referencing* dictionary as autofree.
-			 *
-			 *	Changing this to dict->autofree=true will break things. :(
-			 */
-			(*dict_def)->autofree = true;
-		}
-
 		/*
 		 *	The reference is to the root of the foreign protocol, we're done.
 		 */
diff --git a/src/lib/util/dict_priv.h b/src/lib/util/dict_priv.h
index d204f0a2fcc..ffe704f9280 100644
--- a/src/lib/util/dict_priv.h
+++ b/src/lib/util/dict_priv.h
@@ -80,11 +80,11 @@ struct fr_dict {
 	bool			in_protocol_by_num;	//!< Whether the dictionary has been inserted into the
 							//!< protocol_by_num table.
 
-	bool			autoloaded;		//!< manual vs autoload
-
 	bool			string_based;		//!< TACACS, etc.
 
-	bool			autofree;		//!< from dict_fixup.
+	bool			loading;		//!< from fr_dict_protocol_afrom_file();
+
+	bool			loaded;			//!< from fr_dict_protocol_afrom_file();
 
 	fr_hash_table_t		*vendors_by_name;	//!< Lookup vendor by name.
 	fr_hash_table_t		*vendors_by_num;	//!< Lookup vendor by PEN.
diff --git a/src/lib/util/dict_tokenize.c b/src/lib/util/dict_tokenize.c
index bb99d93724e..360335c5079 100644
--- a/src/lib/util/dict_tokenize.c
+++ b/src/lib/util/dict_tokenize.c
@@ -2363,11 +2363,11 @@ static int _dict_from_file(dict_tokenize_ctx_t *ctx,
 			 */
 			dict_fixup_init(NULL, &ctx->fixup);
 
-			// check if there's a linked library for the
-			// protocol.  The values can be unknown (we
-			// try to load one), or non-existent, or
-			// known.  For the last two, we don't try to
-			// load anything.
+			/*
+			 *	We're in the middle of loading this dictionary.  Tell
+			 *	fr_dict_protocol_afrom_file() to suppress recursive references.
+			 */
+			found->loading = true;
 
 			ctx->dict = found;
 
@@ -2909,10 +2909,28 @@ int fr_dict_protocol_afrom_file(fr_dict_t **out, char const *proto_name, char co
 	 *	has already been loaded and return that.
 	 */
 	dict = dict_by_protocol_name(proto_name);
-	if (dict && dict->autoloaded) {
-		 dict_dependent_add(dict, dependent);
-		*out = dict;
-		return 0;
+	if (dict) {
+		/*
+		 *	If we're in the middle of loading this dictionary, then the only way we get back here
+		 *	is via a circular reference.  So we catch that, and drop the circular dependency.
+		 *
+		 *	When we have A->B->A, it means that we don't need to track B->A, because we track
+		 *	A->B.  And if A is freed, then B is freed.
+		 */
+		if (!dict->loading) dict_dependent_add(dict, dependent);
+
+		/*
+		 *	But we only return a pre-existing dict if _this function_ has loaded it.
+		 */
+		if (dict->loaded) {
+			*out = dict;
+			return 0;
+		}
+
+		/*
+		 *	Set the flag to true _before_ loading the file.  That prevents recursion.
+		 */
+		dict->loaded = true;
 	}
 
 	if (!proto_dir) {
@@ -2934,6 +2952,7 @@ int fr_dict_protocol_afrom_file(fr_dict_t **out, char const *proto_name, char co
 	 */
 	if (dict_from_file(dict_gctx->internal, dict_dir, FR_DICTIONARY_FILE, NULL, 0) < 0) {
 	error:
+		dict->loading = false;
 		talloc_free(dict_dir);
 		return -1;
 	}
@@ -2947,13 +2966,15 @@ int fr_dict_protocol_afrom_file(fr_dict_t **out, char const *proto_name, char co
 		goto error;
 	}
 
-	talloc_free(dict_dir);
-
 	/*
-	 *	If we're autoloading a previously defined dictionary,
-	 *	then mark up the dictionary as now autoloaded.
+	 *	Initialize the library.
 	 */
-	if (!dict->autoloaded) dict->autoloaded = true;
+	if (dict->proto && dict->proto->init) {
+		if (dict->proto->init() < 0) goto error;
+	}
+	dict->loading = false;
+
+	talloc_free(dict_dir);
 
 	dict_dependent_add(dict, dependent);
 
diff --git a/src/lib/util/dict_util.c b/src/lib/util/dict_util.c
index 417e2047956..68762f619b0 100644
--- a/src/lib/util/dict_util.c
+++ b/src/lib/util/dict_util.c
@@ -3347,7 +3347,7 @@ static int _dict_free(fr_dict_t *dict)
 	/*
 	 *	If we called init(), then call free()
 	 */
-	if (dict->autofree) {
+	if (dict->loaded) {
 		fr_assert(dict->proto);
 		fr_assert(dict->proto->free);
 		dict->proto->free();