Module: ngircd.git Branch: master Commit: 6ebb31ab35e7f9258f4df9d0bfd111dc75677bfe URL: http://ngircd.barton.de/cgi-bin/gitweb.cgi?p=ngircd.git&a=commit;h=6ebb3...

Author: Alexander Barton alex@barton.de Date: Wed Jul 14 10:29:05 2010 +0200

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.

---

src/ngircd/conn.c | 4 ---- src/ngircd/irc-login.c | 5 ++++- src/ngircd/proc.c | 30 ++++++++++++------------------ src/ngircd/proc.h | 4 +--- src/ngircd/resolve.c | 7 +++++-- 5 files changed, 22 insertions(+), 28 deletions(-)

diff --git a/src/ngircd/conn.c b/src/ngircd/conn.c index d8df627..58a3cbf 100644 --- 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 b1b739b..03fea99 100644 --- 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 1e8cac3..dbcff6f 100644 --- 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; }

@@ -88,21 +92,6 @@ Proc_Fork(PROC_STAT *proc, int *pipefds, void (*cbfunc)(int, short)) }

/** - * 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; }

diff --git a/src/ngircd/proc.h b/src/ngircd/proc.h index 40a2c29..57612f1 100644 --- 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 b88ec11..9bc3a87 100644 --- 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