Merge branch 'th/bgo737731-kill-process' into th/TESTth/TEST

4 files changed, 215 insertions, 64 deletions

diff --git a/src/NetworkManagerUtils.c b/src/NetworkManagerUtils.c
index f4c3df9f5e..30be6b7c0b 100644
--- a/src/NetworkManagerUtils.c
+++ b/src/NetworkManagerUtils.c
@@ -172,6 +172,67 @@ nm_spawn_process (const char *args)
 	return status;
 }
 
+/**
+ * nm_utils_get_start_time_for_pid:
+ * @pid: the process identifier
+ *
+ * Originally copied from polkit source (src/polkit/polkitunixprocess.c)
+ * and adjusted.
+ *
+ * Returns: the timestamp when the process started (by parsing /proc/$PID/stat).
+ * If an error occurs (e.g. the process does not exist), 0 is returned.
+ **/
+guint64
+nm_utils_get_start_time_for_pid (pid_t pid)
+{
+	guint64 start_time;
+	gchar *filename;
+	gchar *contents;
+	size_t length;
+	gchar **tokens;
+	guint num_tokens;
+	gchar *p;
+	gchar *endp;
+
+	start_time = 0;
+	contents = NULL;
+
+	filename = g_strdup_printf ("/proc/%d/stat", pid);
+
+	if (!g_file_get_contents (filename, &contents, &length, NULL))
+		goto out;
+
+	/* start time is the token at index 19 after the '(process name)' entry - since only this
+	 * field can contain the ')' character, search backwards for this to avoid malicious
+	 * processes trying to fool us
+	 */
+	p = strrchr (contents, ')');
+	if (p == NULL)
+		goto out;
+	p += 2; /* skip ') ' */
+	if (p - contents >= (int) length)
+		goto out;
+
+	tokens = g_strsplit (p, " ", 0);
+
+	num_tokens = g_strv_length (tokens);
+
+	if (num_tokens < 20)
+		goto out;
+
+	start_time = strtoull (tokens[19], &endp, 10);
+	if (endp == tokens[19])
+		goto out;
+
+	g_strfreev (tokens);
+
+ out:
+	g_free (filename);
+	g_free (contents);
+
+	return start_time;
+}
+
 /******************************************************************************************/
 
 typedef struct {
@@ -194,6 +255,7 @@ typedef struct {
 } KillChildAsyncData;
 
 #define LOG_NAME_FMT "kill child process '%s' (%ld)"
+#define LOG_NAME_PROCESS_FMT "kill process '%s' (%ld)"
 #define LOG_NAME_ARGS log_name,(long)pid
 
 static KillChildAsyncData *
@@ -330,7 +392,7 @@ _kc_invoke_callback (pid_t pid, guint64 log_domain, const char *log_name, NMUtil
  * @pid: the process id of the process to kill
  * @sig: signal to send initially. Set to 0 to send not signal.
  * @log_domain: the logging domain used for logging (LOGD_NONE to suppress logging)
- * @log_name: (allow-none): for logging, the name of the processes to kill
+ * @log_name: for logging, the name of the processes to kill
  * @wait_before_kill_msec: Waittime in milliseconds before sending %SIGKILL signal. Set this value
  * to zero, not to send %SIGKILL. If @sig is already %SIGKILL, this parameter is ignored.
  * @callback: (allow-none): callback after the child terminated. This function will always
@@ -422,7 +484,7 @@ nm_utils_kill_child_async (pid_t pid, int sig, guint64 log_domain,
  * second %SIGKILL signal is not sent after @wait_before_kill_msec milliseconds.
  * @log_domain: log debug information for this domain. Errors and warnings are logged both
  * as %LOGD_CORE and @log_domain.
- * @log_name: (allow-none): name of the process to kill for logging.
+ * @log_name: name of the process to kill for logging.
  * @child_status: (out) (allow-none): return the exit status of the child, if no error occured.
  * @wait_before_kill_msec: Waittime in milliseconds before sending %SIGKILL signal. Set this value
  * to zero, not to send %SIGKILL. If @sig is already %SIGKILL, this parameter has not effect.
@@ -588,7 +650,143 @@ out:
 		*child_status = success ? status : -1;
 	return success;
 }
+
+/* nm_utils_kill_process_sync:
+ * @pid: process id to kill
+ * @start_time: the start time of the process to kill (as obtained by nm_utils_get_start_time_for_pid()).
+ * This is an additional argument, to help (somewhat) not to kill the wrong process as @pid
+ * might get recycled. Pass 0.
+ * @sig: signal to sent initially. If 0, no signal is sent. If %SIGKILL, the
+ * second %SIGKILL signal is not sent after @wait_before_kill_msec milliseconds.
+ * @log_domain: log debug information for this domain. Errors and warnings are logged both
+ * as %LOGD_CORE and @log_domain.
+ * @log_name: name of the process to kill for logging.
+ * @wait_before_kill_msec: Waittime in milliseconds before sending %SIGKILL signal. Set this value
+ * to zero, not to send %SIGKILL. If @sig is already %SIGKILL, this parameter has not effect.
+ * @sleep_duration_msec: the synchronous function sleeps repeatedly waiting for the child to terminate.
+ * Set to zero, to use the default (meaning 20 wakeups per seconds).
+ *
+ * Kill a non-child process synchronously and wait. This function will not return before the
+ * process with PID @pid is gone.
+ **/
+void
+nm_utils_kill_process_sync (pid_t pid, guint64 start_time, int sig, guint64 log_domain,
+                            const char *log_name, guint32 wait_before_kill_msec,
+                            guint32 sleep_duration_msec)
+{
+	int errsv;
+	guint64 start_time0;
+	gint64 wait_until, now, wait_start_us;
+	gulong sleep_time, sleep_duration_usec;
+	int loop_count = 0;
+	gboolean was_waiting = FALSE;
+	char buf_wait[KC_WAITED_TO_STRING];
+
+	g_return_if_fail (pid > 0);
+	g_return_if_fail (log_name != NULL);
+	g_return_if_fail (wait_before_kill_msec > 0);
+
+	start_time0 = nm_utils_get_start_time_for_pid (pid);
+	if (start_time0 == 0) {
+		nm_log_dbg (log_domain, LOG_NAME_PROCESS_FMT ": cannot kill process %ld because it seems already gone",
+		            LOG_NAME_ARGS, (long int) pid);
+		return;
+	}
+	if (start_time != 0 && start_time != start_time0) {
+		nm_log_dbg (log_domain, LOG_NAME_PROCESS_FMT ": don't kill process %ld because the start_time is unexpectedly %lu instead of %ld",
+		            LOG_NAME_ARGS, (long int) pid, (long unsigned) start_time0, (long unsigned) start_time);
+		return;
+	}
+
+	if (kill (pid, sig) != 0) {
+		errsv = errno;
+		/* ESRCH means, process does not exist or is already a zombie. */
+		if (errsv == ESRCH) {
+			nm_log_dbg (log_domain, LOG_NAME_PROCESS_FMT ": failed to send %s because process seems gone",
+			            LOG_NAME_ARGS, _kc_signal_to_string (sig));
+		} else {
+			nm_log_warn (LOGD_CORE | log_domain, LOG_NAME_PROCESS_FMT ": failed to send %s: %s (%d)",
+			             LOG_NAME_ARGS, _kc_signal_to_string (sig), strerror (errsv), errsv);
+		}
+		return;
+	}
+
+	/* wait for the process to terminated... */
+
+	wait_start_us = nm_utils_get_monotonic_timestamp_us ();
+
+	sleep_duration_usec = (sleep_duration_msec <= 0) ? (G_USEC_PER_SEC / 20) : MIN (G_MAXULONG, ((gint64) sleep_duration_msec) * 1000L);
+	wait_until = wait_start_us + (((gint64) wait_before_kill_msec) * 1000L);
+
+	while (TRUE) {
+		start_time = nm_utils_get_start_time_for_pid (pid);
+
+		if (start_time != start_time0) {
+			nm_log_dbg (log_domain, LOG_NAME_PROCESS_FMT ": process is gone after sending signal %s%s",
+			            LOG_NAME_ARGS, _kc_signal_to_string (sig),
+			            was_waiting ? _kc_waited_to_string (buf_wait, wait_start_us) : "");
+			return;
+		}
+
+		if (kill (pid, 0) != 0) {
+			errsv = errno;
+			/* ESRCH means, process does not exist or is already a zombie. */
+			if (errsv == ESRCH) {
+				nm_log_dbg (log_domain, LOG_NAME_PROCESS_FMT ": process is gone or a zombie after sending signal %s%s",
+				            LOG_NAME_ARGS, _kc_signal_to_string (sig),
+				            was_waiting ? _kc_waited_to_string (buf_wait, wait_start_us) : "");
+			} else {
+				nm_log_warn (LOGD_CORE | log_domain, LOG_NAME_PROCESS_FMT ": failed to kill(%ld, 0): %s (%d)%s",
+				             LOG_NAME_ARGS, (long int) pid, strerror (errsv), errsv,
+				             was_waiting ? _kc_waited_to_string (buf_wait, wait_start_us) : "");
+			}
+			return;
+		}
+
+		sleep_time = sleep_duration_usec;
+		if (wait_until != 0) {
+			now = nm_utils_get_monotonic_timestamp_us ();
+			if (sig != SIGKILL && now >= wait_until) {
+				/* Still not dead. SIGKILL now... */
+				nm_log_dbg (log_domain, LOG_NAME_PROCESS_FMT ": sending SIGKILL", LOG_NAME_ARGS);
+				if (kill (pid, SIGKILL) != 0) {
+					errsv = errno;
+					/* ESRCH means, process does not exist or is already a zombie. */
+					if (errsv != ESRCH) {
+						nm_log_dbg (log_domain, LOG_NAME_PROCESS_FMT ": process is gone or a zombie%s",
+						            LOG_NAME_ARGS, _kc_waited_to_string (buf_wait, wait_start_us));
+					} else {
+						nm_log_warn (LOGD_CORE | log_domain, LOG_NAME_PROCESS_FMT ": failed to send SIGKILL (after sending %s), %s (%d)%s",
+						             LOG_NAME_ARGS, _kc_signal_to_string (sig), strerror (errsv), errsv,
+						             _kc_waited_to_string (buf_wait, wait_start_us));
+					}
+					return;
+				}
+				sig = SIGKILL;
+				was_waiting = TRUE;
+				wait_until = 0;
+				loop_count = 0; /* reset the loop_count. Now we really expect the process to die quickly. */
+			} else {
+				if (!was_waiting) {
+					nm_log_dbg (log_domain, LOG_NAME_PROCESS_FMT ": waiting up to %ld milliseconds for process to disappear after sending %s...",
+					            LOG_NAME_ARGS, (long) wait_before_kill_msec, _kc_signal_to_string (sig));
+					was_waiting = TRUE;
+				}
+				sleep_time = MIN (wait_until - now, sleep_duration_usec);
+			}
+		}
+
+		if (loop_count < 20) {
+			/* At the beginning we expect the process to die fast.
+			 * Limit the sleep time, the limit doubles with every iteration. */
+			sleep_time = MIN (sleep_time, (((guint64) 1) << loop_count) * G_USEC_PER_SEC / 2000);
+			loop_count++;
+		}
+		g_usleep (sleep_time);
+	}
+}
 #undef LOG_NAME_FMT
+#undef LOG_NAME_PROCESS_FMT
 #undef LOG_NAME_ARGS
 
 /**
diff --git a/src/NetworkManagerUtils.h b/src/NetworkManagerUtils.h
index eead08aae3..d66a1ddd9e 100644
--- a/src/NetworkManagerUtils.h
+++ b/src/NetworkManagerUtils.h
@@ -63,6 +63,12 @@ str_if_set (const char *str, const char *fallback)
 	return str ? str : fallback;
 }
 
+guint64 nm_utils_get_start_time_for_pid (pid_t pid);
+
+void nm_utils_kill_process_sync (pid_t pid, guint64 start_time, int sig, guint64 log_domain,
+                                 const char *log_name, guint32 wait_before_kill_msec,
+                                 guint32 sleep_duration_msec);
+
 typedef void (*NMUtilsKillChildAsyncCb) (pid_t pid, gboolean success, int child_status, void *user_data);
 void nm_utils_kill_child_async (pid_t pid, int sig, guint64 log_domain, const char *log_name,
                                 guint32 wait_before_kill_msec,
diff --git a/src/dhcp-manager/nm-dhcp-client.c b/src/dhcp-manager/nm-dhcp-client.c
index 76700628ec..085f7f3655 100644
--- a/src/dhcp-manager/nm-dhcp-client.c
+++ b/src/dhcp-manager/nm-dhcp-client.c
@@ -216,7 +216,7 @@ nm_dhcp_client_stop_pid (pid_t pid, const char *iface)
 {
 	char *name = iface ? g_strdup_printf ("dhcp-client-%s", iface) : NULL;
 
-	g_return_if_fail (pid > 25);
+	g_return_if_fail (pid > 1);
 
 	nm_utils_kill_child_sync (pid, SIGTERM, LOGD_DHCP, name ? name : "dhcp-client", NULL,
 	                          1000 / 2, 1000 / 20);
@@ -544,11 +544,14 @@ nm_dhcp_client_stop_existing (const char *pid_file, const char *binary_name)
 	errno = 0;
 	tmp = strtol (pid_contents, NULL, 10);
 	if ((errno == 0) && (tmp > 1)) {
+		guint64 start_time;
 		const char *exe;
 
 		/* Ensure the process is a DHCP client */
+		start_time = nm_utils_get_start_time_for_pid (tmp);
 		proc_path = g_strdup_printf ("/proc/%ld/cmdline", tmp);
-		if (g_file_get_contents (proc_path, &proc_contents, NULL, NULL)) {
+		if (   start_time
+		    && g_file_get_contents (proc_path, &proc_contents, NULL, NULL)) {
 			exe = strrchr (proc_contents, '/');
 			if (exe)
 				exe++;
@@ -556,7 +559,8 @@ nm_dhcp_client_stop_existing (const char *pid_file, const char *binary_name)
 				exe = proc_contents;
 
 			if (!strcmp (exe, binary_name))
-				nm_dhcp_client_stop_pid ((pid_t) tmp, NULL);
+				nm_utils_kill_process_sync (tmp, start_time, SIGTERM, LOGD_DHCP,
+				                            "dhcp-client", 1000 / 2, 1000 / 20);
 		}
 	}
 
diff --git a/src/nm-auth-subject.c b/src/nm-auth-subject.c
index 79b7e2a174..779b711e63 100644
--- a/src/nm-auth-subject.c
+++ b/src/nm-auth-subject.c
@@ -35,6 +35,7 @@
 #include "nm-dbus-manager.h"
 #include "nm-enum-types.h"
 #include "nm-glib-compat.h"
+#include "NetworkManagerUtils.h"
 
 G_DEFINE_TYPE (NMAuthSubject, nm_auth_subject, G_TYPE_OBJECT)
 
@@ -60,64 +61,6 @@ typedef struct {
 	} unix_process;
 } NMAuthSubjectPrivate;
 
-
-
-/**************************************************************/
-
-/* copied from polkit source (src/polkit/polkitunixprocess.c)
- * and adjusted.
- */
-static guint64
-get_start_time_for_pid (pid_t pid)
-{
-	guint64 start_time;
-	gchar *filename;
-	gchar *contents;
-	size_t length;
-	gchar **tokens;
-	guint num_tokens;
-	gchar *p;
-	gchar *endp;
-
-	start_time = 0;
-	contents = NULL;
-
-	filename = g_strdup_printf ("/proc/%d/stat", pid);
-
-	if (!g_file_get_contents (filename, &contents, &length, NULL))
-		goto out;
-
-	/* start time is the token at index 19 after the '(process name)' entry - since only this
-	 * field can contain the ')' character, search backwards for this to avoid malicious
-	 * processes trying to fool us
-	 */
-	p = strrchr (contents, ')');
-	if (p == NULL)
-		goto out;
-	p += 2; /* skip ') ' */
-	if (p - contents >= (int) length)
-		goto out;
-
-	tokens = g_strsplit (p, " ", 0);
-
-	num_tokens = g_strv_length (tokens);
-
-	if (num_tokens < 20)
-		goto out;
-
-	start_time = strtoull (tokens[19], &endp, 10);
-	if (endp == tokens[19])
-		goto out;
-
-	g_strfreev (tokens);
-
- out:
-	g_free (filename);
-	g_free (contents);
-
-	return start_time;
-}
-
 /**************************************************************/
 
 #define CHECK_SUBJECT(self, error_value) \
@@ -414,7 +357,7 @@ constructed (GObject *object)
 		if (!priv->unix_process.dbus_sender || !*priv->unix_process.dbus_sender)
 			break;
 
-		priv->unix_process.start_time = get_start_time_for_pid (priv->unix_process.pid);
+		priv->unix_process.start_time = nm_utils_get_start_time_for_pid (priv->unix_process.pid);
 
 		if (!priv->unix_process.start_time) {
 			/* could not detect the process start time. The subject is invalid, but don't