From 6e9ae3e84486bdc90d6bc0a965ac2a977d438505 Mon Sep 17 00:00:00 2001 From: Vito Caputo Date: Mon, 3 Jul 2023 07:49:40 -0700 Subject: 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. --- src/til_module_context.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) (limited to 'src') 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; } -- cgit v1.2.3