Test for failure of the fork() when a root process creates a log file.

diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog
index 18e8d1ff8bdda44052dde8d4eee01fe7d8ff3a48..4cb08c1dc6b13208c46dce07769a1a1d5d153a52 100644 (file)
--- a/doc/doc-txt/ChangeLog
+++ b/doc/doc-txt/ChangeLog
@@ -1,4 +1,4 @@
-$Cambridge: exim/doc/doc-txt/ChangeLog,v 1.104 2005/03/29 14:53:09 ph10 Exp $
+$Cambridge: exim/doc/doc-txt/ChangeLog,v 1.105 2005/03/29 15:19:25 ph10 Exp $

 
 Change log file for Exim from version 4.21
 -------------------------------------------
 
 Change log file for Exim from version 4.21
 -------------------------------------------


@@ -99,6 +99,15 @@ PH/17 The API for radiusclient changed at release 0.4.0. Unfortunately, the
 PH/18 Installed Lars Mainka's patch for the support of CRL collections in
       files or directories, for OpenSSL.
 
 PH/18 Installed Lars Mainka's patch for the support of CRL collections in
       files or directories, for OpenSSL.
 

+PH/19 When an Exim process that is running as root has to create an Exim log
+      file, it does so in a subprocess that runs as exim:exim so as to get the
+      ownership right at creation (otherwise, other Exim processes might see
+      the file with the wrong ownership). There was no test for failure of this
+      fork() call, which would lead to the process getting stuck as it waited
+      for a non-existent subprocess. Forks do occasionally fail when resources
+      run out. I reviewed all the other calls to fork(); they all seem to check
+      for failure.
+

 
 A note about Exim versions 4.44 and 4.50
 ----------------------------------------
 
 A note about Exim versions 4.44 and 4.50
 ----------------------------------------

diff --git a/src/src/log.c b/src/src/log.c
index 0d8b3d0d62ef870a1efd5403e4c3ada6c9fcb512..d2ef9e51865ad6af1c2dded8ca965ed90732aa6e 100644 (file)
--- a/src/src/log.c
+++ b/src/src/log.c
@@ -1,4 +1,4 @@
-/* $Cambridge: exim/src/src/log.c,v 1.2 2005/01/04 10:00:42 ph10 Exp $ */
+/* $Cambridge: exim/src/src/log.c,v 1.3 2005/03/29 15:19:25 ph10 Exp $ */

 
 /*************************************************
 *     Exim - an Internet mail transport agent    *
 
 /*************************************************
 *     Exim - an Internet mail transport agent    *


@@ -321,8 +321,9 @@ if (*fd >= 0)
 /* Open was not successful: try creating the file. If this is a root process,
 we must do the creating in a subprocess set to exim:exim in order to ensure
 that the file is created with the right ownership. Otherwise, there can be a
 /* Open was not successful: try creating the file. If this is a root process,
 we must do the creating in a subprocess set to exim:exim in order to ensure
 that the file is created with the right ownership. Otherwise, there can be a

-race if an exim process is trying to write to the log at the same time. The use
-of SIGUSR1 by the exiwhat utility can provoke a lot of simultaneous writing. */
+race if another Exim process is trying to write to the log at the same time.
+The use of SIGUSR1 by the exiwhat utility can provoke a lot of simultaneous
+writing. */

 
 euid = geteuid();
 
 
 euid = geteuid();
 


@@ -350,10 +351,16 @@ else if (euid == root_uid)
     _exit((create_log(buffer) < 0)? 1 : 0);
     }
 
     _exit((create_log(buffer) < 0)? 1 : 0);
     }
 

-  /* Wait for the subprocess. If it succeeded retry the open. */
+  /* If we created a subprocess, wait for it. If it succeeded retry the open. */

 
 

-  while (waitpid(pid, &status, 0) != pid);
-  if (status == 0) *fd = Uopen(buffer, O_APPEND|O_WRONLY, LOG_MODE);
+  if (pid > 0)
+    {
+    while (waitpid(pid, &status, 0) != pid);
+    if (status == 0) *fd = Uopen(buffer, O_APPEND|O_WRONLY, LOG_MODE);
+    }
+
+  /* If we failed to create a subprocess, we are in a bad way. We fall through
+  with *fd still < 0, and errno set, letting the code below handle the error. */

   }
 
 /* If we now have an open file, set the close-on-exec flag and return. */
   }
 
 /* If we now have an open file, set the close-on-exec flag and return. */