commit 6ebb31ab35e7f9258f4df9d0bfd111dc75677bfe
from: Alexander Barton <alex@barton.de>
date: Wed Jul 14 08:29:05 2010 UTC

Remove Proc_Kill(), use timeout to kill child processes

This avoids a race and potentionally killing the wrong process on
systems that use randomized process IDs; now the child itself is
responsible to exit in a timely manner using SIGALRM.

commit - cf93881dfbd7b6317d2a41f87e9751b74354203b
commit + 6ebb31ab35e7f9258f4df9d0bfd111dc75677bfe
blob - d8df6274ab7ac376db618a44c6bd77e02d9d8ad4
blob + 58a3cbfd20d7159903776a3d6060258becbb4aae
--- src/ngircd/conn.c
+++ src/ngircd/conn.c
@@ -1090,10 +1090,6 @@ Conn_Close( CONN_ID Idx, const char *LogMsg, const cha
 		    Idx, My_Connections[Idx].host, port,
 		    in_k, out_k);
 	}
-
-	/* Kill possibly running subprocess */
-	if (Proc_InProgress(&My_Connections[Idx].proc_stat))
-		Proc_Kill(&My_Connections[Idx].proc_stat);
 
 	/* Servers: Modify time of next connect attempt? */
 	Conf_UnsetServer( Idx );
blob - b1b739b85a666c921371531ba4882da9991d6b72
blob + 03fea99ad4e8f79ceb3a426d073f75444817933e
--- src/ngircd/irc-login.c
+++ src/ngircd/irc-login.c
@@ -789,7 +789,10 @@ Hello_User(CLIENT * Client)
 		return DISCONNECTED;
 	}
 
-	pid = Proc_Fork(Conn_GetProcStat(conn), pipefd, cb_Read_Auth_Result);
+	/* Fork child process for PAM authentication; and make sure that the
+	 * process timeout is set higher than the login timeout! */
+	pid = Proc_Fork(Conn_GetProcStat(conn), pipefd,
+			cb_Read_Auth_Result, Conf_PongTimeout + 1);
 	if (pid > 0) {
 		LogDebug("Authenticator for connection %d created (PID %d).",
 			 conn, pid);
blob - 1e8cac36eea9d92a9eea14ba0b4797e5cbe3a063
blob + dbcff6f1aa5cfe0edf16ac4b90aace58a681698f
--- src/ngircd/proc.c
+++ src/ngircd/proc.c
@@ -23,6 +23,7 @@
 
 #include "log.h"
 #include "io.h"
+#include "conn.h"
 
 #include "exp.h"
 #include "proc.h"
@@ -42,7 +43,7 @@ Proc_InitStruct (PROC_STAT *proc)
  * Fork a child process.
  */
 GLOBAL pid_t
-Proc_Fork(PROC_STAT *proc, int *pipefds, void (*cbfunc)(int, short))
+Proc_Fork(PROC_STAT *proc, int *pipefds, void (*cbfunc)(int, short), int timeout)
 {
 	pid_t pid;
 
@@ -67,7 +68,10 @@ Proc_Fork(PROC_STAT *proc, int *pipefds, void (*cbfunc
 	case 0:
 		/* New child process: */
 		signal(SIGTERM, Proc_GenericSignalHandler);
+		signal(SIGALRM, Proc_GenericSignalHandler);
 		close(pipefds[0]);
+		alarm(timeout);
+		Conn_CloseAllSockets();
 		return 0;
 	}
 
@@ -88,21 +92,6 @@ Proc_Fork(PROC_STAT *proc, int *pipefds, void (*cbfunc
 }
 
 /**
- * Kill forked child process.
- */
-GLOBAL void
-Proc_Kill(PROC_STAT *proc)
-{
-	assert(proc != NULL);
-
-	if (proc->pipe_fd > 0)
-		io_close(proc->pipe_fd);
-	if (proc->pid > 0)
-		kill(proc->pid, SIGTERM);
-	Proc_InitStruct(proc);
-}
-
-/**
  * Generic signal handler for forked child processes.
  */
 GLOBAL void
@@ -114,12 +103,17 @@ Proc_GenericSignalHandler(int Signal)
 		Log_Subprocess(LOG_DEBUG, "Child got TERM signal, exiting.");
 #endif
 		exit(1);
+	case SIGALRM:
+#ifdef DEBUG
+		Log_Subprocess(LOG_DEBUG, "Child got ALARM signal, exiting.");
+#endif
+		exit(1);
 	}
 }
 
 /**
  * Read bytes from a pipe of a forked child process.
- * In addition, this function makes sure that the child process is dead
+ * In addition, this function makes sure that the child process is ignored
  * after all data has been read or a fatal error occurred.
  */
 GLOBAL size_t
@@ -142,7 +136,7 @@ Proc_Read(PROC_STAT *proc, void *buffer, size_t buflen
 	else if (bytes_read == 0)
 		LogDebug("Can't read from child process %ld: EOF", proc->pid);
 #endif
-	Proc_Kill(proc);
+	Proc_InitStruct(proc);
 	return (size_t)bytes_read;
 }
 
blob - 40a2c292cf178130921e5542e3218a9a6b925a8b
blob + 57612f172d5e61e2339ea703758a0283b5b9be7c
--- src/ngircd/proc.h
+++ src/ngircd/proc.h
@@ -26,10 +26,8 @@ typedef struct _Proc_Stat {
 GLOBAL void Proc_InitStruct PARAMS((PROC_STAT *proc));
 
 GLOBAL pid_t Proc_Fork PARAMS((PROC_STAT *proc, int *pipefds,
-			       void (*cbfunc)(int, short)));
+			       void (*cbfunc)(int, short), int timeout));
 
-GLOBAL void Proc_Kill PARAMS((PROC_STAT *proc));
-
 GLOBAL void Proc_GenericSignalHandler PARAMS((int Signal));
 
 GLOBAL size_t Proc_Read PARAMS((PROC_STAT *proc, void *buffer, size_t buflen));
blob - b88ec11ce6bfa53135f543003590b37e30f3c4b7
blob + 9bc3a87a07e3a1fd91e960505d5e783e1c90f76a
--- src/ngircd/resolve.c
+++ src/ngircd/resolve.c
@@ -12,6 +12,8 @@
  */
 
 
+#define RESOLVER_TIMEOUT (Conf_PongTimeout*3)/4
+
 #include "portab.h"
 
 #include "imp.h"
@@ -33,6 +35,7 @@
 
 #include "array.h"
 #include "conn.h"
+#include "conf.h"
 #include "defines.h"
 #include "log.h"
 #include "ng_ipaddr.h"
@@ -63,7 +66,7 @@ Resolve_Addr(PROC_STAT * s, const ng_ipaddr_t *Addr, i
 
 	assert(s != NULL);
 
-	pid = Proc_Fork(s, pipefd, cbfunc);
+	pid = Proc_Fork(s, pipefd, cbfunc, RESOLVER_TIMEOUT);
 	if (pid > 0) {
 		LogDebug("Resolver for %s created (PID %d).", ng_ipaddr_tostr(Addr), pid);
 		return true;
@@ -89,7 +92,7 @@ Resolve_Name( PROC_STAT *s, const char *Host, void (*c
 
 	assert(s != NULL);
 
-	pid = Proc_Fork(s, pipefd, cbfunc);
+	pid = Proc_Fork(s, pipefd, cbfunc, RESOLVER_TIMEOUT);
 	if (pid > 0) {
 		/* Main process */
 #ifdef DEBUG