auditd: Fix signal handling

Rewrite the main loop to use ppoll() instead of just blocking on read,
blocking the signals we care about when we aren't polling.

I didn't bother replacing alarm() with setitimer(); the alarm code
is dead anyway since there is no way for max_idletime to acquire a
non-zero value.

While here, avoid leaking the pid file and trigger descriptors to the
log child.

PR:		295840
MFC after:	1 week
Sponsored by:	Klara, Inc.
Reviewed by:	kevans
Differential Revision:	https://reviews.freebsd.org/D57451

contrib/openbsm/bin/auditd/audit_warn.c

@@ -29,6 +29,7 @@
 
 #include <sys/types.h>
 
+#include <signal.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <unistd.h>
@@ -57,6 +58,9 @@ auditwarnlog(char *args[])
 		/*
 		 * Child.
 		 */
+#ifndef USE_MACH_IPC
+		sigprocmask(SIG_SETMASK, &auditd_origmask, NULL);
+#endif /* !USE_MACH_IPC */
 		execv(AUDITWARN_SCRIPT, loc_args);
 		syslog(LOG_ERR, "Could not exec %s (%m)\n",
 		    AUDITWARN_SCRIPT);

contrib/openbsm/bin/auditd/auditd.c

@@ -107,6 +107,19 @@ static gid_t	audit_review_gid = -1;
  */
 static char	*lastfile = NULL;
 
+/*
+ * File descriptor to our locked pid file.
+ */
+static int	pidfd;
+
+#ifndef USE_MACH_IPC
+/*
+ * Original signal mask in effect at startup.  Used by the main event loop
+ * and the log child.
+ */
+sigset_t	auditd_origmask;
+#endif /* !USE_MACH_IPC */
+
 /*
  * Error starting auditd. Run warn script and exit.
  */
@@ -354,12 +367,20 @@ close_misc(void)
 		auditd_log_err("Couldn't remove %s: %m", AUDITD_PIDFILE);
 		return (1);
 	}
+	close(pidfd);
+	pidfd = -1;
 	endac();
 
 	if (auditd_close_trigger() != 0) {
 		auditd_log_err("Error closing trigger messaging mechanism");
 		return (1);
 	}
+
+#ifndef USE_MACH_IPC
+	/* Restore the original signal mask. */
+	sigprocmask(SIG_SETMASK, &auditd_origmask, NULL);
+#endif /* !USE_MACH_IPC */
+
 	return (0);
 }
 
@@ -416,9 +437,17 @@ static int
 register_daemon(void)
 {
 	struct sigaction action;
-	FILE * pidfile;
-	int fd;
-	pid_t pid;
+	sigset_t sigmask;
+
+#ifndef USE_MACH_IPC
+	/* Set up the signal mask. */
+	sigemptyset(&sigmask);
+	sigaddset(&sigmask, SIGTERM);
+	sigaddset(&sigmask, SIGALRM);
+	sigaddset(&sigmask, SIGCHLD);
+	sigaddset(&sigmask, SIGHUP);
+	sigprocmask(SIG_BLOCK, &sigmask, &auditd_origmask);
+#endif /* !USE_MACH_IPC */
 
 	/* Set up the signal hander. */
 	action.sa_handler = auditd_relay_signal;
@@ -449,29 +478,30 @@ register_daemon(void)
 		fail_exit();
 	}
 
