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


Mike Larkin <mlarkin@nested.page> writes:

> 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

ok dv@ as well if not committed already.

>
> -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");
>>
>>