From: Arran Cudbard-Bell <a.cudbardb@freeradius.org>
Date: Fri, 9 Jun 2023 15:22:02 +0000 (-0400)
Subject: subrequest:  Don't crash in debug builds when a detached subrequest is about to exit
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=68b38f5eb2ddddfba93b87d4ad19fc3f8ff189be;p=thirdparty%2Ffreeradius-server.git

subrequest:  Don't crash in debug builds when a detached subrequest is about to exit
---

diff --git a/src/lib/unlang/subrequest_child.c b/src/lib/unlang/subrequest_child.c
index b53f5fd2189..d3c586cf5ae 100644
--- a/src/lib/unlang/subrequest_child.c
+++ b/src/lib/unlang/subrequest_child.c
@@ -168,13 +168,33 @@ static void unlang_subrequest_child_signal(request_t *request, fr_signal_t actio
 static unlang_action_t unlang_subrequest_child_done(rlm_rcode_t *p_result,
 						    UNUSED int *p_priority, request_t *request, void *uctx)
 {
-	unlang_frame_state_subrequest_t	*state = talloc_get_type_abort(uctx, unlang_frame_state_subrequest_t);
+	unlang_frame_state_subrequest_t	*state;
 
 	/*
 	 *	Child was detached, nothing more to do.
+	 *
+	 *	This frame was left on the stack when the child
+	 *	detached.  It's normally meant to trigger a
+	 *	resume in the parent, but as there is no parent
+	 *	it just becomes a noop and gets popped.
+	 *
+	 *	This is cheaper/less complex then rooting it
+	 *	out at detach time and unsetting the repeat
+	 *	function.
 	 */
 	if (!request->parent) return UNLANG_ACTION_CALCULATE_RESULT;
 
+	/*
+	 *	'state' in this context, is the frame state
+	 *	in the parent request, so we cannot examine it
+	 *	until AFTER we've determined there is still
+	 *	a parent, else the memory could've been freed.
+	 *
+	 *	i.e. don't move the get_type_abort call onto
+	 *	the same line as the state variable declaration.
+	 */
+	state = talloc_get_type_abort(uctx, unlang_frame_state_subrequest_t);
+
 	/*
 	 *	Place child state back inside the parent
 	 */
diff --git a/src/tests/keywords/subrequest-detach-parallel-redundant b/src/tests/keywords/subrequest-detach-parallel-redundant
new file mode 100644
index 00000000000..3a6ae41d736
--- /dev/null
+++ b/src/tests/keywords/subrequest-detach-parallel-redundant
@@ -0,0 +1,22 @@
+#
+#  PRE: subrequest
+#
+
+# This is mostly a regression test, if it doesn't crash we're good
+subrequest {
+	# At this point the main request continues
+	detach
+
+	parallel {
+		redundant {
+			ok
+			fail
+		}
+		redundant {
+			fail
+			ok
+		}
+	}
+}
+
+success