Fix use-after-free when destroying interfaces on shutdown

main/module.c

@@ -11,6 +11,7 @@
 
 #include <assert.h>
 #include <fnmatch.h>
+#include <string.h>
 #include <sys/queue.h>
 
 static STAILQ_HEAD(, gr_api_handler) handlers = STAILQ_HEAD_INITIALIZER(handlers);
@@ -61,11 +62,21 @@ void gr_register_module(struct gr_module *mod) {
 static bool module_is_child(const void *mod, const void *maybe_child) {
 	const struct gr_module *c = maybe_child;
 	const struct gr_module *m = mod;
+	char depends_on[512];
 
 	if (c->depends_on == NULL)
 		return false;
 
-	return fnmatch(c->depends_on, m->name, 0) == 0;
+	// split on commas
+	assert(strlen(c->depends_on) < sizeof(depends_on));
+	memccpy(depends_on, c->depends_on, 0, sizeof(depends_on));
+
+	for (char *dep = strtok(depends_on, ","); dep != NULL; dep = strtok(NULL, ",")) {
+		if (fnmatch(dep, m->name, 0) == 0)
+			return true;
+	}
+
+	return false;
 }
 
 void modules_init(struct event_base *ev_base) {

modules/infra/control/control_output.c

@@ -129,7 +129,7 @@ static void control_output_fini(struct event_base *) {
 
 static struct gr_module control_output_module = {
 	.name = "control_output",
-	.depends_on = "*",
+	.depends_on = "graph",
 	.init = control_output_init,
 	.fini = control_output_fini,
 };

modules/infra/control/graph.c

@@ -579,7 +579,6 @@ static void graph_fini(struct event_base *) {
 
 static struct gr_module graph_module = {
 	.name = "graph",
-	.depends_on = "iface",
 	.init = graph_init,
 	.fini = graph_fini,
 };

modules/infra/control/iface.c

@@ -469,7 +469,7 @@ static void iface_fini(struct event_base *) {
 
 static struct gr_module iface_module = {
 	.name = "iface",
-	.depends_on = "*route",
+	.depends_on = "*route,control_output",
 	.init = iface_init,
 	.fini = iface_fini,
 };

modules/infra/control/nexthop.c

@@ -693,7 +693,7 @@ static struct gr_event_serializer nh_serializer = {
 
 static struct gr_module module = {
 	.name = "nexthop",
-	.depends_on = "rcu",
+	.depends_on = "rcu,control_output",
 	.init = nh_init,
 	.fini = nh_fini,
 };

smoke/_init.sh

@@ -41,18 +41,21 @@ cleanup() {
 	status="$?"
 	set +e
 	sh -x $tmp/cleanup
-	# delete all non-port interfaces first
-	grcli interface show |
-	grep -Ev -e ^NAME -e '\<port[[:space:]]+devargs=' -e '\<loopback\>' |
-	while read -r name _; do
-		grcli interface del "$name"
-	done
-	# then delete all ports
-	grcli interface show |
-	grep -ve ^NAME -e '\<loopback\>' |
-	while read -r name _; do
-		grcli interface del "$name"
-	done
+
+	if socat FILE:/dev/null UNIX-CONNECT:$GROUT_SOCK_PATH 2>/dev/null; then
+		# delete all non-port interfaces first
+		grcli interface show |
+		grep -Ev -e ^NAME -e '\<port[[:space:]]+devargs=' -e '\<loopback\>' |
+		while read -r name _; do
+			grcli interface del "$name"
+		done
+		# then delete all ports
+		grcli interface show |
+		grep -ve ^NAME -e '\<loopback\>' |
+		while read -r name _; do
+			grcli interface del "$name"
+		done
+	fi
 	[ -s $tmp/restore_interfaces ] && sh -x $tmp/restore_interfaces
 
 	kill %?grcli

smoke/shutdown_test.sh

@@ -0,0 +1,38 @@
+#!/bin/bash
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright (c) 2024 Robin Jarry
+
+. $(dirname $0)/_init.sh
+
+grcli -xe <<EOF
+interface add port p0 devargs net_null0,no-rx=1 vrf 1
+interface add port p1 devargs net_null1,no-rx=1 vrf 2
+nexthop add l3 iface p0 id 42 address 1.2.3.4
+nexthop add l3 iface p0 id 45
+nexthop add l3 iface p0 id 47 address 1.2.3.7
+nexthop add l3 iface p1 id 1042 address f00:ba4::1
+nexthop add l3 iface p1 id 1047 address f00:ba4::100
+nexthop add l3 iface p0 address ba4:f00::1 mac ba:d0:ca:ca:00:02
+nexthop add l3 iface p1 address 4.3.2.1 mac ba:d0:ca:ca:00:01
+nexthop add blackhole id 666 vrf 1
+nexthop add reject id 123456 vrf 2
+address add 10.0.0.1/24 iface p0
+address add 10.1.0.1/24 iface p1
+route add 0.0.0.0/0 via 10.0.0.2 vrf 1
+route add 4.5.21.2/27 via id 47 vrf 1
+route add 172.16.47.0/24 via id 1047 vrf 2
+address add 2345::1/24 iface p0
+address add 2346::1/24 iface p1
+route add ::/0 via 2345::2 vrf 1
+route add 2521:111::4/37 via id 1047 vrf 1
+route add 2521:112::/64 via id 45 vrf 2
+route add 2521:113::/64 via id 47 vrf 1
+interface set port p0 rxqs 2
+interface set port p1 rxqs 2
+EOF
+
+kill -INT "$grout_pid"
+wait "$grout_pid" || fail "grout crashed on shutdown"
+
+# grout has been shutdown, avoid running any grcli command
+sed -i '/^grcli/d' $tmp/cleanup