Hi,

This is a much more extensive patch, but I believe it does finally
close the hole if you keep your PID files in a root-owned directory.

Please test this; I plan on releasing 2.81 tomorrow.

Regards,

Dianne.


diff --git a/Changelog b/Changelog
index da1a867..b9f09fa 100644
--- a/Changelog
+++ b/Changelog
@@ -2,6 +2,18 @@ WARNING: Before upgrading MIMEDefang, please search this file 
for
 *** NOTE INCOMPATIBILITY ** to see if anything has changed that might
 affect your filter.
 
+2017-08-31 Dianne Skoll <d...@roaringpenguin.com>
+
+       * Make mimedefang and mimedefang-multiplexor write their PID files
+       as root to avoid an unprivileged user tampering with the pidfiles.
+
+       *** NOTE INCOMPATIBILITY ***
+
+       You should move your PID files out of the MIMEDefang spool directory
+       and into a standard root-owned directory like /var/run.  Use the -o
+       option to create lock files in the spool directory.  The sample
+       init scripts have been updated to reflect this.
+
 2017-07-24 Dianne Skoll <d...@roaringpenguin.com>
 
        * MIMEDefang 2.80 RELEASED
diff --git a/examples/init-script.in b/examples/init-script.in
index 346efca..8da803f 100755
--- a/examples/init-script.in
+++ b/examples/init-script.in
@@ -10,8 +10,10 @@
 RETVAL=0
 prog='mimedefang'
 SPOOLDIR='@SPOOLDIR@'
-PID="$SPOOLDIR/$prog.pid"
-MXPID="$SPOOLDIR/$prog-multiplexor.pid"
+PID="/var/run/$prog.pid"
+MXPID="/var/run/$prog-multiplexor.pid"
+LOCK="$SPOOLDIR/$prog.lock"
+MXLOCK="$SPOOLDIR/$prog-multiplexor.lock"
 
 # These lines keep SpamAssassin happy.  Not needed if you
 # aren't using SpamAssassin.
@@ -229,7 +231,7 @@ start_it() {
     else
        EMBEDFLAG=""
     fi
-    $PROGDIR/$prog-multiplexor -p $MXPID \
+    $PROGDIR/$prog-multiplexor -p $MXPID -o $MXLOCK \
        $EMBEDFLAG \
        `[ -n "$SPOOLDIR" ] && echo "-z $SPOOLDIR"` \
        `[ -n "$FILTER" ] && echo "-f $FILTER"` \
@@ -269,7 +271,7 @@ start_it() {
     # Start mimedefang
     printf "%-60s" "Starting $prog: "
     rm -f $SOCKET > /dev/null 2>&1
-    $PROGDIR/$prog -P $PID -R $LOOPBACK_RESERVED_CONNECTIONS \
+    $PROGDIR/$prog -P $PID -o $LOCK -R $LOOPBACK_RESERVED_CONNECTIONS \
        -m $MX_SOCKET \
        `[ -n "$SPOOLDIR" ] && echo "-z $SPOOLDIR"` \
        `[ -n "$MX_USER" ] && echo "-U $MX_USER"` \
diff --git a/mimedefang-multiplexor.8.in b/mimedefang-multiplexor.8.in
index 980b57d..3505e48 100644
--- a/mimedefang-multiplexor.8.in
+++ b/mimedefang-multiplexor.8.in
@@ -117,7 +117,18 @@ for the format of \fIsocket\fR.
 .TP
 .B \-p \fIfileName\fR
 Causes \fBmimedefang-multiplexor\fR to write its process-ID (after
-becoming a daemon) to the specified file.
+becoming a daemon) to the specified file.  The file will be owned
+by root.
+
+.TP
+.B \-o \fIfileName\fR
+Causes \fbmimedefang-multiplexor\fR to use \fIfileName\fR as a lock
+file to avoid multiple instances from running.  If you supply
+\fB\-p\fR but not \fB\-o\fR, then \fbmimedefang-multiplexor\fR
+constructs a lock file by appending ".lock" to the pid file.  However,
+this is less secure than having a root-owned pid file in a root-owned
+directory and a lock file writable by the user named by the \fB\-U\fR
+option.  (The lock file must be writable by the \fB\-U\fR user.)
 
 .TP
 .B \-f \fIfilter_path\fR
diff --git a/mimedefang-multiplexor.c b/mimedefang-multiplexor.c
index 13b77b9..2e44f12 100644
--- a/mimedefang-multiplexor.c
+++ b/mimedefang-multiplexor.c
@@ -56,6 +56,12 @@
 static void limit_mem_usage(unsigned long rss, unsigned long as);
 #endif
 
+static char *pidfile = NULL;
+static char *lockfile = NULL;
+
+/* Number of file descriptors to close when forking */
+#define CLOSEFDS 256
+
 /* Weird case, but hey... */
 #if defined(HAVE_WAIT3) && !defined(HAVE_SETRLIMIT)
 #include <sys/resource.h>
@@ -346,6 +352,7 @@ static int get_hourly_history_totals(int cmd, time_t now, 
int hours, int *total,
 
 #define NUM_FREE_SLAVES    (SlaveCount[STATE_IDLE] + SlaveCount[STATE_STOPPED])
 #define NUM_RUNNING_SLAVES (SlaveCount[STATE_IDLE] + SlaveCount[STATE_BUSY] + 
SlaveCount[STATE_KILLED])
+#define REPORT_FAILURE(msg) do { if (kidpipe[1] >= 0) { write(kidpipe[1], "E" 
msg, strlen(msg)+1); } else { fprintf(stderr, "%s\n", msg); } } while(0)
 
 /**********************************************************************
 * %FUNCTION: state_name
@@ -496,6 +503,7 @@ usage(void)
     fprintf(stderr, "  -v                -- Print version and exit\n");
     fprintf(stderr, "  -t filename       -- Log statistics to filename\n");
     fprintf(stderr, "  -p filename       -- Write process-ID in filename\n");
+    fprintf(stderr, "  -o file           -- Use specified file as a lock 
file\n");
     fprintf(stderr, "  -T                -- Log statistics to syslog\n");
     fprintf(stderr, "  -u                -- Flush stats file after each 
write\n");
     fprintf(stderr, "  -Z                -- Accept and process status updates 
from busy slaves\n");
@@ -565,10 +573,13 @@ main(int argc, char *argv[], char **env)
     int sock, unpriv_sock;
     int c;
     int n;
-    char *pidfile = NULL;
+    int pidfile_fd = -1;
+    int lockfile_fd = -1;
     char *user = NULL;
     char *options;
     int facility = LOG_MAIL;
+    int kidpipe[2] = {-1, -1};
+    char kidmsg[256];
 
     time_t now;
 
@@ -593,13 +604,13 @@ main(int argc, char *argv[], char **env)
     if (getuid() != geteuid()) {
        fprintf(stderr, "ERROR: %s is NOT intended to run suid! Exiting.\n",
                argv[0]);
-       exit(1);
+       exit(EXIT_FAILURE);
     }
 
     if (getgid() != getegid()) {
        fprintf(stderr, "ERROR: %s is NOT intended to run sgid! Exiting.\n",
                argv[0]);
-       exit(1);
+       exit(EXIT_FAILURE);
     }
 
     Settings.minSlaves     = 0;
@@ -636,9 +647,9 @@ main(int argc, char *argv[], char **env)
     Settings.debugSlaveScheduling = 0;
 
 #ifndef HAVE_SETRLIMIT
-    options = 
"GAa:Tt:um:x:y:r:i:b:c:s:hdlf:p:w:F:W:U:S:q:Q:I:DEO:X:Y:N:vZP:z:";
+    options = 
"GAa:Tt:um:x:y:r:i:b:c:s:hdlf:p:o:w:F:W:U:S:q:Q:I:DEO:X:Y:N:vZP:z:";
 #else
-    options = 
"GAa:Tt:um:x:y:r:i:b:c:s:hdlf:p:w:F:W:U:S:q:Q:L:R:M:I:DEO:X:Y:N:vZP:z:";
+    options = 
"GAa:Tt:um:x:y:r:i:b:c:s:hdlf:p:o:w:F:W:U:S:q:Q:L:R:M:I:DEO:X:Y:N:vZP:z:";
 #endif
     while((c = getopt(argc, argv, options)) != -1) {
        switch(c) {
@@ -669,7 +680,7 @@ main(int argc, char *argv[], char **env)
            break;
        case 'v':
            printf("mimedefang-multiplexor version %s\n", VERSION);
-           exit(0);
+           exit(EXIT_SUCCESS);
 
        case 'E':
            #ifdef EMBED_PERL
@@ -830,6 +841,16 @@ main(int argc, char *argv[], char **env)
                exit(EXIT_FAILURE);
            }
            break;
+       case 'o':
+           /* Use this as our lock file */
+           if (lockfile != NULL) free(lockfile);
+
+           lockfile = strdup(optarg);
+           if (!lockfile) {
+               fprintf(stderr, "%s: Out of memory\n", argv[0]);
+               exit(EXIT_FAILURE);
+           }
+           break;
        case 'f':
            /* Filter program */
            if (optarg[0] != '/') {
@@ -919,6 +940,17 @@ main(int argc, char *argv[], char **env)
            strcat((char *) Settings.sockName, "/mimedefang-multiplexor.sock");
     }
 
