app.c: make sure that no non-async-signal-safe syscalls are used after

fork before exec Posix does only allow async-signal-safe syscalls after fork before exec. As asterisk ignores this, functions like TrySystem or System sometimes end up in a deadlocked child process. The patch prevents the use of non-async-signal-safe syscalls. ASTERISK-28776 Change-Id: Idc76365c0592ee3f3b3bd72a4f48f7a098978e8e
d-patch · May 8, 2020 · 6b2d945 · 6b2d945
1 parent 7fbfbe7
commit 6b2d945
Show file tree

Hide file tree

Showing 3 changed files with 75 additions and 58 deletions.
diff --git a/main/app.c b/main/app.c
@@ -81,6 +81,9 @@ struct zombie {
 
 static AST_LIST_HEAD_STATIC(zombies, zombie);
 
+#ifdef HAVE_CAP
+static cap_t child_cap;
+#endif
 /*
  * @{ \brief Define \ref stasis topic objects
  */
@@ -3003,12 +3006,7 @@ int ast_safe_fork(int stop_reaper)
 	} else {
 		/* Child */
 #ifdef HAVE_CAP
-		cap_t cap = cap_from_text("cap_net_admin-eip");
-
-		if (cap_set_proc(cap)) {
-			ast_log(LOG_WARNING, "Unable to remove capabilities.\n");
-		}
-		cap_free(cap);
+		cap_set_proc(child_cap);
 #endif
 
 		/* Before we unblock our signals, return our trapped signals back to the defaults */
@@ -3118,6 +3116,9 @@ struct stasis_topic *ast_queue_topic(const char *queuename)
 
 static void app_cleanup(void)
 {
+#ifdef HAS_CAP
+	cap_free(child_cap);
+#endif
 	ao2_cleanup(queue_topic_pool);
 	queue_topic_pool = NULL;
 	ao2_cleanup(queue_topic_all);
@@ -3127,7 +3128,9 @@ static void app_cleanup(void)
 int app_init(void)
 {
 	ast_register_cleanup(app_cleanup);
-
+#ifdef HAVE_CAP
+	child_cap = cap_from_text("cap_net_admin-eip");
+#endif
 	queue_topic_all = stasis_topic_create("queue:all");
 	if (!queue_topic_all) {
 		return -1;

diff --git a/main/asterisk.c b/main/asterisk.c
@@ -388,6 +388,10 @@ static int multi_thread_safe;
 
 static char randompool[256];
 
+#ifdef HAVE_CAP
+static cap_t child_cap;
+#endif
+
 static int sig_alert_pipe[2] = { -1, -1 };
 static struct {
 	 unsigned int need_reload:1;
@@ -1099,13 +1103,7 @@ static pid_t safe_exec_prep(int dualfork)
 
 	if (pid == 0) {
 #ifdef HAVE_CAP
-		cap_t cap = cap_from_text("cap_net_admin-eip");
-
-		if (cap_set_proc(cap)) {
-			/* Careful with order! Logging cannot happen after we close FDs */
-			ast_log(LOG_WARNING, "Unable to remove capabilities.\n");
-		}
-		cap_free(cap);
+		cap_set_proc(child_cap);
 #endif
 #ifdef HAVE_WORKING_FORK
 		if (ast_opt_high_priority) {
@@ -1804,10 +1802,8 @@ int ast_set_priority(int pri)
 	if (pri) {
 		sched.sched_priority = 10;
 		if (sched_setscheduler(0, SCHED_RR, &sched)) {
-			ast_log(LOG_WARNING, "Unable to set high priority\n");
 			return -1;
-		} else
-			ast_verb(1, "Set to realtime thread\n");
+		}
 	} else {
 		sched.sched_priority = 0;
 		/* According to the manpage, these parameters can never fail. */
@@ -3920,8 +3916,14 @@ int main(int argc, char *argv[])
 		exit(1);
 	}
 
+#ifdef HAVE_CAP
+	child_cap = cap_from_text("cap_net_admin-eip");
+#endif
 	/* Not a remote console? Start the daemon. */
 	asterisk_daemon(isroot, runuser, rungroup);
+#ifdef HAS_CAP
+	cap_free(child_cap);
+#endif
 	return 0;
 }
 

diff --git a/main/strcompat.c b/main/strcompat.c
@@ -38,6 +38,8 @@
 
 #include "asterisk/utils.h"
 
+#define POLL_SIZE 1024
+
 #ifndef HAVE_STRSEP
 char *strsep(char **str, const char *delims)
 {
@@ -426,59 +428,69 @@ int ffsll(long long n)
 #ifndef HAVE_CLOSEFROM
 void closefrom(int n)
 {
-	long x;
+	int maxfd;
+#ifndef _SC_OPEN_MAX
 	struct rlimit rl;
-	DIR *dir;
-	char path[16], *result;
-	struct dirent *entry;
-
-	snprintf(path, sizeof(path), "/proc/%d/fd", (int) getpid());
-	if ((dir = opendir(path))) {
-		while ((entry = readdir(dir))) {
-			/* Skip . and .. */
-			if (entry->d_name[0] == '.') {
-				continue;
-			}
-			if ((x = strtol(entry->d_name, &result, 10)) && x >= n) {
-#ifdef STRICT_COMPAT
-				close(x);
+#endif
+	struct pollfd fds[POLL_SIZE];
+	int fd=n, loopmax, i;
+#ifndef STRICT_COMPAT
+	long flags;
+#endif
+
+#ifndef _SC_OPEN_MAX
+	if (getrlimit(RLIMIT_NOFILE, &rl) == -1) {
+		maxfd = -1;
+	} else {
+		maxfd = rl.rlim_cur;
+	}
 #else
-				/* This isn't strictly compatible, but it's actually faster
-				 * for our purposes to set the CLOEXEC flag than to close
-				 * file descriptors.
-				 */
-				long flags = fcntl(x, F_GETFD);
-				if (flags == -1 && errno == EBADF) {
-					continue;
-				}
-				fcntl(x, F_SETFD, flags | FD_CLOEXEC);
+	maxfd = sysconf (_SC_OPEN_MAX);
 #endif
-			}
+
+	if (maxfd == -1 || maxfd > 65536) {
+		/* A more reasonable value.  Consider that the primary source of
+		 * file descriptors in Asterisk are UDP sockets, of which we are
+		 * limited to 65,535 per address.  We additionally limit that down
+		 * to about 10,000 sockets per protocol.  While the kernel will
+		 * allow us to set the fileno limit higher (up to 4.2 billion),
+		 * there really is no practical reason for it to be that high.
+		 *
+		 * sysconf as well as getrlimit can return -1 on error. Let's set
+		 * maxfd to the mentioned reasonable value of 65,535 in this case.
+		 */
+		maxfd = 65536;
+	}
+
+	while (fd < maxfd) {
+		loopmax = maxfd - fd;
+		if (loopmax > POLL_SIZE) {
+			loopmax = POLL_SIZE;
 		}
-		closedir(dir);
-	} else {
-		getrlimit(RLIMIT_NOFILE, &rl);
-		if (rl.rlim_cur > 65535) {
-			/* A more reasonable value.  Consider that the primary source of
-			 * file descriptors in Asterisk are UDP sockets, of which we are
-			 * limited to 65,535 per address.  We additionally limit that down
-			 * to about 10,000 sockets per protocol.  While the kernel will
-			 * allow us to set the fileno limit higher (up to 4.2 billion),
-			 * there really is no practical reason for it to be that high.
-			 */
-			rl.rlim_cur = 65535;
+		for (i = 0; i < loopmax; i++) {
+			fds[i].fd = fd+i;
+			fds[i].events = 0;
 		}
-		for (x = n; x < rl.rlim_cur; x++) {
+		poll(fds, loopmax, 0);
+		for (i = 0; i < loopmax; i++) {
+			if (fds[i].revents == POLLNVAL) {
+				continue;
+			}
 #ifdef STRICT_COMPAT
-			close(x);
+			close(fds[i].fd);
 #else
-			long flags = fcntl(x, F_GETFD);
+			/* This isn't strictly compatible, but it's actually faster
+			 * for our purposes to set the CLOEXEC flag than to close
+			 * file descriptors.
+			 */
+			flags = fcntl(fds[i].fd, F_GETFD);
 			if (flags == -1 && errno == EBADF) {
 				continue;
 			}
-			fcntl(x, F_SETFD, flags | FD_CLOEXEC);
+			fcntl(fds[i].fd, F_SETFD, flags | FD_CLOEXEC);
 #endif
 		}
+		fd += loopmax;
 	}
 }
 #endif