From: Arran Cudbard-Bell <a.cudbardb@freeradius.org>
Date: Wed, 10 Nov 2021 21:36:06 +0000 (-0600)
Subject: Keep a pointer to the module instance in the thread instance
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=7817f8fb64faa96d958fe33a6af57c9a7e5ca66b;p=thirdparty%2Ffreeradius-server.git

Keep a pointer to the module instance in the thread instance
---

diff --git a/src/lib/server/module.c b/src/lib/server/module.c
index 0338591ce9..7d1bb328fd 100644
--- a/src/lib/server/module.c
+++ b/src/lib/server/module.c
@@ -1191,15 +1191,15 @@ static int _module_thread_inst_array_free(module_thread_instance_t **array)
 
 		ti = talloc_get_type_abort(array[i], module_thread_instance_t);
 
-		if (ti->module) DEBUG4("Worker cleaning up %s thread instance data (%p/%p)", ti->module->name, ti, ti->data);
+		if (ti->mi) DEBUG4("Worker cleaning up %s thread instance data (%p/%p)",
+				   ti->mi->module->name, ti, ti->data);
 
 		/*
 		 *	Check for ti->module is a hack
 		 *	and should be removed along with
 		 *	starting the instance number at 0
 		 */
-		if (ti->module && ti->module->thread_detach) (void) ti->module->thread_detach(ti->el, ti->data);
-
+		if (ti->mi && ti->mi->module->thread_detach) ti->mi->module->thread_detach(ti->el, ti->data);
 		talloc_free(ti);
 	}
 
@@ -1219,7 +1219,7 @@ static int _module_thread_inst_array_free(module_thread_instance_t **array)
  */
 int modules_thread_instantiate(TALLOC_CTX *ctx, fr_event_list_t *el)
 {
-	void				*instance;
+	void			*instance;
 	fr_rb_iter_inorder_t	iter;
 
 	/*
@@ -1247,8 +1247,7 @@ int modules_thread_instantiate(TALLOC_CTX *ctx, fr_event_list_t *el)
 
 		MEM(ti = talloc_zero(module_thread_inst_array, module_thread_instance_t));
 		ti->el = el;
-		ti->module = mi->module;
-		ti->mod_inst = mi->dl_inst->data;	/* For efficient lookups */
+		ti->mi = mi;
 
 		if (mi->module->thread_inst_size) {
 			MEM(ti->data = talloc_zero_array(ti, uint8_t, mi->module->thread_inst_size));
@@ -1264,7 +1263,7 @@ int modules_thread_instantiate(TALLOC_CTX *ctx, fr_event_list_t *el)
 			}
 		}
 
-		DEBUG4("Worker alloced %s thread instance data (%p/%p)", ti->module->name, ti, ti->data);
+		DEBUG4("Worker alloced %s thread instance data (%p/%p)", ti->mi->module->name, ti, ti->data);
 		if (mi->module->thread_instantiate) {
 			if (mi->module->thread_instantiate(mi->dl_inst->conf, mi->dl_inst->data, el, ti->data) < 0) {
 				PERROR("Thread instantiation failed for module \"%s\"", mi->name);
diff --git a/src/lib/server/module.h b/src/lib/server/module.h
index 8f46855578..f35d79b60e 100644
--- a/src/lib/server/module.h
+++ b/src/lib/server/module.h
@@ -242,12 +242,7 @@ struct module_thread_instance_s {
 
 	fr_event_list_t			*el;		//!< Event list associated with this thread.
 
-	module_t const			*module;	//!< Public module structure.  Cached for convenience,
-							///< and to prevent use-after-free if the global data
-							///< is freed before the thread instance data.
-
-	void				*mod_inst;	//!< Avoids thread_inst->inst->dl_inst->data.
-							///< This is in the hot path, so it makes sense.
+	module_instance_t const		*mi;		//!< As opposed to the thread local inst.
 
 	uint64_t			total_calls;	//! total number of times we've been called
 	uint64_t			active_callers; //! number of active callers.  i.e. number of current yields