[PATCH] casperd should detach from controlling session

[Xin Li]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

On 03/17/14 02:26, Pawel Jakub Dawidek wrote:
> On Thu, Mar 13, 2014 at 02:08:36PM -0700, Xin Li wrote:
>> -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA512
>> 
>> Hi, Pawel,
>> 
>> I have noticed that casperd's child (zygote) would still use 
>> controlling session from parent.  This can be observed by running
>> ps - -ax on systems running casperd, where the child have a
>> spurious console associated.
>> 
>> The attached patch would fix it.  May I commit it against -HEAD?
> 
> Hmm, daemon(3) does call setsid(2) already... Are you sure casperd 
> wasn't running with -F?

Oh, sure daemon(3) indeed does setsid(2) but casperd calls it after
zygote_init() so it has no effect to the zygote process, maybe
something like this instead?

Index: sbin/casperd/casperd.c
===================================================================
- --- sbin/casperd/casperd.c	(revision 263272)
+++ sbin/casperd/casperd.c	(working copy)
@@ -671,9 +671,6 @@ main(int argc, char *argv[])
 	pjdlog_prefix_set("(casperd) ");
 	pjdlog_debug_set(debug);

- -	if (zygote_init() < 0)
- -		pjdlog_exit(1, "Unable to create zygote process");
- -
 	pfh = pidfile_open(pidfile, 0600, &otherpid);
 	if (pfh == NULL) {
 		if (errno == EEXIST) {
@@ -699,6 +696,9 @@ main(int argc, char *argv[])
 		pjdlog_debug(1, "PID stored in %s.", pidfile);
 	}

+	if (zygote_init() < 0)
+		pjdlog_exit(1, "Unable to create zygote process");
+
 	/*
 	 * Register core services.
 	 */


>> By the way, the zygote child also closes file descriptor 4 twice 
>> (harmless; it's either sp[0] or the /dev/null which is closed
>> before starting zygote_main, or before returning from stdnull().
>> Based on the construct of the code, I believe both close() can be
>> omitted.  If this makes sense I'll submit a new patch.
> 
> I'd prefer to leave stdnull() as-is. I open /dev/null there and I
> close it in the same function. Not closing it there would make it
> confusing and would make function reuse error-prone.
> 
> If 'sock' has even higher number we will call close(2) on non-open 
> descriptors, but this is because there is no closerange(from, to) 
> syscall.
> 
> If you meant something else, do send me the patch and I can
> comment further.
> 
>> Index: sbin/casperd/zygote.c 
>> ===================================================================
>>
>> 
- --- sbin/casperd/zygote.c	(revision 263112)
>> +++ sbin/casperd/zygote.c	(working copy) @@ -63,6 +63,9 @@
>> stdnull(void) if (fd == -1) errx(1, "Unable to open %s",
>> _PATH_DEVNULL);
>> 
>> +	if (setsid() == -1) +		errx(1, "Unable to detach from
>> session"); + if (dup2(fd, STDIN_FILENO) == -1) errx(1, "Unable to
>> cover stdin"); if (dup2(fd, STDOUT_FILENO) == -1)
> 


- -- 
Xin LI <delphij at delphij.net>    https://www.delphij.net/
FreeBSD - The Power to Serve!           Live free or die
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (FreeBSD)

iQIcBAEBCgAGBQJTJ5ywAAoJEJW2GBstM+nshbgP/iRGivWPqnhjQPKbCNSGuG0Z
KZ3BKwD5rOt2DPMUxUd6fx9GJxaGZ6jM1NZGJko8wjwNHqBqShHNErJw19MhdTs/
x+2ROnLYf1RLgpwrLG8u50y4FU2MWViizw23foJksnu/pcdkLLo7pWzKOphabBZC
1N2EnM9E4M/iIMJyLH0dl9r4chLQqj6+KeKB2HX5It+HnRcuV/07Ut0aGomQaaNn
vZ+pwTAppXbgC//OYFoTYzaeaXfOCuHe+UtBsjAHMsQiDpG0ITCzae4MYcVwQeVc
w7DPIVJtUQ1efLc91dbqbHpSh8mymkOKV3EKsFG012RMzBZmksBD7CfexV8pAHr3
/l1sZCbqfg/siUVoU4lrpIOJz4/BeeWUNahYv8DCneQneIi9gH0SPnI8DKR3Zu9b
cp0VwsxFLtdyW15m6+lTyna286HQWhhZrfoogGvrLji/rZTWD7e/UTxLC7aV4MHK
sV/wAEMOVLnNv9h3FZdAtxTNYsj5QNfGjaRCBNfbZq1WGxnf27kHZJBQVKkOt8bq
CzCFmwVlkD4kPNMbY3WLL4oWbKVzmzE4yO/w6imSW3mmSCsNdNkA09zimOxD5ENh
Bkhr97P/2b10ftpBeckMuVweTdCNemlMu4YrlodeKP337qYJW1zPcWyH6/tVBvtw
Npm65R1ePctDfdjzePrG
=hyg9
-----END PGP SIGNATURE-----