<p>Friendly Automation <strong>submitted</strong> this change.</p><p><a href="https://gerrit.asterisk.org/c/asterisk/+/14392">View Change</a></p><div style="white-space:pre-wrap">Approvals:
George Joseph: Looks good to me, approved
Friendly Automation: Approved for Submit
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">app.c: make sure that no non-async-signal-safe syscalls are used after<br>fork before exec<br><br>Posix does only allow async-signal-safe syscalls after fork before exec.<br>As asterisk ignores this, functions like TrySystem or System sometimes<br>end up in a deadlocked child process. The patch prevents the use of<br>non-async-signal-safe syscalls.<br><br>ASTERISK-28776<br><br>Change-Id: Idc76365c0592ee3f3b3bd72a4f48f7a098978e8e<br>---<br>M main/app.c<br>M main/asterisk.c<br>M main/strcompat.c<br>3 files changed, 75 insertions(+), 58 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/main/app.c b/main/app.c</span><br><span>index ae3f416..09c0123 100644</span><br><span>--- a/main/app.c</span><br><span>+++ b/main/app.c</span><br><span>@@ -81,6 +81,9 @@</span><br><span> </span><br><span> static AST_LIST_HEAD_STATIC(zombies, zombie);</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+#ifdef HAVE_CAP</span><br><span style="color: hsl(120, 100%, 40%);">+static cap_t child_cap;</span><br><span style="color: hsl(120, 100%, 40%);">+#endif</span><br><span> /*</span><br><span> * @{ \brief Define \ref stasis topic objects</span><br><span> */</span><br><span>@@ -3003,12 +3006,7 @@</span><br><span> } else {</span><br><span> /* Child */</span><br><span> #ifdef HAVE_CAP</span><br><span style="color: hsl(0, 100%, 40%);">- cap_t cap = cap_from_text("cap_net_admin-eip");</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- if (cap_set_proc(cap)) {</span><br><span style="color: hsl(0, 100%, 40%);">- ast_log(LOG_WARNING, "Unable to remove capabilities.\n");</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(0, 100%, 40%);">- cap_free(cap);</span><br><span style="color: hsl(120, 100%, 40%);">+ cap_set_proc(child_cap);</span><br><span> #endif</span><br><span> </span><br><span> /* Before we unblock our signals, return our trapped signals back to the defaults */</span><br><span>@@ -3118,6 +3116,9 @@</span><br><span> </span><br><span> static void app_cleanup(void)</span><br><span> {</span><br><span style="color: hsl(120, 100%, 40%);">+#ifdef HAS_CAP</span><br><span style="color: hsl(120, 100%, 40%);">+ cap_free(child_cap);</span><br><span style="color: hsl(120, 100%, 40%);">+#endif</span><br><span> ao2_cleanup(queue_topic_pool);</span><br><span> queue_topic_pool = NULL;</span><br><span> ao2_cleanup(queue_topic_all);</span><br><span>@@ -3127,7 +3128,9 @@</span><br><span> int app_init(void)</span><br><span> {</span><br><span> ast_register_cleanup(app_cleanup);</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(120, 100%, 40%);">+#ifdef HAVE_CAP</span><br><span style="color: hsl(120, 100%, 40%);">+ child_cap = cap_from_text("cap_net_admin-eip");</span><br><span style="color: hsl(120, 100%, 40%);">+#endif</span><br><span> queue_topic_all = stasis_topic_create("queue:all");</span><br><span> if (!queue_topic_all) {</span><br><span> return -1;</span><br><span>diff --git a/main/asterisk.c b/main/asterisk.c</span><br><span>index 6ae9219..000e1a2 100644</span><br><span>--- a/main/asterisk.c</span><br><span>+++ b/main/asterisk.c</span><br><span>@@ -388,6 +388,10 @@</span><br><span> </span><br><span> static char randompool[256];</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+#ifdef HAVE_CAP</span><br><span style="color: hsl(120, 100%, 40%);">+static cap_t child_cap;</span><br><span style="color: hsl(120, 100%, 40%);">+#endif</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> static int sig_alert_pipe[2] = { -1, -1 };</span><br><span> static struct {</span><br><span> unsigned int need_reload:1;</span><br><span>@@ -1099,13 +1103,7 @@</span><br><span> </span><br><span> if (pid == 0) {</span><br><span> #ifdef HAVE_CAP</span><br><span style="color: hsl(0, 100%, 40%);">- cap_t cap = cap_from_text("cap_net_admin-eip");</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- if (cap_set_proc(cap)) {</span><br><span style="color: hsl(0, 100%, 40%);">- /* Careful with order! Logging cannot happen after we close FDs */</span><br><span style="color: hsl(0, 100%, 40%);">- ast_log(LOG_WARNING, "Unable to remove capabilities.\n");</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(0, 100%, 40%);">- cap_free(cap);</span><br><span style="color: hsl(120, 100%, 40%);">+ cap_set_proc(child_cap);</span><br><span> #endif</span><br><span> #ifdef HAVE_WORKING_FORK</span><br><span> if (ast_opt_high_priority) {</span><br><span>@@ -1804,10 +1802,8 @@</span><br><span> if (pri) {</span><br><span> sched.sched_priority = 10;</span><br><span> if (sched_setscheduler(0, SCHED_RR, &sched)) {</span><br><span style="color: hsl(0, 100%, 40%);">- ast_log(LOG_WARNING, "Unable to set high priority\n");</span><br><span> return -1;</span><br><span style="color: hsl(0, 100%, 40%);">- } else</span><br><span style="color: hsl(0, 100%, 40%);">- ast_verb(1, "Set to realtime thread\n");</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span> } else {</span><br><span> sched.sched_priority = 0;</span><br><span> /* According to the manpage, these parameters can never fail. */</span><br><span>@@ -3920,8 +3916,14 @@</span><br><span> exit(1);</span><br><span> }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+#ifdef HAVE_CAP</span><br><span style="color: hsl(120, 100%, 40%);">+ child_cap = cap_from_text("cap_net_admin-eip");</span><br><span style="color: hsl(120, 100%, 40%);">+#endif</span><br><span> /* Not a remote console? Start the daemon. */</span><br><span> asterisk_daemon(isroot, runuser, rungroup);</span><br><span style="color: hsl(120, 100%, 40%);">+#ifdef HAS_CAP</span><br><span style="color: hsl(120, 100%, 40%);">+ cap_free(child_cap);</span><br><span style="color: hsl(120, 100%, 40%);">+#endif</span><br><span> return 0;</span><br><span> }</span><br><span> </span><br><span>diff --git a/main/strcompat.c b/main/strcompat.c</span><br><span>index 0034c21..877c11c 100644</span><br><span>--- a/main/strcompat.c</span><br><span>+++ b/main/strcompat.c</span><br><span>@@ -38,6 +38,8 @@</span><br><span> </span><br><span> #include "asterisk/utils.h"</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+#define POLL_SIZE 1024</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> #ifndef HAVE_STRSEP</span><br><span> char *strsep(char **str, const char *delims)</span><br><span> {</span><br><span>@@ -426,59 +428,69 @@</span><br><span> #ifndef HAVE_CLOSEFROM</span><br><span> void closefrom(int n)</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">- long x;</span><br><span style="color: hsl(120, 100%, 40%);">+ int maxfd;</span><br><span style="color: hsl(120, 100%, 40%);">+#ifndef _SC_OPEN_MAX</span><br><span> struct rlimit rl;</span><br><span style="color: hsl(0, 100%, 40%);">- DIR *dir;</span><br><span style="color: hsl(0, 100%, 40%);">- char path[16], *result;</span><br><span style="color: hsl(0, 100%, 40%);">- struct dirent *entry;</span><br><span style="color: hsl(120, 100%, 40%);">+#endif</span><br><span style="color: hsl(120, 100%, 40%);">+ struct pollfd fds[POLL_SIZE];</span><br><span style="color: hsl(120, 100%, 40%);">+ int fd=n, loopmax, i;</span><br><span style="color: hsl(120, 100%, 40%);">+#ifndef STRICT_COMPAT</span><br><span style="color: hsl(120, 100%, 40%);">+ long flags;</span><br><span style="color: hsl(120, 100%, 40%);">+#endif</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- snprintf(path, sizeof(path), "/proc/%d/fd", (int) getpid());</span><br><span style="color: hsl(0, 100%, 40%);">- if ((dir = opendir(path))) {</span><br><span style="color: hsl(0, 100%, 40%);">- while ((entry = readdir(dir))) {</span><br><span style="color: hsl(0, 100%, 40%);">- /* Skip . and .. */</span><br><span style="color: hsl(0, 100%, 40%);">- if (entry->d_name[0] == '.') {</span><br><span style="color: hsl(120, 100%, 40%);">+#ifndef _SC_OPEN_MAX</span><br><span style="color: hsl(120, 100%, 40%);">+ if (getrlimit(RLIMIT_NOFILE, &rl) == -1) {</span><br><span style="color: hsl(120, 100%, 40%);">+ maxfd = -1;</span><br><span style="color: hsl(120, 100%, 40%);">+ } else {</span><br><span style="color: hsl(120, 100%, 40%);">+ maxfd = rl.rlim_cur;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+#else</span><br><span style="color: hsl(120, 100%, 40%);">+ maxfd = sysconf (_SC_OPEN_MAX);</span><br><span style="color: hsl(120, 100%, 40%);">+#endif</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ if (maxfd == -1 || maxfd > 65536) {</span><br><span style="color: hsl(120, 100%, 40%);">+ /* A more reasonable value. Consider that the primary source of</span><br><span style="color: hsl(120, 100%, 40%);">+ * file descriptors in Asterisk are UDP sockets, of which we are</span><br><span style="color: hsl(120, 100%, 40%);">+ * limited to 65,535 per address. We additionally limit that down</span><br><span style="color: hsl(120, 100%, 40%);">+ * to about 10,000 sockets per protocol. While the kernel will</span><br><span style="color: hsl(120, 100%, 40%);">+ * allow us to set the fileno limit higher (up to 4.2 billion),</span><br><span style="color: hsl(120, 100%, 40%);">+ * there really is no practical reason for it to be that high.</span><br><span style="color: hsl(120, 100%, 40%);">+ *</span><br><span style="color: hsl(120, 100%, 40%);">+ * sysconf as well as getrlimit can return -1 on error. Let's set</span><br><span style="color: hsl(120, 100%, 40%);">+ * maxfd to the mentioned reasonable value of 65,535 in this case.</span><br><span style="color: hsl(120, 100%, 40%);">+ */</span><br><span style="color: hsl(120, 100%, 40%);">+ maxfd = 65536;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ while (fd < maxfd) {</span><br><span style="color: hsl(120, 100%, 40%);">+ loopmax = maxfd - fd;</span><br><span style="color: hsl(120, 100%, 40%);">+ if (loopmax > POLL_SIZE) {</span><br><span style="color: hsl(120, 100%, 40%);">+ loopmax = POLL_SIZE;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+ for (i = 0; i < loopmax; i++) {</span><br><span style="color: hsl(120, 100%, 40%);">+ fds[i].fd = fd+i;</span><br><span style="color: hsl(120, 100%, 40%);">+ fds[i].events = 0;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+ poll(fds, loopmax, 0);</span><br><span style="color: hsl(120, 100%, 40%);">+ for (i = 0; i < loopmax; i++) {</span><br><span style="color: hsl(120, 100%, 40%);">+ if (fds[i].revents == POLLNVAL) {</span><br><span> continue;</span><br><span> }</span><br><span style="color: hsl(0, 100%, 40%);">- if ((x = strtol(entry->d_name, &result, 10)) && x >= n) {</span><br><span> #ifdef STRICT_COMPAT</span><br><span style="color: hsl(0, 100%, 40%);">- close(x);</span><br><span style="color: hsl(120, 100%, 40%);">+ close(fds[i].fd);</span><br><span> #else</span><br><span style="color: hsl(0, 100%, 40%);">- /* This isn't strictly compatible, but it's actually faster</span><br><span style="color: hsl(0, 100%, 40%);">- * for our purposes to set the CLOEXEC flag than to close</span><br><span style="color: hsl(0, 100%, 40%);">- * file descriptors.</span><br><span style="color: hsl(0, 100%, 40%);">- */</span><br><span style="color: hsl(0, 100%, 40%);">- long flags = fcntl(x, F_GETFD);</span><br><span style="color: hsl(0, 100%, 40%);">- if (flags == -1 && errno == EBADF) {</span><br><span style="color: hsl(0, 100%, 40%);">- continue;</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(0, 100%, 40%);">- fcntl(x, F_SETFD, flags | FD_CLOEXEC);</span><br><span style="color: hsl(0, 100%, 40%);">-#endif</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(0, 100%, 40%);">- closedir(dir);</span><br><span style="color: hsl(0, 100%, 40%);">- } else {</span><br><span style="color: hsl(0, 100%, 40%);">- getrlimit(RLIMIT_NOFILE, &rl);</span><br><span style="color: hsl(0, 100%, 40%);">- if (rl.rlim_cur > 65535) {</span><br><span style="color: hsl(0, 100%, 40%);">- /* A more reasonable value. Consider that the primary source of</span><br><span style="color: hsl(0, 100%, 40%);">- * file descriptors in Asterisk are UDP sockets, of which we are</span><br><span style="color: hsl(0, 100%, 40%);">- * limited to 65,535 per address. We additionally limit that down</span><br><span style="color: hsl(0, 100%, 40%);">- * to about 10,000 sockets per protocol. While the kernel will</span><br><span style="color: hsl(0, 100%, 40%);">- * allow us to set the fileno limit higher (up to 4.2 billion),</span><br><span style="color: hsl(0, 100%, 40%);">- * there really is no practical reason for it to be that high.</span><br><span style="color: hsl(120, 100%, 40%);">+ /* This isn't strictly compatible, but it's actually faster</span><br><span style="color: hsl(120, 100%, 40%);">+ * for our purposes to set the CLOEXEC flag than to close</span><br><span style="color: hsl(120, 100%, 40%);">+ * file descriptors.</span><br><span> */</span><br><span style="color: hsl(0, 100%, 40%);">- rl.rlim_cur = 65535;</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(0, 100%, 40%);">- for (x = n; x < rl.rlim_cur; x++) {</span><br><span style="color: hsl(0, 100%, 40%);">-#ifdef STRICT_COMPAT</span><br><span style="color: hsl(0, 100%, 40%);">- close(x);</span><br><span style="color: hsl(0, 100%, 40%);">-#else</span><br><span style="color: hsl(0, 100%, 40%);">- long flags = fcntl(x, F_GETFD);</span><br><span style="color: hsl(120, 100%, 40%);">+ flags = fcntl(fds[i].fd, F_GETFD);</span><br><span> if (flags == -1 && errno == EBADF) {</span><br><span> continue;</span><br><span> }</span><br><span style="color: hsl(0, 100%, 40%);">- fcntl(x, F_SETFD, flags | FD_CLOEXEC);</span><br><span style="color: hsl(120, 100%, 40%);">+ fcntl(fds[i].fd, F_SETFD, flags | FD_CLOEXEC);</span><br><span> #endif</span><br><span> }</span><br><span style="color: hsl(120, 100%, 40%);">+ fd += loopmax;</span><br><span> }</span><br><span> }</span><br><span> #endif</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.asterisk.org/c/asterisk/+/14392">change 14392</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.asterisk.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.asterisk.org/c/asterisk/+/14392"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: 17 </div>
<div style="display:none"> Gerrit-Change-Id: Idc76365c0592ee3f3b3bd72a4f48f7a098978e8e </div>
<div style="display:none"> Gerrit-Change-Number: 14392 </div>
<div style="display:none"> Gerrit-PatchSet: 2 </div>
<div style="display:none"> Gerrit-Owner: Joshua Colp <jcolp@sangoma.com> </div>
<div style="display:none"> Gerrit-Reviewer: Friendly Automation </div>
<div style="display:none"> Gerrit-Reviewer: George Joseph <gjoseph@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: nappsoft <infos@nappsoft.ch> </div>
<div style="display:none"> Gerrit-MessageType: merged </div>