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.

diff --git a/src/ngircd/conn.c b/src/ngircd/conn.c
index d8df6274ab7ac376db618a44c6bd77e02d9d8ad4..58a3cbfd20d7159903776a3d6060258becbb4aae 100644 (file)
--- a/src/ngircd/conn.c
+++ b/src/ngircd/conn.c
@@ -1091,10 +1091,6 @@ Conn_Close( CONN_ID Idx, const char *LogMsg, const char *FwdMsg, bool InformClie
                    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 );
 

diff --git a/src/ngircd/irc-login.c b/src/ngircd/irc-login.c
index b1b739b85a666c921371531ba4882da9991d6b72..03fea99ad4e8f79ceb3a426d073f75444817933e 100644 (file)
--- a/src/ngircd/irc-login.c
+++ b/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);

diff --git a/src/ngircd/proc.c b/src/ngircd/proc.c
index 1e8cac36eea9d92a9eea14ba0b4797e5cbe3a063..dbcff6f1aa5cfe0edf16ac4b90aace58a681698f 100644 (file)
--- a/src/ngircd/proc.c
+++ b/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)(int, short))
        case 0:
                /* New child process: */
                signal(SIGTERM, Proc_GenericSignalHandler);
+               signal(SIGALRM, Proc_GenericSignalHandler);
                close(pipefds[0]);
+               alarm(timeout);
+               Conn_CloseAllSockets();
                return 0;
        }
 
@@ -87,21 +91,6 @@ Proc_Fork(PROC_STAT *proc, int *pipefds, void (*cbfunc)(int, short))
        return pid;
 }
 
-/**
- * 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.
  */
@@ -112,6 +101,11 @@ Proc_GenericSignalHandler(int Signal)
        case SIGTERM:
 #ifdef DEBUG
                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);
        }
@@ -119,7 +113,7 @@ Proc_GenericSignalHandler(int Signal)
 
 /**
  * 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;
 }
 

diff --git a/src/ngircd/proc.h b/src/ngircd/proc.h
index 40a2c292cf178130921e5542e3218a9a6b925a8b..57612f172d5e61e2339ea703758a0283b5b9be7c 100644 (file)
--- a/src/ngircd/proc.h
+++ b/src/ngircd/proc.h
@@ -26,9 +26,7 @@ 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)));
-
-GLOBAL void Proc_Kill PARAMS((PROC_STAT *proc));
+                              void (*cbfunc)(int, short), int timeout));
 
 GLOBAL void Proc_GenericSignalHandler PARAMS((int Signal));
 

diff --git a/src/ngircd/resolve.c b/src/ngircd/resolve.c
index b88ec11ce6bfa53135f543003590b37e30f3c4b7..9bc3a87a07e3a1fd91e960505d5e783e1c90f76a 100644 (file)
--- a/src/ngircd/resolve.c
+++ b/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, int identsock,
 
        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 (*cbfunc)(int, short))
 
        assert(s != NULL);
 
-       pid = Proc_Fork(s, pipefd, cbfunc);
+       pid = Proc_Fork(s, pipefd, cbfunc, RESOLVER_TIMEOUT);
        if (pid > 0) {
                /* Main process */
 #ifdef DEBUG