From: Philip Hazel <ph10@hermes.cam.ac.uk>
Date: Tue, 29 Mar 2005 15:19:25 +0000 (+0000)
Subject: Test for failure of the fork() when a root process creates a log file.
X-Git-Tag: exim-4_51~36
X-Git-Url: https://git.exim.org/exim.git/commitdiff_plain/901f42cb9f12332dd04a14cea87d47ef7e66e2e2?ds=sidebyside

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 18e8d1ff8..4cb08c1dc 100644
--- 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
 -------------------------------------------
@@ -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/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
 ----------------------------------------
diff --git a/src/src/log.c b/src/src/log.c
index 0d8b3d0d6..d2ef9e518 100644
--- 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    *
@@ -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
-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();
 
@@ -350,10 +351,16 @@ else if (euid == root_uid)
     _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. */