+    /* Open the pidfile as root.  We'll write the pid later on in the 
grandchild */
+    if (pidfile) {
+       pidfile_fd = open(pidfile, O_RDWR|O_CREAT, 0666);
+       if (pidfile_fd < 0) {
+           syslog(LOG_ERR, "Could not open PID file %s: %m", pidfile);
+           exit(EXIT_FAILURE);
+       }
+       /* It needs to be world-readable */
+       fchmod(pidfile_fd, 0644);
+    }
+
     /* Drop privileges */
     if (user) {
        pw = getpwnam(user);
@@ -964,6 +996,63 @@ main(int argc, char *argv[], char **env)
        Settings.maxRecipokPerDomain = 0;
     }
 
+    /* Daemonize */
+    if (!nodaemon) {
+       /* Set up a pipe so child can report back when it's happy */
+       if (pipe(kidpipe) < 0) {
+           perror("pipe");
+           exit(EXIT_FAILURE);
+       }
+       i = fork();
+       if (i < 0) {
+           fprintf(stderr, "%s: fork() failed\n", argv[0]);
+           exit(EXIT_FAILURE);
+       } else if (i != 0) {
+           /* parent */
+           /* Wait for a message from kid */
+           close(kidpipe[1]);
+           i = read(kidpipe[0], kidmsg, sizeof(kidmsg) - 1);
+           if (i < 0) {
+               fprintf(stderr, "Error reading message from child: %s\n",
+                       strerror(errno));
+               exit(EXIT_FAILURE);
+           }
+           /* Zero-terminate the string */
+           kidmsg[i] = 0;
+           if (i == 1 && kidmsg[0] == 'X') {
+               /* Child indicated successful startup */
+               exit(EXIT_SUCCESS);
+           }
+           if (i > 1 && kidmsg[0] == 'E') {
+               /* Child indicated error */
+               fprintf(stderr, "Error from child: %s\n", kidmsg+1);
+               exit(EXIT_FAILURE);
+           }
+           /* Unknown status from child */
+           fprintf(stderr, "Unknown reply from child: %s\n", kidmsg);
+           exit(EXIT_FAILURE);
+       }
+       setsid();
+       signal(SIGHUP, SIG_IGN);
+       i = fork();
+       if (i < 0) {
+           fprintf(stderr, "%s: fork() failed\n", argv[0]);
+           exit(EXIT_FAILURE);
+       } else if (i != 0) {
+           exit(EXIT_SUCCESS);
+       }
+
+    }
+
+    /* Do the locking */
+    if (pidfile || lockfile) {
+       if ( (lockfile_fd = write_and_lock_pidfile(pidfile, lockfile, 
pidfile_fd)) < 0) {
+           REPORT_FAILURE("Cannot lock lockfile: Is another copy running?");
+           exit(EXIT_FAILURE);
+       }
+       pidfile_fd = -1;
+    }
+
     /* Initialize history buckets */
     init_history();
 
