[Asterisk-code-review] asterisk.c: Non-root users also get the astcanary after core... (asterisk[master])

[Anonymous Coward]

Anonymous Coward #1000019 has submitted this change and it was merged.

Change subject: asterisk.c: Non-root users also get the astcanary after core restart.
......................................................................


asterisk.c: Non-root users also get the astcanary after core restart.

Without this change, a 'core restart' would kill the astcanary forever
if you're not running as root. Both with and without this patch, the
scheduling priority was still SCHED_RR after restart.

Additionally, the astcanary is now spawned if you start with high
priority and Asterisk doesn't get a chance to lower it. For example
through: `chrt -r 10 sudo -u asterisk asterisk -c`

Also reap killed astcanary processes on core restart.

ASTERISK-26352 #close

Change-Id: Iacb49f26491a0717084ad46ed96b0bea5f627a55
---
M main/asterisk.c
1 file changed, 33 insertions(+), 2 deletions(-)

Approvals:
  Anonymous Coward #1000019: Verified
  Joshua Colp: Looks good to me, approved
  Corey Farrell: Looks good to me, but someone else must approve



diff --git a/main/asterisk.c b/main/asterisk.c
index 37a69df..974c51d 100644
--- a/main/asterisk.c
+++ b/main/asterisk.c
@@ -1778,6 +1778,26 @@
 		fprintf(stdout, "\033]1;%s\007", text);
 }
 
+/*! \brief Check whether we were set to high(er) priority. */
+static int has_priority(void)
+{
+	/* Neither of these calls should fail with these arguments. */
+#ifdef __linux__
+	/* For SCHED_OTHER, SCHED_BATCH and SCHED_IDLE, this will return
+	 * 0. For the realtime priorities SCHED_RR and SCHED_FIFO, it
+	 * will return something >= 1. */
+	return sched_getscheduler(0);
+#else
+	/* getpriority() can return a value in -20..19 (or even -INF..20)
+	 * where negative numbers are high priority. We don't bother
+	 * checking errno. If the query fails and it returns -1, we'll
+	 * assume that we're running at high prio; a safe assumption
+	 * that will enable the resource starvation monitor (canary)
+	 * just in case. */
+	return (getpriority(PRIO_PROCESS, 0) < 0);
+#endif
+}
+
 /*! \brief Set priority on all known threads. */
 static int set_priority_all(int pri)
 {
@@ -3796,8 +3816,11 @@
 /* Used by libc's atexit(3) function */
 static void canary_exit(void)
 {
-	if (canary_pid > 0)
+	if (canary_pid > 0) {
+		int status;
 		kill(canary_pid, SIGKILL);
+		waitpid(canary_pid, &status, 0);
+	}
 }
 
 /* Execute CLI commands on startup.  Run by main() thread. */
@@ -4340,8 +4363,16 @@
 	__ast_mm_init_phase_1();
 #endif	/* defined(__AST_DEBUG_MALLOC) */
 
+	/* Check whether high prio was succesfully set by us or some
+	 * other incantation. */
+	if (has_priority()) {
+		ast_set_flag(&ast_options, AST_OPT_FLAG_HIGH_PRIORITY);
+	} else {
+		ast_clear_flag(&ast_options, AST_OPT_FLAG_HIGH_PRIORITY);
+	}
+
 	/* Spawning of astcanary must happen AFTER the call to daemon(3) */
-	if (isroot && ast_opt_high_priority) {
+	if (ast_opt_high_priority) {
 		snprintf(canary_filename, sizeof(canary_filename), "%s/alt.asterisk.canary.tweet.tweet.tweet", ast_config_AST_RUN_DIR);
 
 		/* Don't let the canary child kill Asterisk, if it dies immediately */

-- 
To view, visit https://gerrit.asterisk.org/3920
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Iacb49f26491a0717084ad46ed96b0bea5f627a55
Gerrit-PatchSet: 4
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Walter Doekes <walter+asterisk at wjd.nu>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Walter Doekes <walter+asterisk at wjd.nu>