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