From: Mike Larkin <mlarkin@nested.page>
Subject: Re: httpd,vmd,snmpd,relayd,iked: sync daemon(), don't orphan children
To: Tobias Heider <tobias.heider@stusta.de>
Cc: tech@openbsd.org, Dave Voutila <dv@sisu.io>, kn@openbsd.org
Date: Fri, 5 Apr 2024 19:59:00 -0700

On Thu, Apr 04, 2024 at 11:16:45PM +0200, Tobias Heider wrote:
> On Thu, Apr 04, 2024 at 04:28:24PM -0400, Dave Voutila wrote:
> >
> > Tobias Heider <tobias.heider@stusta.de> writes:
> >
> > > On Thu, Apr 04, 2024 at 07:38:48PM +0200, Florian Obser wrote:
> > >> snmpd and vmd also use proc.c and seem to have similar code in main().
> > >>
> > >> I'm running neither so I have no idea if they have the same problem, but
> > >> it seems likely.
> > >
> > > Right, I don't either. Let me see if I can find more.
> > >
> > > Looks like relayd was already fixed by benno two years ago:
> > > https://github.com/openbsd/src/commit/7a99b37dfd03f1f281b20ab09f836d05794a9da5
> > >
> >
> > vmd definitely does this the way httpd is/was doing it. Happy to look at
> > a diff or will add it to my short term todo
> >
>
> Alright, so here is a diff that syncs all of our proc.c daemons to call daemon()
> in proc_init() for the parent only to not orphan their child processes.
>
> In addition I am also moving the iked call to the same place as in relayd,
> changed the vmd error message to the same as the others and always use fatal()
> instead of err().
>
> ok?
>

ok mlarkin on the vmd(8) parts

-ml

> diff 3b374a56007af68fb6fd1926b370d0a3e8d6dbca 3c269a7b5c44fca594ff2b42f7dc43e37dbe1e49
> commit - 3b374a56007af68fb6fd1926b370d0a3e8d6dbca
> commit + 3c269a7b5c44fca594ff2b42f7dc43e37dbe1e49
> blob - 534427b2b6585d55df1971a56d1e13c99de62612
> blob + a21dda0712a81b38e5afb7aa2ba0f08437226c8d
> --- sbin/iked/proc.c
> +++ sbin/iked/proc.c
> @@ -231,9 +231,10 @@ proc_init(struct privsep *ps, struct privsep_proc *pro
>
>  	if (proc_id == PROC_PARENT) {
>  		privsep_process = PROC_PARENT;
> +		proc_setup(ps, procs, nproc);
> +
>  		if (!debug && daemon(0, 0) == -1)
>  			fatal("failed to daemonize");
> -		proc_setup(ps, procs, nproc);
>
>  		/*
>  		 * Create the children sockets so we can use them
> blob - af863cf2710dadce01a85cb618fe5bfbb91b9adc
> blob + 3cc506f1fb8d0f75deed32fb16b9f5331a0481bb
> --- usr.sbin/httpd/httpd.c
> +++ usr.sbin/httpd/httpd.c
> @@ -220,8 +220,6 @@ main(int argc, char *argv[])
>  	proc_init(ps, procs, nitems(procs), debug, argc0, argv, proc_id);
>
>  	log_procinit("parent");
> -	if (!debug && daemon(1, 0) == -1)
> -		err(1, "failed to daemonize");
>
>  	if (ps->ps_noaction == 0)
>  		log_info("startup");
> blob - e8b08dd23175817590214abb7b433caf541e215d
> blob + 113072d34ffdc78ec79a55d418409842570acce5
> --- usr.sbin/httpd/proc.c
> +++ usr.sbin/httpd/proc.c
> @@ -205,6 +205,9 @@ proc_init(struct privsep *ps, struct privsep_proc *pro
>  		privsep_process = PROC_PARENT;
>  		proc_setup(ps, procs, nproc);
>
> +		if (!debug && daemon(1, 0) == -1)
> +			fatal("failed to daemonize");
> +
>  		/*
>  		 * Create the children sockets so we can use them
>  		 * to distribute the rest of the socketpair()s using
> blob - 941857b712434a6c0aadbbf58ec621fd6f38fcee
> blob + 1962362c37d79ebc90bda3e869bb0454bcd59990
> --- usr.sbin/snmpd/proc.c
> +++ usr.sbin/snmpd/proc.c
> @@ -205,6 +205,9 @@ proc_init(struct privsep *ps, struct privsep_proc *pro
>  		privsep_process = PROC_PARENT;
>  		proc_setup(ps, procs, nproc);
>
> +		if (!debug && daemon(0, 0) == -1)
> +			fatal("failed to daemonize");
> +
>  		/*
>  		 * Create the children sockets so we can use them
>  		 * to distribute the rest of the socketpair()s using
> blob - d3ecfd6f0276c55b44f55627f4d15d0baf146916
> blob + 0136f2801aec68508982b6ec69b5a08b4608deb6
> --- usr.sbin/snmpd/snmpd.c
> +++ usr.sbin/snmpd/snmpd.c
> @@ -222,8 +222,6 @@ main(int argc, char *argv[])
>  	env->sc_engine_boots = 0;
>
>  	proc_init(ps, procs, nitems(procs), debug, argc0, argv0, proc_id);
> -	if (!debug && daemon(0, 0) == -1)
> -		err(1, "failed to daemonize");
>
>  	log_procinit("parent");
>  	log_info("startup");
> blob - ac6455dd7c70fbfc76c7107b31cc95b5ff55eb87
> blob + b1d787de9e16da595433eeb9710cc31a3a4716c4
> --- usr.sbin/vmd/proc.c
> +++ usr.sbin/vmd/proc.c
> @@ -205,6 +205,9 @@ proc_init(struct privsep *ps, struct privsep_proc *pro
>  		privsep_process = PROC_PARENT;
>  		proc_setup(ps, procs, nproc);
>
> +		if (!debug && daemon(0, 0) == -1)
> +			fatal("failed to daemonize");
> +
>  		/*
>  		 * Create the children sockets so we can use them
>  		 * to distribute the rest of the socketpair()s using
> blob - 887e1cc9bf8b09abe70c873d96145673252bcb22
> blob + 62c11da7f8c4eb4a1dec768255ed2cb1f1f5224c
> --- usr.sbin/vmd/vmd.c
> +++ usr.sbin/vmd/vmd.c
> @@ -947,9 +947,6 @@ main(int argc, char **argv)
>  	proc_init(ps, procs, nitems(procs), env->vmd_debug, argc0, argv,
>  	    proc_id);
>
> -	if (!env->vmd_debug && daemon(0, 0) == -1)
> -		fatal("can't daemonize");
> -
>  	if (ps->ps_noaction == 0)
>  		log_info("startup");
>
>