til_module_context: untap early in module_context_free()

This was a UAF bug when the context being freed contained a
driving tap... just careless placement of the untap call.

It needs to occur before destroying the context, because module
contexts are often used to store the til_tap_t instances.

1 files changed, 11 insertions, 7 deletions

diff --git a/src/til_module_context.c b/src/til_module_context.c
index 8b13410..b11aad2 100644
--- a/src/til_module_context.c
+++ b/src/til_module_context.c
@@ -72,6 +72,17 @@ static inline void * module_context_free(til_module_context_t *module_context)
 	stream = module_context->stream;
 	setup = module_context->setup;
 
+	/* cleanup any pipes this context might have had in the stream.  If the
+	 * module's destroy_context() also does this it's harmlessly idempotent
+	 * besides wasting some cycles.  But by always doing it here, we're sure
+	 * to not leave dangling references.
+	 *
+	 * XXX: note untapping _must_ happen before destroying the context, since
+	 * the context may contain the driving tap and untap accesses it for
+	 * checking ownership.
+	 */
+	til_stream_untap_owner(stream, module_context);
+
 	if (module_context->module->destroy_context)
 		module_context->module->destroy_context(module_context);
 	else
@@ -79,13 +90,6 @@ static inline void * module_context_free(til_module_context_t *module_context)
 
 	(void) til_setup_free(setup); /* free last just in case the module destructor makes use of it */
 
-	/* cleanup any pipes this context might have had in the stream, if the
-	 * module's destroy_context() also does this it's harmlessly idempotent
-	 * besides wasting some cycles.  But by always doing it here, we're sure
-	 * to not leave dangling references.
-	 */
-	til_stream_untap_owner(stream, module_context);
-
 	return NULL;
 }