Re: ospfd and ripd dump core on exit

Alexander Mukhin <[email protected]> Fri, 8 May 2026 21:17:47 +0300
Newsgroups gmane.os.openbsd.bugs
Message-ID <af4oy274O98ShmwW@hs>
On Tue, May 05, 2026 at 04:04:57PM +0200, Jan Klemkow wrote:
> 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);

Thank you, Jan.
I confirm that your patch fixes the problem for ospfd and ospf6d.

-- 
Alexander.