From 941d7c51d73f82b677d82e3080e191360d4678f5 Mon Sep 17 00:00:00 2001
From: Ariadne Conill <ariadne@dereferenced.org>
Date: Sun, 19 Sep 2021 05:57:16 -0500
Subject: [PATCH] implement execution timeouts for executors

Previously, it was possible for an executor to hang forever.  To mitigate
this, we now implement process execution timeouts for executors, by looping
with waitpid(..., WNOHANG) and sleeping.

This could be implemented more efficiently with process descriptors, see
pidfd_open(2), but that interface is Linux-specific and is only available
on Linux 5.3 or newer.
---
 cmd/multicall-exec-options.c | 14 +++++++++++++-
 libifupdown/execute.c        | 35 +++++++++++++++++++++++++++--------
 libifupdown/execute.h        |  1 +
 libifupdown/lifecycle.c      |  3 ++-
 4 files changed, 43 insertions(+), 10 deletions(-)

diff --git a/cmd/multicall-exec-options.c b/cmd/multicall-exec-options.c
index f56228c..5350e62 100644
--- a/cmd/multicall-exec-options.c
+++ b/cmd/multicall-exec-options.c
@@ -21,10 +21,13 @@
 #include <getopt.h>
 #include "cmd/multicall.h"
 
+#define DEFAULT_TIMEOUT		300
+
 struct lif_execute_opts exec_opts = {
 	.interfaces_file = INTERFACES_FILE,
 	.executor_path = EXECUTOR_PATH,
-	.state_file = STATE_FILE
+	.state_file = STATE_FILE,
+	.timeout = DEFAULT_TIMEOUT,
 };
 
 static void
@@ -74,6 +77,14 @@ set_force(const char *opt_arg)
 	exec_opts.force = true;
 }
 
+static void
+set_timeout(const char *opt_arg)
+{
+	exec_opts.timeout = atoi(opt_arg);
+	if (exec_opts.timeout < 0)
+		exec_opts.timeout = DEFAULT_TIMEOUT;
+}
+
 static struct if_option exec_options[] = {
 	{'f', "force", NULL, "force (de)configuration", false, set_force},
 	{'i', "interfaces", "interfaces FILE", "use FILE for interface definitions", true, set_interfaces_file},
@@ -82,6 +93,7 @@ static struct if_option exec_options[] = {
 	{'v', "verbose", NULL, "show what commands are being run", false, set_verbose},
 	{'E', "executor-path", "executor-path PATH", "use PATH for executor directory", true, set_executor_path},
 	{'S', "state-file", "state-file FILE", "use FILE for state", true, set_state_file},
+	{'T', "timeout", "timeout TIMEOUT", "wait TIMEOUT seconds for executors to complete", true, set_timeout},
 };
 
 struct if_option_group exec_option_group = {
diff --git a/libifupdown/execute.c b/libifupdown/execute.c
index 69af1c0..2f7229a 100644
--- a/libifupdown/execute.c
+++ b/libifupdown/execute.c
@@ -30,6 +30,31 @@
 
 #define SHELL	"/bin/sh"
 
+/* TODO: Add support for Linux process descriptors once it is okay to require
+ * Linux 5.3 or newer.
+ */
+static inline bool
+lif_process_monitor(const char *cmdbuf, pid_t child, int timeout_sec)
+{
+	int status;
+	int ticks = 0;
+
+	while (ticks < timeout_sec)
+	{
+		if (waitpid(child, &status, WNOHANG) == child)
+			return WIFEXITED(status) && WEXITSTATUS(status) == 0;
+
+		sleep(1);
+		ticks++;
+	}
+
+	fprintf(stderr, "execution of '%s': timeout after %d seconds\n", cmdbuf, timeout_sec);
+	kill(child, SIGKILL);
+	waitpid(child, &status, 0);
+
+	return false;
+}
+
 bool
 lif_execute_fmt(const struct lif_execute_opts *opts, char *const envp[], const char *fmt, ...)
 {
@@ -55,10 +80,7 @@ lif_execute_fmt(const struct lif_execute_opts *opts, char *const envp[], const c
 		return false;
 	}
 
-	int status;
-	waitpid(child, &status, 0);
-
-	return WIFEXITED(status) && WEXITSTATUS(status) == 0;
+	return lif_process_monitor(cmdbuf, child, opts->timeout);
 }
 
 bool
@@ -118,11 +140,8 @@ lif_execute_fmt_with_result(const struct lif_execute_opts *opts, char *buf, size
 		return false;
 	}
 
-	int status;
 no_result:
-	waitpid(child, &status, 0);
-
-	return WIFEXITED(status) && WEXITSTATUS(status) == 0;
+	return lif_process_monitor(cmdbuf, child, opts->timeout);
 }
 
 bool
diff --git a/libifupdown/execute.h b/libifupdown/execute.h
index 5a871a9..905cd84 100644
--- a/libifupdown/execute.h
+++ b/libifupdown/execute.h
@@ -27,6 +27,7 @@ struct lif_execute_opts {
 	const char *executor_path;
 	const char *interfaces_file;
 	const char *state_file;
+	int timeout;
 };
 
 extern bool lif_execute_fmt(const struct lif_execute_opts *opts, char *const envp[], const char *fmt, ...);
diff --git a/libifupdown/lifecycle.c b/libifupdown/lifecycle.c
index 4be5678..6bc9ffe 100644
--- a/libifupdown/lifecycle.c
+++ b/libifupdown/lifecycle.c
@@ -94,7 +94,8 @@ query_dependents_from_executors(const struct lif_execute_opts *opts, char *const
 		struct lif_execute_opts exec_opts = {
 			.verbose = opts->verbose,
 			.executor_path = opts->executor_path,
-			.interfaces_file = opts->interfaces_file
+			.interfaces_file = opts->interfaces_file,
+			.timeout = opts->timeout,
 		};
 
 		if (strcmp(entry->key, "use"))