@@ -983,16 +1072,18 @@ main(int argc, char *argv[], char **env)
     AllSlaves = calloc(Settings.maxSlaves, sizeof(Slave));
 
     if (!AllSlaves) {
-       fprintf(stderr, "%s: Unable to allocate memory for slaves\n",
-               argv[0]);
+       REPORT_FAILURE("Unable to allocate memory for slaves");
+       if (pidfile) unlink(pidfile);
+       if (lockfile) unlink(lockfile);
        exit(EXIT_FAILURE);
     }
 
     /* Make an event selector */
     es = Event_CreateSelector();
     if (!es) {
-       fprintf(stderr, "%s: Could not create event selector: %s\n",
-               argv[0], strerror(errno));
+       REPORT_FAILURE("Could not create event selector");
+       if (pidfile) unlink(pidfile);
+       if (lockfile) unlink(lockfile);
        exit(EXIT_FAILURE);
     }
 
@@ -1051,21 +1142,21 @@ main(int argc, char *argv[], char **env)
 
     if (sock < 0) {
        if (sock == -2) {
-           fprintf(stderr, "%s: Argument to -s option must be a\nUNIX-domain 
socket, not a TCP socket.\n", argv[0]);
+           REPORT_FAILURE("Argument to -s option must be a UNIX-domain socket, 
not a TCP socket.");
        } else {
-           fprintf(stderr, "%s: Unable to create listening socket on path 
%s\n",
-                   argv[0],
-                   Settings.sockName);
+           REPORT_FAILURE("Unable to create listening socket.");
        }
+       if (pidfile) unlink(pidfile);
+       if (lockfile) unlink(lockfile);
        exit(EXIT_FAILURE);
     }
     set_cloexec(sock);
 
     /* Set up an accept loop */
     if (!EventTcp_CreateAcceptor(es, sock, handleAccept)) {
-       fprintf(stderr, "%s: Could not make accept() handler: %s\n",
-               argv[0],
-               strerror(errno));
+       REPORT_FAILURE("Could not make accept() handler");
+       if (pidfile) unlink(pidfile);
+       if (lockfile) unlink(lockfile);
        exit(EXIT_FAILURE);
     }
 
@@ -1077,16 +1168,16 @@ main(int argc, char *argv[], char **env)
                                                Settings.listenBacklog, 0);
        umask(file_umask);
        if (unpriv_sock < 0) {
-           fprintf(stderr, "%s: Unable to create listening socket on path 
%s\n",
-                   argv[0],
-                   Settings.unprivSockName);
+           REPORT_FAILURE("Unable to create unprivileged listening socket");
+           if (pidfile) unlink(pidfile);
+           if (lockfile) unlink(lockfile);
            exit(EXIT_FAILURE);
        }
        set_cloexec(unpriv_sock);
        if (!EventTcp_CreateAcceptor(es, unpriv_sock, handleUnprivAccept)) {
-           fprintf(stderr, "%s: Could not make accept() handler: %s\n",
-                   argv[0],
-                   strerror(errno));
+           REPORT_FAILURE("Could not make accept() handler");
+           if (pidfile) unlink(pidfile);
+           if (lockfile) unlink(lockfile);
            exit(EXIT_FAILURE);
        }
     }
@@ -1096,7 +1187,9 @@ main(int argc, char *argv[], char **env)
 
     /* Set up pipe for signal handler for slave termination notification*/
     if (pipe(Pipe) < 0) {
-       perror("pipe");
+       REPORT_FAILURE("pipe() call failed");
+       if (pidfile) unlink(pidfile);
+       if (lockfile) unlink(lockfile);
        exit(EXIT_FAILURE);
     }
 
@@ -1106,42 +1199,28 @@ main(int argc, char *argv[], char **env)
     /* Create event handler for pipe */
     if (!Event_AddHandler(es, Pipe[0],
                          EVENT_FLAG_READABLE, handlePipe, NULL)) {
-       fprintf(stderr, "%s: Could not make pipe handler: %s\n",
-               argv[0],
-               strerror(errno));
+       REPORT_FAILURE("Could not make pipe handler");
+       if (pidfile) unlink(pidfile);
+       if (lockfile) unlink(lockfile);
        exit(EXIT_FAILURE);
     }
 