-	if ((pidfile = fopen(AUDITD_PIDFILE, "a")) == NULL) {
+	/* Open the pid file. */
+	pidfd = open(AUDITD_PIDFILE, O_CREAT | O_WRONLY | O_APPEND | O_CLOEXEC,
+	    S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
+	if (pidfd < 0) {
 		auditd_log_err("Could not open PID file");
 		audit_warn_tmpfile();
 		return (-1);
 	}
 
 	/* Attempt to lock the pid file; if a lock is present, exit. */
-	fd = fileno(pidfile);
-	if (flock(fd, LOCK_EX | LOCK_NB) < 0) {
+	if (flock(pidfd, LOCK_EX | LOCK_NB) < 0) {
 		auditd_log_err(
 		    "PID file is locked (is another auditd running?).");
 		audit_warn_ebusy();
 		return (-1);
 	}
 
-	pid = getpid();
-	ftruncate(fd, 0);
-	if (fprintf(pidfile, "%u\n", pid) < 0) {
+	/* Write our pid to the pid file and leave it open. */
+	ftruncate(pidfd, 0);
+	if (dprintf(pidfd, "%u\n", getpid()) < 0) {
 		/* Should not start the daemon. */
 		fail_exit();
 	}
 
-	fflush(pidfile);
 	return (0);
 }
 

contrib/openbsm/bin/auditd/auditd.h

@@ -96,5 +96,8 @@ void	auditd_terminate(void);
 int	auditd_config_controls(void);
 void	auditd_reap_children(void);
 
+#ifndef USE_MACH_IPC
+extern sigset_t auditd_origmask;
+#endif /* !USE_MACH_IPC */
 
 #endif /* !_AUDITD_H_ */

contrib/openbsm/bin/auditd/auditd_fbsd.c

@@ -33,6 +33,7 @@
 
 #include <errno.h>
 #include <fcntl.h>
+#include <poll.h>
 #include <stdarg.h>
 #include <signal.h>
 #include <string.h>
@@ -57,10 +58,7 @@ static int	auditing_state = AUD_STATE_INIT;
  */
 static int	max_idletime = 0;
 
-static int	sigchlds, sigchlds_handled;
-static int	sighups, sighups_handled;
-static int	sigterms, sigterms_handled;
-static int	sigalrms, sigalrms_handled;
+static volatile sig_atomic_t	signaled[NSIG];
 
 static int	triggerfd = 0;
 
@@ -83,7 +81,7 @@ auditd_openlog(int debug, gid_t __unused gid)
 }
 
 /*
- * Log messages at different priority levels. 
+ * Log messages at different priority levels.
  */
 void
 auditd_log_err(const char *fmt, ...)
@@ -154,7 +152,7 @@ auditd_set_state(int state)
 {
 	int old_auditing_state = auditing_state;
 
-	if (state == AUD_STATE_INIT) 
+	if (state == AUD_STATE_INIT)
 		init_audit_state();
 	else
 		auditing_state = state;
@@ -173,7 +171,6 @@ auditd_set_state(int state)
 int
 auditd_get_state(void)
 {
-
 	if (auditing_state == AUD_STATE_INIT)
 		init_audit_state();
 
@@ -186,8 +183,8 @@ auditd_get_state(void)
 int
 auditd_open_trigger(int __unused launchd_flag)
 {
-
-	return ((triggerfd = open(AUDIT_TRIGGER_FILE, O_RDONLY, 0)));
+	triggerfd = open(AUDIT_TRIGGER_FILE, O_RDONLY | O_CLOEXEC);
+	return (triggerfd);
 }
 
 /*
@@ -196,56 +193,66 @@ auditd_open_trigger(int __unused launchd_flag)
 int
 auditd_close_trigger(void)
 {
-	
 	return (close(triggerfd));
 }
 
-/* 
+/*
  * The main event loop.  Wait for trigger messages or signals and handle them.
  * It should not return unless there is a problem.
  */
 void
 auditd_wait_for_events(void)
 {
-	int num;
+	struct pollfd pfd;
+	ssize_t ret;
 	unsigned int trigger;
 
+	pfd.fd = triggerfd;
+	pfd.events = POLLIN;
+	pfd.revents = 0;
 	for (;;) {
-		num = read(triggerfd, &trigger, sizeof(trigger));
-		if ((num == -1) && (errno != EINTR)) {
-			auditd_log_err("%s: error %d", __FUNCTION__, errno);
-			return;
-		}
-		
 		/* Reset the idle time alarm, if used. */
-		if (max_idletime)
+		if (max_idletime != 0)
 			alarm(max_idletime);
 
-		if (sigterms != sigterms_handled) {
+		/* Check if any signals were caught. */
+		if (signaled[SIGTERM]) {
+			signaled[SIGTERM] = 0;
 			auditd_log_debug("%s: SIGTERM", __FUNCTION__);
 			auditd_terminate();
-			/* not reached */ 
+			/* not reached */
 		}
-		if (sigalrms != sigalrms_handled) {
+		if (signaled[SIGALRM]) {
+			signaled[SIGALRM] = 0;
 			auditd_log_debug("%s: SIGALRM", __FUNCTION__);
 			auditd_terminate();
-			/* not reached */ 
+			/* not reached */
 		}
- 		if (sigchlds != sigchlds_handled) {
-			sigchlds_handled = sigchlds;
+		if (signaled[SIGCHLD]) {
+			signaled[SIGCHLD] = 0;
 			auditd_reap_children();
 		}
-		if (sighups != sighups_handled) {
+		if (signaled[SIGHUP]) {
+			signaled[SIGHUP] = 0;
 			auditd_log_debug("%s: SIGHUP", __FUNCTION__);
-			sighups_handled = sighups;
 			auditd_config_controls();
 		}
 
-		if (num == -1)
+		/* Now wait for a trigger or signal. */
+		if ((ret = ppoll(&pfd, 1, NULL, &auditd_origmask)) < 0 &&
+		    errno != EINTR) {
+			auditd_log_err("%s: error %d", __FUNCTION__, errno);
+			break;
+		}
+		if (ret <= 0)
 			continue;
-		if (num == 0) {
+		if ((ret = read(triggerfd, &trigger, sizeof(trigger))) < 0) {
+			auditd_log_err("%s: error %d", __FUNCTION__, errno);
+			break;
+		}
+		if (ret == 0) {
 			auditd_log_err("%s: read EOF", __FUNCTION__);
-			return;
+			break;
 		}
 		auditd_handle_trigger(trigger);
 	}
@@ -258,15 +265,7 @@ auditd_wait_for_events(void)
  * context.
  */
 void
-auditd_relay_signal(int signal)
+auditd_relay_signal(int signo)
 {
-        if (signal == SIGHUP)
-                sighups++;
-        if (signal == SIGTERM)
-                sigterms++;
-        if (signal == SIGCHLD)
-                sigchlds++;
-	if (signal == SIGALRM)
-		sigalrms++;
+	signaled[signo] = 1;
 }
-