Re: ospfd and ripd dump core on exit

On Mon, Dec 08, 2025 at 10:04:21PM +0300, Alexander Mukhin wrote:
> On Thu, Dec 04, 2025 at 04:51:55PM +0300, Alexander Mukhin wrote:
> > >Synopsis:	ospfd and ripd dump core on exit
> > >Category:	user
> > >Environment:
> > 	System      : OpenBSD 7.8
> > 	Details     : OpenBSD 7.8 (GENERIC) #54: Sun Oct 12 12:45:58 MDT 2025
> > 			 [email protected]:/usr/src/sys/arch/amd64/compile/GENERIC
> > 
> > 	Architecture: OpenBSD.amd64
> > 	Machine     : amd64
> > >Description:
> > 	ospfd and ripd dump core on exit when static redistribution is enabled
> > >How-To-Repeat:
> > 	# cat ospfd.conf
> > 	redistribute static
> > 	area 0.0.0.0 {
> > 		interface vio1
> > 	}
> > 	# route add ...
> > 	# ospfd -d -v -f ospfd.conf
> > 	...
> > 	^C
> > 	...
> > 	Segmentation fault (core dumped)
> > 
> > 	# cat ripd.conf
> > 	redistribute static
> > 	interface vio1
> > 	# route add ...
> > 	# ripd -d -v -f ripd.conf
> > 	...
> > 	^C
> > 	...
> > 	Segmentation fault (core dumped)
> > 
> > >Fix:
> > 	The reason is the same in both cases - sending imsgs through
> > 	closed pipes. Calling kr_shutdown() before closing pipes helps.
> > 	The proper fix is probably something like Claudio Jeker did
> > 	for ldpd.
> 
> Patches:
> 
> --- ospfd.c
> +++ ospfd.c
> @@ -313,6 +313,8 @@
>  	int			 status;
>  	struct redistribute	*r;
>  
> +	kr_shutdown();
> +
>  	/* close pipes */
>  	imsgbuf_clear(&iev_ospfe->ibuf);
>  	close(iev_ospfe->ibuf.fd);
> @@ -324,7 +326,6 @@
>  		SIMPLEQ_REMOVE_HEAD(&ospfd_conf->redist_list, entry);
>  		free(r);
>  	}
> -	kr_shutdown();
>  	carp_demote_shutdown();
>  
>  	log_debug("waiting for children to terminate");
> 
> 
> --- ripd.c
> +++ ripd.c
> @@ -271,6 +271,8 @@
>  	pid_t		 pid;
>  	int		 status;
>  
> +	kr_shutdown();
> +
>  	/* close pipes */
>  	imsgbuf_clear(&iev_ripe->ibuf);
>  	close(iev_ripe->ibuf.fd);
> @@ -281,8 +283,6 @@
>  		LIST_REMOVE(i, entry);
>  		if_del(i);
>  	}
> -
> -	kr_shutdown();
>  
>  	log_debug("waiting for children to terminate");
>  	do {
> 

Thanks Alexander for your patch.

I would suggest to the following diff.  So, the code is more like the
ospfe_shutdown() in ospfe.c.  Closing the imsg buffer after disabling
the timers, also prevents a race here, because the timer functions also
dealing with imsg stuff.  And the problem is also solved for ospf6d(8).

ok?

Index: usr.sbin/ospf6d/ospf6d.c
===================================================================
RCS file: /cvs/src/usr.sbin/ospf6d/ospf6d.c,v
diff -u -p -r1.61 ospf6d.c
--- usr.sbin/ospf6d/ospf6d.c	21 Nov 2024 13:38:14 -0000	1.61
+++ usr.sbin/ospf6d/ospf6d.c	5 May 2026 12:21:52 -0000
@@ -301,12 +301,6 @@ ospfd_shutdown(void)
 	pid_t	pid;
 	int	status;
 
-	/* close pipes */
-	imsgbuf_clear(&iev_ospfe->ibuf);
-	close(iev_ospfe->ibuf.fd);
-	imsgbuf_clear(&iev_rde->ibuf);
-	close(iev_rde->ibuf.fd);
-
 	control_cleanup();
 	kr_shutdown();
 	carp_demote_shutdown();
@@ -322,6 +316,12 @@ ospfd_shutdown(void)
 			    (pid == rde_pid) ? "route decision engine" :
 			    "ospf engine", WTERMSIG(status));
 	} while (pid != -1 || (pid == -1 && errno == EINTR));
+
+	/* close pipes */
+	imsgbuf_clear(&iev_ospfe->ibuf);
+	close(iev_ospfe->ibuf.fd);
+	imsgbuf_clear(&iev_rde->ibuf);
+	close(iev_rde->ibuf.fd);
 
 	free(iev_ospfe);
 	free(iev_rde);
Index: usr.sbin/ospfd/ospfd.c
===================================================================
RCS file: /cvs/src/usr.sbin/ospfd/ospfd.c,v
diff -u -p -r1.124 ospfd.c
--- usr.sbin/ospfd/ospfd.c	21 Nov 2024 13:38:14 -0000	1.124
+++ usr.sbin/ospfd/ospfd.c	5 May 2026 12:21:52 -0000
@@ -313,12 +313,6 @@ ospfd_shutdown(void)
 	int			 status;
 	struct redistribute	*r;
 
-	/* close pipes */
-	imsgbuf_clear(&iev_ospfe->ibuf);
-	close(iev_ospfe->ibuf.fd);
-	imsgbuf_clear(&iev_rde->ibuf);
-	close(iev_rde->ibuf.fd);
-
 	control_cleanup();
 	while ((r = SIMPLEQ_FIRST(&ospfd_conf->redist_list)) != NULL) {
 		SIMPLEQ_REMOVE_HEAD(&ospfd_conf->redist_list, entry);
@@ -338,6 +332,12 @@ ospfd_shutdown(void)
 			    (pid == rde_pid) ? "route decision engine" :
 			    "ospf engine", WTERMSIG(status));
 	} while (pid != -1 || (pid == -1 && errno == EINTR));
+
+	/* close pipes */
+	imsgbuf_clear(&iev_ospfe->ibuf);
+	close(iev_ospfe->ibuf.fd);
+	imsgbuf_clear(&iev_rde->ibuf);
+	close(iev_rde->ibuf.fd);
 
 	free(iev_ospfe);
 	free(iev_rde);