-    /* Daemonize */
-    if (!nodaemon) {
-       i = fork();
-       if (i < 0) {
-           fprintf(stderr, "%s: fork() failed\n", argv[0]);
-           exit(EXIT_FAILURE);
-       } else if (i != 0) {
-           /* parent */
-           exit(EXIT_SUCCESS);
-       }
-       setsid();
-       signal(SIGHUP, SIG_IGN);
-       i = fork();
-       if (i < 0) {
-           fprintf(stderr, "%s: fork() failed\n", argv[0]);
-           exit(EXIT_FAILURE);
-       } else if (i != 0) {
-           exit(EXIT_SUCCESS);
+    /* Close files */
+    for (i=0; i<CLOSEFDS; i++) {
+       /* Don't close stdin/stdout/stderr if we are not a daemon */
+       if (nodaemon && i < 3) {
+           continue;
        }
-    }
-
-    for (i=0; i<256; i++) {
-       if (i == unpriv_sock || i == sock || i == Pipe[0] || i == Pipe[1]) 
continue;
+       if (i == kidpipe[0] || i == kidpipe[1] || i == lockfile_fd || i == 
unpriv_sock || i == sock || i == Pipe[0] || i == Pipe[1]) continue;
        (void) close(i);
     }
 
-    /* Direct stdin/stdout/stderr to /dev/null */
-    open("/dev/null", O_RDWR);
-    open("/dev/null", O_RDWR);
-    open("/dev/null", O_RDWR);
+    /* Direct stdin/stdout/stderr to /dev/null if we are a daemon */
+    if (!nodaemon) {
+       open("/dev/null", O_RDWR);
+       open("/dev/null", O_RDWR);
+       open("/dev/null", O_RDWR);
+    }
 
     /* Syslog if required */
     if (Settings.syslog_label) {
@@ -1153,20 +1232,14 @@ main(int argc, char *argv[], char **env)
     /* Keep track of our pid */
     ParentPid = getpid();
 
-    /* Write pid */
-    if (pidfile) {
-       if (write_and_lock_pidfile(pidfile) < 0) {
-           exit(1);
-       }
-       free(pidfile);
-    }
-
     /* Set up SIGHUP handler */
     act.sa_handler = hupHandler;
     sigemptyset(&act.sa_mask);
     act.sa_flags = SA_RESTART;
     if (sigaction(SIGHUP, &act, NULL) < 0) {
-       syslog(LOG_ERR, "sigaction failed - exiting: %m");
+       REPORT_FAILURE("sigaction failed - exiting.");
+       if (pidfile) unlink(pidfile);
+       if (lockfile) unlink(lockfile);
        exit(EXIT_FAILURE);
     }
 
@@ -1175,7 +1248,9 @@ main(int argc, char *argv[], char **env)
     sigemptyset(&act.sa_mask);
     act.sa_flags = SA_RESTART;
     if (sigaction(SIGINT, &act, NULL) < 0) {
-       syslog(LOG_ERR, "sigaction failed - exiting: %m");
+       REPORT_FAILURE("sigaction failed - exiting.");
+       if (pidfile) unlink(pidfile);
+       if (lockfile) unlink(lockfile);
        exit(EXIT_FAILURE);
     }
 
@@ -1207,10 +1282,12 @@ main(int argc, char *argv[], char **env)
 
     /* Set signal handler for SIGCHLD */
     if (set_sigchld_handler() < 0) {
-       syslog(LOG_ERR, "sigaction failed - exiting: %m");
+       REPORT_FAILURE("sigaction failed - exiting.");
 #ifdef EMBED_PERL
        if (Settings.useEmbeddedPerl) term_embedded_interpreter();
 #endif
+       if (pidfile) unlink(pidfile);
+       if (lockfile) unlink(lockfile);
        exit(EXIT_FAILURE);
     }
 
@@ -1267,6 +1344,10 @@ main(int argc, char *argv[], char **env)
        }
     }
 
+    /* Tell the waiting parent that everything is fine */
+    write(kidpipe[1], "X", 1);
+    close(kidpipe[1]);
+
     /* And loop... */
     while(1) {
        if (Event_HandleEvent(es) < 0) {
@@ -2599,7 +2680,7 @@ activateSlave(Slave *s, char const *reason)
     if (Settings.useEmbeddedPerl) {
        run_embedded_filter();
        term_embedded_interpreter();
-       exit(0);
+       exit(EXIT_SUCCESS);
     }
 #endif
     if (Settings.wantStatusReports) {
@@ -3174,9 +3255,15 @@ sigterm(int sig)
     /* Only the parent process should handle SIGTERM */
     if (ParentPid != getpid()) {
        syslog(LOG_WARNING, "Child process received SIGTERM before signal 
disposition could be reset!  Exiting!");
-       exit(1);
+       exit(EXIT_FAILURE);
     }
 
+    if (pidfile) {
+       unlink(pidfile);
+    }
+    if (lockfile) {
+       unlink(lockfile);
+    }
     if (DOLOG) {
        if (sig) {
            syslog(LOG_INFO, "Received SIGTERM: Stopping slaves and 
terminating");
@@ -3217,7 +3304,7 @@ sigterm(int sig)
 #ifdef EMBED_PERL
            if (Settings.useEmbeddedPerl) term_embedded_interpreter();
 #endif
-           exit(1);
+           exit(EXIT_FAILURE);
        }
        if (j != 9) {
            sleep(1);
@@ -3247,7 +3334,7 @@ sigterm(int sig)
 #ifdef EMBED_PERL
            if (Settings.useEmbeddedPerl) term_embedded_interpreter();
 #endif
-           exit(1);
+           exit(EXIT_FAILURE);
        }
        if (j != 9) {
            sleep(1);
diff --git a/mimedefang.8.in b/mimedefang.8.in
index 3151098..e0f45ca 100644
--- a/mimedefang.8.in
+++ b/mimedefang.8.in
@@ -139,7 +139,18 @@ the directory containing the failed message using syslog.
 .TP
 .B \-P \fIfileName\fR
 Causes \fBmimedefang\fR to write its process-ID (after
-becoming a daemon) to the specified file.
+becoming a daemon) to the specified file.  The file will be
+owned by root.
+
+.TP
+.B \-o \fIfileName\fR
+Causes \fbmimedefang\fR to use \fIfileName\fR as a lock file to avoid
+multiple instances from running.  If you supply \fB\-P\fR but not
+\fB\-o\fR, then \fbmimedefang\fR constructs a lock file by appending
+".lock" to the pid file.  However, this is less secure than having a
+root-owned pid file in a root-owned directory and a lock file writable
+by the user named by the \fB\-U\fR option.  (The lock file must be
+writable by the \fB\-U\fR user.)
 
 .TP
 .B \-R \fInum\fR
diff --git a/mimedefang.c b/mimedefang.c
index 7e74137..b311838 100644
--- a/mimedefang.c
+++ b/mimedefang.c
@@ -109,6 +109,8 @@ extern int find_syslog_facility(char const *facility_name);
 #define MD_SMFI_TRY(func, args) do { if (func args != MI_SUCCESS) 
syslog(LOG_WARNING, "%s: %s returned MI_FAILURE", data->qid, #func); } while (0)
 
 char *scan_body = NULL;
+static char *pidfile = NULL;
+static char *lockfile = NULL;
 
 #define KEY_FILE CONFDIR "/mimedefang-ip-key"
 
@@ -474,25 +476,6 @@ set_reply(SMFICTX *ctx,
 }
 
 /**********************************************************************
-*%FUNCTION: closefiles
-*%ARGUMENTS:
-* notthis - file descriptor that should not be closed
-*%RETURNS:
-* Nothing
-*%DESCRIPTION:
-* Hack -- closes a whole bunch of file descriptors.  Use after a fork()
-*         to conserve descriptors.
-***********************************************************************/
-static void
-closefiles(int notthis)
-{
-    int i;
-    for (i=0; i<CLOSEFDS; i++) {
-       if (i != notthis) (void) close(i);
-    }
-}
-
-/**********************************************************************
 *%FUNCTION: mfconnect
 *%ARGUMENTS:
 * ctx -- Sendmail filter mail context
@@ -2287,6 +2270,7 @@ usage(void)
     fprintf(stderr, "  -t                -- Do recipient checks before 
processing body\n");
     fprintf(stderr, "  -q                -- Allow new connections to be queued 
by multiplexor\n");
     fprintf(stderr, "  -P file           -- Write process-ID of daemon to 
specified file\n");
+    fprintf(stderr, "  -o file           -- Use specified file as a lock 
file\n");
     fprintf(stderr, "  -T                -- Log filter times to syslog\n");
     fprintf(stderr, "  -b n              -- Set listen() backlog to n\n");
     fprintf(stderr, "  -C                -- Try very hard to conserve file 
descriptors\n");
@@ -2306,7 +2290,8 @@ usage(void)
     exit(EXIT_FAILURE);
 }
 
-#define REPORT_FAILURE(msg, len) do { if (kidpipe[1] >= 0) { write(kidpipe[1], 
"E" msg, len+1); } else { fprintf(stderr, "%s\n", msg); } } while(0)
+#define REPORT_FAILURE(msg) do { if (kidpipe[1] >= 0) { write(kidpipe[1], "E" 
msg, strlen(msg)+1); } else { fprintf(stderr, "%s\n", msg); } } while(0)
+
 /**********************************************************************
 * %FUNCTION: main
 * %ARGUMENTS:
@@ -2322,7 +2307,6 @@ main(int argc, char **argv)
     int c;
     int mx_alive;
     pid_t i;
-    char *pidfile = NULL;
     struct passwd *pw = NULL;
     FILE *fp;
     int facility = LOG_MAIL;
@@ -2331,7 +2315,10 @@ main(int argc, char **argv)
     int got_p_option = 0;
     int kidpipe[2];
     char kidmsg[256];
-
+    int pidfile_fd = -1;
+    int lockfile_fd = -1;
+    int rc;
+    int j;
     mode_t socket_umask = 077;
     mode_t file_umask   = 077;
 
@@ -2388,7 +2375,7 @@ main(int argc, char **argv)
     }
 
     /* Process command line options */
-    while ((c = getopt(argc, argv, 
"GNCDHL:MP:R:S:TU:Xa:b:cdhkm:p:qrstvx:z:y")) != -1) {
+    while ((c = getopt(argc, argv, 
"GNCDHL:MP:o:R:S:TU:Xa:b:cdhkm:p:qrstvx:z:y")) != -1) {
        switch (c) {
        case 'y':
            setsymlist_ok = 1;
@@ -2521,6 +2508,16 @@ main(int argc, char **argv)
                exit(EXIT_FAILURE);
            }
            break;
+       case 'o':
+           /* Use this as our lock file */
+           if (lockfile != NULL) free(lockfile);
+
+           lockfile = strdup(optarg);
+           if (!lockfile) {
+               fprintf(stderr, "%s: Out of memory\n", argv[0]);
+               exit(EXIT_FAILURE);
+           }
+           break;
        case 'P':
            /* Write our pid to this file */
            if (pidfile != NULL) free(pidfile);
@@ -2611,6 +2608,17 @@ main(int argc, char **argv)
        exit(EXIT_FAILURE);
     }
 
+    /* Open the pidfile as root.  We'll write the pid later on in the 
grandchild */
+    if (pidfile) {
+       pidfile_fd = open(pidfile, O_RDWR|O_CREAT, 0666);
+       if (pidfile_fd < 0) {
+           syslog(LOG_ERR, "Could not open PID file %s: %m", pidfile);
+           exit(EXIT_FAILURE);
+       }
+       /* It needs to be world-readable */
+       fchmod(pidfile_fd, 0644);
+    }
+
     /* Look up user */
     if (user) {
        pw = getpwnam(user);
@@ -2655,6 +2663,8 @@ main(int argc, char **argv)
        exit(EXIT_FAILURE);
     }
 
+    (void) closelog();
+
     /* Daemonize */
     if (!nodaemon) {
        /* Set up a pipe so child can report back when it's happy */
@@ -2676,7 +2686,7 @@ main(int argc, char **argv)
            if (i < 0) {
                fprintf(stderr, "Error reading message from child: %s\n",
                        strerror(errno));
-               exit(1);
+               exit(EXIT_FAILURE);
            }
            /* Zero-terminate the string */
            kidmsg[i] = 0;
@@ -2700,9 +2710,7 @@ main(int argc, char **argv)
        signal(SIGHUP, SIG_IGN);
        i = fork();
        if (i < 0) {
-           fprintf(stderr, "%s: fork() failed\n", argv[0]);
-           /* Signal the waiting parent */
-           write(kidpipe[1], "Efork() failed", 14);
+           REPORT_FAILURE("fork() failed");
            exit(EXIT_FAILURE);
        } else if (i != 0) {
            exit(EXIT_SUCCESS);
@@ -2713,23 +2721,32 @@ main(int argc, char **argv)
        kidpipe[1] = -1;
     }
 
-    /* Write pid */
-    if (pidfile) {
-       if (write_and_lock_pidfile(pidfile) < 0) {
-           /* Signal the waiting parent */
-           REPORT_FAILURE("Cannot lock pidfile: Is another copy running?", 45);
-           exit(1);
+    /* In the actual daemon */
+    for (j=0; j<CLOSEFDS; j++) {
+       /* If we are not a daemon, leave stdin/stdout/stderr open */
+       if (nodaemon && j < 3) {
+           continue;
+       }
+       if (j != pidfile_fd && j != kidpipe[1]) {
+           close(j);
        }
-       free(pidfile);
     }
 
-    (void) closelog();
-    closefiles(kidpipe[1]);
+    /* Do the locking */
+    if (pidfile || lockfile) {
+       if ( (lockfile_fd = write_and_lock_pidfile(pidfile, lockfile, 
pidfile_fd)) < 0) {
+           /* Signal the waiting parent */
+           REPORT_FAILURE("Cannot lock lockfile: Is another copy running?");
+           exit(EXIT_FAILURE);
+       }
+    }
 
     /* Direct stdin/stdout/stderr to /dev/null */
-    open("/dev/null", O_RDWR);
-    open("/dev/null", O_RDWR);
-    open("/dev/null", O_RDWR);
+    if (!nodaemon) {
+       open("/dev/null", O_RDWR);
+       open("/dev/null", O_RDWR);
+       open("/dev/null", O_RDWR);
+    }
 
     openlog("mimedefang", LOG_PID, facility);
 
@@ -2770,7 +2787,9 @@ main(int argc, char **argv)
        syslog(LOG_INFO, "Multiplexor alive - entering main loop");
     } else {
        /* Signal the waiting parent */
-       REPORT_FAILURE("Multiplexor socket did not appear.  Exiting.", 44);
+       REPORT_FAILURE("Multiplexor socket did not appear.  Exiting.");
+       if (pidfile) unlink(pidfile);
+       if (lockfile) unlink(lockfile);
        exit(EXIT_FAILURE);
     }
 
@@ -2779,7 +2798,14 @@ main(int argc, char **argv)
        write(kidpipe[1], "X", 1);
        close(kidpipe[1]);
     }
-    return smfi_main();
+    rc = (int) smfi_main();
+    if (pidfile) {
+       unlink(pidfile);
+    }
+    if (lockfile) {
+       unlink(lockfile);
+    }
+    return rc;
 }
 
 /**********************************************************************
diff --git a/mimedefang.h b/mimedefang.h
index 381316d..59cf247 100644
--- a/mimedefang.h
+++ b/mimedefang.h
@@ -66,7 +66,7 @@ extern int make_listening_socket(char const *str, int 
backlog, int must_be_unix)
 extern void do_delay(char const *sleepstr);
 extern int is_localhost(struct sockaddr *);
 extern int remove_local_socket(char const *str);
-extern int write_and_lock_pidfile(char const *pidfile);
+extern int write_and_lock_pidfile(char const *pidfile, char *lockfile, int fd);
 #ifdef EMBED_PERL
 extern int make_embedded_interpreter(char const *progPath,
                                     char const *subFilter,
diff --git a/redhat/mimedefang-init.in b/redhat/mimedefang-init.in
index d3a514f..8f1aff0 100644
--- a/redhat/mimedefang-init.in
+++ b/redhat/mimedefang-init.in
@@ -31,7 +31,7 @@
 #              scans on incoming mail messages.
 # processname: mimedefang
 # config: @CONFDIR_EVAL@/mimedefang-filter
-# pidfile: @SPOOLDIR@/mimedefang.pid
+# pidfile: /var/run/mimedefang.pid
 
 ### BEGIN INIT INFO
 # Provides:          mimedefang
@@ -122,36 +122,37 @@ start() {
     echo -n "Starting $prog-multiplexor: "
     [ -e $MX_SOCKET ] && rm -f $MX_SOCKET
     # Tricky stuff below... "echo -E" won't work, hence the two-step.
-    daemon $PROGDIR/$prog-multiplexor -p @SPOOLDIR@/$prog-multiplexor.pid \
-       $([ -n "$FILTER" ] && echo "-f $FILTER") \
-       $([ -n "$SYSLOG_FACILITY" ] && echo "-S $SYSLOG_FACILITY") \
-       $([ -n "$SUBFILTER" ] && echo "-F $SUBFILTER") \
-       $([ -n "$MX_MINIMUM" ] && echo "-m $MX_MINIMUM") \
-       $([ -n "$MX_MAXIMUM" ] && echo "-x $MX_MAXIMUM") \
-       $([ -n "$MX_RECIPOK_PERDOMAIN_LIMIT" ] && echo "-y 
$MX_RECIPOK_PERDOMAIN_LIMIT") \
-       $([ -n "$MX_USER" ] && echo "-U $MX_USER") \
-       $([ -n "$MX_IDLE" ] && echo "-i $MX_IDLE") \
-       $([ -n "$MX_BUSY" ] && echo "-b $MX_BUSY") \
-       $([ -n "$MX_QUEUE_SIZE" ] && echo "-q $MX_QUEUE_SIZE") \
-       $([ -n "$MX_QUEUE_TIMEOUT" ] && echo "-Q $MX_QUEUE_TIMEOUT") \
-       $([ -n "$MX_REQUESTS" ] && echo "-r $MX_REQUESTS") \
-       $([ -n "$MX_MAP_SOCKET" ] && echo "-N $MX_MAP_SOCKET") \
-       $([ -n "$MX_SLAVE_DELAY" ] && echo "-w $MX_SLAVE_DELAY") \
-       $([ -n "$MX_MIN_SLAVE_DELAY" ] && echo "-W $MX_MIN_SLAVE_DELAY") \
-       $([ -n "$MX_LOG_SLAVE_STATUS_INTERVAL" ] && echo "-L 
$MX_LOG_SLAVE_STATUS_INTERVAL") \
-       $([ -n "$MX_MAX_RSS" ] && echo "-R $MX_MAX_RSS") \
-       $([ -n "$MX_MAX_AS" ] && echo "-M $MX_MAX_AS") \
-       $([ "$MX_EMBED_PERL" = "yes" ] && (echo -n "-"; echo "E")) \
-       $([ "$MX_LOG" = "yes" ] && echo "-l") \
-       $([ "$MX_STATS" = "yes" ] && echo "-t /var/log/mimedefang/stats") \
-       $([ "$MX_STATUS_UPDATES" = "yes" ] && echo "-Z") \
-       $([ "$MX_STATS" = "yes" -a "$MX_FLUSH_STATS" = "yes" ] && echo "-u") \
-       $([ -n "$MX_TICK_REQUEST" ] && echo "-X $MX_TICK_REQUEST") \
-       $([ -n "$MX_TICK_PARALLEL" ] && echo "-P $MX_TICK_PARALLEL") \
-       $([ "$MX_STATS_SYSLOG" = "yes" ] && echo "-T") \
-       $([ "$MD_ALLOW_GROUP_ACCESS" = "yes" ] && echo "-G") \
-       $([ -n "$MX_NOTIFIER" ] && echo "-O $MX_NOTIFIER") \
-       -s $MX_SOCKET
+    daemon $PROGDIR/$prog-multiplexor -p /var/run/$prog-multiplexor.pid \
+          -o @SPOOLDIR@/$prog-multiplexor.lock \
+          $([ -n "$FILTER" ] && echo "-f $FILTER") \
+          $([ -n "$SYSLOG_FACILITY" ] && echo "-S $SYSLOG_FACILITY") \
+          $([ -n "$SUBFILTER" ] && echo "-F $SUBFILTER") \
+          $([ -n "$MX_MINIMUM" ] && echo "-m $MX_MINIMUM") \
+          $([ -n "$MX_MAXIMUM" ] && echo "-x $MX_MAXIMUM") \
+          $([ -n "$MX_RECIPOK_PERDOMAIN_LIMIT" ] && echo "-y 
$MX_RECIPOK_PERDOMAIN_LIMIT") \
+          $([ -n "$MX_USER" ] && echo "-U $MX_USER") \
+          $([ -n "$MX_IDLE" ] && echo "-i $MX_IDLE") \
+          $([ -n "$MX_BUSY" ] && echo "-b $MX_BUSY") \
+          $([ -n "$MX_QUEUE_SIZE" ] && echo "-q $MX_QUEUE_SIZE") \
+          $([ -n "$MX_QUEUE_TIMEOUT" ] && echo "-Q $MX_QUEUE_TIMEOUT") \
+          $([ -n "$MX_REQUESTS" ] && echo "-r $MX_REQUESTS") \
+          $([ -n "$MX_MAP_SOCKET" ] && echo "-N $MX_MAP_SOCKET") \
+          $([ -n "$MX_SLAVE_DELAY" ] && echo "-w $MX_SLAVE_DELAY") \
+          $([ -n "$MX_MIN_SLAVE_DELAY" ] && echo "-W $MX_MIN_SLAVE_DELAY") \
+          $([ -n "$MX_LOG_SLAVE_STATUS_INTERVAL" ] && echo "-L 
$MX_LOG_SLAVE_STATUS_INTERVAL") \
+          $([ -n "$MX_MAX_RSS" ] && echo "-R $MX_MAX_RSS") \
+          $([ -n "$MX_MAX_AS" ] && echo "-M $MX_MAX_AS") \
+          $([ "$MX_EMBED_PERL" = "yes" ] && (echo -n "-"; echo "E")) \
+          $([ "$MX_LOG" = "yes" ] && echo "-l") \
+          $([ "$MX_STATS" = "yes" ] && echo "-t /var/log/mimedefang/stats") \
+          $([ "$MX_STATUS_UPDATES" = "yes" ] && echo "-Z") \
+          $([ "$MX_STATS" = "yes" -a "$MX_FLUSH_STATS" = "yes" ] && echo "-u") 
\
+          $([ -n "$MX_TICK_REQUEST" ] && echo "-X $MX_TICK_REQUEST") \
+          $([ -n "$MX_TICK_PARALLEL" ] && echo "-P $MX_TICK_PARALLEL") \
+          $([ "$MX_STATS_SYSLOG" = "yes" ] && echo "-T") \
+          $([ "$MD_ALLOW_GROUP_ACCESS" = "yes" ] && echo "-G") \
+          $([ -n "$MX_NOTIFIER" ] && echo "-O $MX_NOTIFIER") \
+          -s $MX_SOCKET
     echo
 
     # Start daemon
@@ -162,31 +163,27 @@ start() {
     # thread-creation will fail on a very busy server.
     ulimit -s 2048
 
-    daemon $PROGDIR/$prog -P @SPOOLDIR@/$prog.pid \
-       -m $MX_SOCKET \
-       $([ -n "$LOOPBACK_RESERVED_CONNECTIONS" ] && echo "-R 
$LOOPBACK_RESERVED_CONNECTIONS") \
-       $([ -n "$MX_USER" ] && echo "-U $MX_USER") \
-       $([ -n "$SYSLOG_FACILITY" ] && echo "-S $SYSLOG_FACILITY") \
-       $([ "$LOG_FILTER_TIME" = "yes" ] && echo "-T") \
-       $([ "$MX_RELAY_CHECK" = "yes" ] && echo "-r") \
-       $([ "$MX_HELO_CHECK" = "yes" ] && echo "-H") \
-       $([ "$MX_SENDER_CHECK" = "yes" ] && echo "-s") \
-       $([ "$MX_RECIPIENT_CHECK" = "yes" ] && echo "-t") \
-       $([ "$KEEP_FAILED_DIRECTORIES" = "yes" ] && echo "-k") \
-       $([ "$MD_ALLOW_GROUP_ACCESS" = "yes" ] && echo "-G") \
-       $([ -n "$MD_EXTRA" ] && echo "$MD_EXTRA") \
-       $([ "$ALLOW_NEW_CONNECTIONS_TO_QUEUE" = "yes" ] && echo "-q") \
-       -p $SOCKET
+    daemon $PROGDIR/$prog -P /var/run/$prog.pid \
+          -o @SPOOLDIR@/$prog.lock \
+          -m $MX_SOCKET \
+          $([ -n "$LOOPBACK_RESERVED_CONNECTIONS" ] && echo "-R 
$LOOPBACK_RESERVED_CONNECTIONS") \
+          $([ -n "$MX_USER" ] && echo "-U $MX_USER") \
+          $([ -n "$SYSLOG_FACILITY" ] && echo "-S $SYSLOG_FACILITY") \
+          $([ "$LOG_FILTER_TIME" = "yes" ] && echo "-T") \
+          $([ "$MX_RELAY_CHECK" = "yes" ] && echo "-r") \
+          $([ "$MX_HELO_CHECK" = "yes" ] && echo "-H") \
+          $([ "$MX_SENDER_CHECK" = "yes" ] && echo "-s") \
+          $([ "$MX_RECIPIENT_CHECK" = "yes" ] && echo "-t") \
+          $([ "$KEEP_FAILED_DIRECTORIES" = "yes" ] && echo "-k") \
+          $([ "$MD_ALLOW_GROUP_ACCESS" = "yes" ] && echo "-G") \
+          $([ -n "$MD_EXTRA" ] && echo "$MD_EXTRA") \
+          $([ "$ALLOW_NEW_CONNECTIONS_TO_QUEUE" = "yes" ] && echo "-q") \
+          -p $SOCKET
     RETVAL=$?
     echo
 
     [ $RETVAL -eq 0 ] && touch /var/lock/subsys/$prog
 
-    # Red Hat gets upset if pid files are not under /var/run, so let's
-    # keep Red Hat happy...
-    sleep 1
-    [ -f @SPOOLDIR@/$prog.pid ] && cp -f @SPOOLDIR@/$prog.pid /var/run
-    [ -f @SPOOLDIR@/$prog-multiplexor.pid ] && cp -f 
@SPOOLDIR@/$prog-multiplexor.pid /var/run
     return $RETVAL
 }
 
@@ -219,7 +216,7 @@ stop() {
     echo
 
     [ -e $SOCKET ] && rm -f $SOCKET
-    [ -f @SPOOLDIR@/$prog.pid ] && rm -f @SPOOLDIR@/$prog.pid
+    [ -f /var/run/$prog.pid ] && rm -f /var/run/$prog.pid
     [ -f /var/run/$prog.pid ] && rm -f /var/run/$prog.pid
 
     # Stop daemon
@@ -232,8 +229,8 @@ stop() {
     if [ "$1" = "wait" ] ; then
        printf "Waiting for daemons to exit"
        WAITPID=""
-       test -f @SPOOLDIR@/$prog.pid && WAITPID=`cat @SPOOLDIR@/$prog.pid`
-       test -f @SPOOLDIR@/prog-multiplexor.pid && WAITPID="$WAITPID `cat 
@SPOOLDIR@/prog-multiplexor.pid`"
+       test -f /var/run/$prog.pid && WAITPID=`cat /var/run/$prog.pid`
+       test -f /var/run//prog-multiplexor.pid && WAITPID="$WAITPID `cat 
/var/run//prog-multiplexor.pid`"
        n=0
        while [ -n "$WAITPID" ] ; do
            W2=""
@@ -252,7 +249,7 @@ stop() {
        echo ""
     fi
 
-    [ -f @SPOOLDIR@/$prog-multiplexor.pid ] && rm -f 
@SPOOLDIR@/$prog-multiplexor.pid
+    [ -f /var/run/$prog-multiplexor.pid ] && rm -f 
/var/run/$prog-multiplexor.pid
     [ -f /var/run/$prog-multiplexor.pid ] && rm -f 
/var/run/$prog-multiplexor.pid
 
     [ $RETVAL -eq 0 ] && rm -f /var/lock/subsys/$prog
@@ -306,8 +303,8 @@ case "$1" in
                echo "Could not communicate with $prog-multiplexor"
            fi
        else
-           if [ -r @SPOOLDIR@/$prog-multiplexor.pid ] ; then
-               kill -INT `cat @SPOOLDIR@/$prog-multiplexor.pid`
+           if [ -r /var/run/$prog-multiplexor.pid ] ; then
+               kill -INT `cat /var/run/$prog-multiplexor.pid`
                RETVAL=$?
                if [ $RETVAL = 0 ] ; then
                    echo "Told $prog-multiplexor to force reread of filter 
rules."
diff --git a/utils.c b/utils.c
index 7d4f9c1..4e84059 100644
--- a/utils.c
+++ b/utils.c
@@ -1300,30 +1300,53 @@ free_debug(void *ctx, void *x, char const *fname, int 
line)
 #endif
 
 int
-write_and_lock_pidfile(char const *pidfile)
+write_and_lock_pidfile(char const *pidfile, char *lockfile, int pidfile_fd)
 {
-    int fd;
     struct flock fl;
     char buf[64];
+    int lockfile_fd;
+    size_t len;
+
+    if (!lockfile) {
+       if (!pidfile) {
+           return -1;
+       }
+       len = strlen(pidfile) + 6;
+       /* If no lockfile was supplied, construct one based on pidfile */
+       lockfile = malloc(len);
+       if (!lockfile) {
+           return -1;
+       }
+
+       snprintf(lockfile, len, "%s.lock", pidfile);
+    }
+
+    lockfile_fd = open(lockfile, O_RDWR|O_CREAT, 0666);
+    if (lockfile_fd < 0) {
+       return -1;
+    }
 
     fl.l_type = F_WRLCK;
     fl.l_whence = SEEK_SET;
     fl.l_start = 0;
     fl.l_len = 0;
 
-    fd = open(pidfile, O_RDWR|O_CREAT, 0666);
-    if (fd < 0) {
-       syslog(LOG_ERR, "Could not open PID file %s: %m", pidfile);
+    if (fcntl(lockfile_fd, F_SETLK, &fl) < 0) {
+       syslog(LOG_ERR, "Could not lock lockfile file %s: %m.  Is another copy 
running?", lockfile);
        return -1;
     }
-    if (fcntl(fd, F_SETLK, &fl) < 0) {
-       syslog(LOG_ERR, "Could not lock PID file %s: %m.  Is another copy 
running?", pidfile);
-       return -1;
+    if (pidfile_fd >= 0) {
+       ftruncate(pidfile_fd, 0);
+       snprintf(buf, sizeof(buf), "%lu\n", (unsigned long) getpid());
+       write(pidfile_fd, buf, strlen(buf));
+
+       /* Close the pidfile fd; no longer needed */
+       if (close(pidfile_fd) < 0) {
+           return -1;
+       }
     }
-    ftruncate(fd, 0);
-    snprintf(buf, sizeof(buf), "%lu\n", (unsigned long) getpid());
-    write(fd, buf, strlen(buf));
-    /* Do NOT close fd... it will close and lock will be released
+
+    /* Do NOT close lockfile_fd... it will close and lock will be released
        when we exit */
-    return 0;
+    return lockfile_fd;
 }

Attachment: pgpdQKVwwBXSM.pgp
Description: OpenPGP digital signature

_______________________________________________
NOTE: If there is a disclaimer or other legal boilerplate in the above
message, it is NULL AND VOID.  You may ignore it.

Visit http://www.mimedefang.org and http://www.roaringpenguin.com
MIMEDefang mailing list MIMEDefang@lists.roaringpenguin.com
http://lists.roaringpenguin.com/mailman/listinfo/mimedefang

Reply via email to