core: merge branch 'th/device-cleanup-bgo751733'

https://bugzilla.gnome.org/show_bug.cgi?id=751733

5 files changed, 60 insertions, 38 deletions

diff --git a/src/NetworkManagerUtils.c b/src/NetworkManagerUtils.c
index 79c173825b..5a129215f7 100644
--- a/src/NetworkManagerUtils.c
+++ b/src/NetworkManagerUtils.c
@@ -358,6 +358,7 @@ nm_utils_modprobe (GError **error, gboolean suppress_error_logging, const char *
  * nm_utils_get_start_time_for_pid:
  * @pid: the process identifier
  * @out_state: return the state character, like R, S, Z. See `man 5 proc`.
+ * @out_ppid: parent process id
  *
  * Originally copied from polkit source (src/polkit/polkitunixprocess.c)
  * and adjusted.
@@ -368,7 +369,7 @@ nm_utils_modprobe (GError **error, gboolean suppress_error_logging, const char *
  * The returned start time counts since boot, in the unit HZ (with HZ usually being (1/100) seconds)
  **/
 guint64
-nm_utils_get_start_time_for_pid (pid_t pid, char *out_state)
+nm_utils_get_start_time_for_pid (pid_t pid, char *out_state, pid_t *out_ppid)
 {
 	guint64 start_time;
 	gs_free gchar *filename = NULL;
@@ -379,6 +380,7 @@ nm_utils_get_start_time_for_pid (pid_t pid, char *out_state)
 	gchar *p;
 	gchar *endp;
 	char state = '\0';
+	gint64 ppid = 0;
 
 	start_time = 0;
 	contents = NULL;
@@ -410,6 +412,9 @@ nm_utils_get_start_time_for_pid (pid_t pid, char *out_state)
 	if (num_tokens < 20)
 		goto out;
 
+	if (out_ppid)
+		ppid = _nm_utils_ascii_str_to_int64 (tokens[1], 10, 1, G_MAXINT, 0);
+
 	errno = 0;
 	start_time = strtoull (tokens[19], &endp, 10);
 	if (*endp != '\0' || errno != 0)
@@ -418,6 +423,8 @@ nm_utils_get_start_time_for_pid (pid_t pid, char *out_state)
 out:
 	if (out_state)
 		*out_state = state;
+	if (out_ppid)
+		*out_ppid = ppid;
 
 	return start_time;
 }
@@ -895,7 +902,7 @@ nm_utils_kill_process_sync (pid_t pid, guint64 start_time, int sig, NMLogDomain
 	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, &p_state);
+	start_time0 = nm_utils_get_start_time_for_pid (pid, &p_state, NULL);
 	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);
@@ -948,7 +955,7 @@ nm_utils_kill_process_sync (pid_t pid, guint64 start_time, int sig, NMLogDomain
 		max_wait_until = 0;
 
 	while (TRUE) {
-		start_time = nm_utils_get_start_time_for_pid (pid, &p_state);
+		start_time = nm_utils_get_start_time_for_pid (pid, &p_state, NULL);
 
 		if (start_time != start_time0) {
 			nm_log_dbg (log_domain, LOG_NAME_PROCESS_FMT ": process is gone after sending signal %s%s",
diff --git a/src/NetworkManagerUtils.h b/src/NetworkManagerUtils.h
index fef5d64cfb..4c4f55b96e 100644
--- a/src/NetworkManagerUtils.h
+++ b/src/NetworkManagerUtils.h
@@ -69,7 +69,7 @@ str_if_set (const char *str, const char *fallback)
 	return str ? str : fallback;
 }
 
-guint64 nm_utils_get_start_time_for_pid (pid_t pid, char *out_state);
+guint64 nm_utils_get_start_time_for_pid (pid_t pid, char *out_state, pid_t *out_ppid);
 
 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,
diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c
index af6d340cad..afe2fcd0a2 100644
--- a/src/devices/nm-device.c
+++ b/src/devices/nm-device.c
@@ -141,6 +141,12 @@ enum {
 #define PENDING_ACTION_AUTOCONF6 "autoconf6"
 
 typedef enum {
+	CLEANUP_TYPE_DECONFIGURE,
+	CLEANUP_TYPE_KEEP,
+	CLEANUP_TYPE_REMOVED,
+} CleanupType;
+
+typedef enum {
 	IP_NONE = 0,
 	IP_WAIT,
 	IP_CONF,
@@ -3198,7 +3204,7 @@ ensure_con_ip6_config (NMDevice *self)
 /* DHCPv4 stuff */
 
 static void
-dhcp4_cleanup (NMDevice *self, gboolean stop, gboolean release)
+dhcp4_cleanup (NMDevice *self, CleanupType cleanup_type, gboolean release)
 {
 	NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self);
 
@@ -3211,7 +3217,8 @@ dhcp4_cleanup (NMDevice *self, gboolean stop, gboolean release)
 
 		nm_device_remove_pending_action (self, PENDING_ACTION_DHCP4, FALSE);
 
-		if (stop)
+		if (   cleanup_type == CLEANUP_TYPE_DECONFIGURE
+		    || cleanup_type == CLEANUP_TYPE_REMOVED)
 			nm_dhcp_client_stop (priv->dhcp4_client, release);
 
 		g_clear_object (&priv->dhcp4_client);
@@ -3410,7 +3417,7 @@ dhcp4_fail (NMDevice *self, gboolean timeout)
 {
 	NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self);
 
-	dhcp4_cleanup (self, TRUE, FALSE);
+	dhcp4_cleanup (self, CLEANUP_TYPE_DECONFIGURE, FALSE);
 	if (timeout || (priv->ip4_state == IP_CONF))
 		nm_device_activate_schedule_ip4_config_timeout (self);
 	else if (priv->ip4_state == IP_DONE)
@@ -3557,7 +3564,7 @@ nm_device_dhcp4_renew (NMDevice *self, gboolean release)
 	_LOGI (LOGD_DHCP4, "DHCPv4 lease renewal requested");
 
 	/* Terminate old DHCP instance and release the old lease */
-	dhcp4_cleanup (self, TRUE, release);
+	dhcp4_cleanup (self, CLEANUP_TYPE_DECONFIGURE, release);
 
 	connection = nm_device_get_connection (self);
 	g_assert (connection);
@@ -3812,7 +3819,7 @@ act_stage3_ip4_config_start (NMDevice *self,
 /* DHCPv6 stuff */
 
 static void
-dhcp6_cleanup (NMDevice *self, gboolean stop, gboolean release)
+dhcp6_cleanup (NMDevice *self, CleanupType cleanup_type, gboolean release)
 {
 	NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self);
 
@@ -3825,7 +3832,8 @@ dhcp6_cleanup (NMDevice *self, gboolean stop, gboolean release)
 			priv->dhcp6_state_sigid = 0;
 		}
 
-		if (stop)
+		if (   cleanup_type == CLEANUP_TYPE_DECONFIGURE
+		    || cleanup_type == CLEANUP_TYPE_REMOVED)
 			nm_dhcp_client_stop (priv->dhcp6_client, release);
 
 		g_clear_object (&priv->dhcp6_client);
@@ -4032,7 +4040,7 @@ dhcp6_fail (NMDevice *self, gboolean timeout)
 {
 	NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self);
 
-	dhcp6_cleanup (self, TRUE, FALSE);
+	dhcp6_cleanup (self, CLEANUP_TYPE_DECONFIGURE, FALSE);
 
 	if (priv->dhcp6_mode == NM_RDISC_DHCP_LEVEL_MANAGED) {
 		if (timeout || (priv->ip6_state == IP_CONF))
@@ -4057,7 +4065,7 @@ dhcp6_timeout (NMDevice *self, NMDhcpClient *client)
 		dhcp6_fail (self, TRUE);
 	else {
 		/* not a hard failure; just live with the RA info */
-		dhcp6_cleanup (self, TRUE, FALSE);
+		dhcp6_cleanup (self, CLEANUP_TYPE_DECONFIGURE, FALSE);
 		if (priv->ip6_state == IP_CONF)
 			nm_device_activate_schedule_ip6_config_result (self);
 	}
@@ -4232,7 +4240,7 @@ nm_device_dhcp6_renew (NMDevice *self, gboolean release)
 	_LOGI (LOGD_DHCP6, "DHCPv6 lease renewal requested");
 
 	/* Terminate old DHCP instance and release the old lease */
-	dhcp6_cleanup (self, TRUE, release);
+	dhcp6_cleanup (self, CLEANUP_TYPE_DECONFIGURE, release);
 
 	/* Start DHCP again on the interface */
 	return dhcp6_start (self, FALSE, NULL);
@@ -4613,7 +4621,7 @@ rdisc_config_changed (NMRDisc *rdisc, NMRDiscConfigMap changed, NMDevice *self)
 	}
 
 	if (changed & NM_RDISC_CONFIG_DHCP_LEVEL) {
-		dhcp6_cleanup (self, TRUE, TRUE);
+		dhcp6_cleanup (self, CLEANUP_TYPE_DECONFIGURE, TRUE);
 
 		priv->dhcp6_mode = rdisc->dhcp_level;
 		if (priv->dhcp6_mode != NM_RDISC_DHCP_LEVEL_NONE) {
@@ -7802,16 +7810,16 @@ nm_device_has_pending_action (NMDevice *self)
 /***********************************************************/
 
 static void
-_cleanup_ip_pre (NMDevice *self, gboolean deconfigure)
+_cleanup_ip_pre (NMDevice *self, CleanupType cleanup_type)
 {
 	NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self);
 
 	priv->ip4_state = priv->ip6_state = IP_NONE;
 	nm_device_queued_ip_config_change_clear (self);
 
-	dhcp4_cleanup (self, deconfigure, FALSE);
+	dhcp4_cleanup (self, cleanup_type, FALSE);
 	arp_cleanup (self);
-	dhcp6_cleanup (self, deconfigure, FALSE);
+	dhcp6_cleanup (self, cleanup_type, FALSE);
 	linklocal6_cleanup (self);
 	addrconf6_cleanup (self);
 	dnsmasq_cleanup (self);
@@ -7837,14 +7845,14 @@ _cancel_activation (NMDevice *self)
 }
 
 static void
-_cleanup_generic_pre (NMDevice *self, gboolean deconfigure)
+_cleanup_generic_pre (NMDevice *self, CleanupType cleanup_type)
 {
 	NMConnection *connection;
 
 	_cancel_activation (self);
 
 	connection = nm_device_get_connection (self);
-	if (   deconfigure
+	if (   cleanup_type == CLEANUP_TYPE_DECONFIGURE
 	    && connection
 	    && !nm_device_uses_assumed_connection (self)) {
 		nm_firewall_manager_remove_from_zone (nm_firewall_manager_get (),
@@ -7855,11 +7863,11 @@ _cleanup_generic_pre (NMDevice *self, gboolean deconfigure)
 	/* Clear any queued transitions */
 	nm_device_queued_state_clear (self);
 
-	_cleanup_ip_pre (self, deconfigure);
+	_cleanup_ip_pre (self, cleanup_type);
 }
 
 static void
-_cleanup_generic_post (NMDevice *self, gboolean deconfigure)
+_cleanup_generic_post (NMDevice *self, CleanupType cleanup_type)
 {
 	NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self);
 	NMDeviceStateReason ignored = NM_DEVICE_STATE_REASON_NONE;
@@ -7900,7 +7908,7 @@ _cleanup_generic_post (NMDevice *self, gboolean deconfigure)
 		g_object_notify (G_OBJECT (self), NM_DEVICE_IP4_ADDRESS);
 	}
 
-	if (deconfigure) {
+	if (cleanup_type == CLEANUP_TYPE_DECONFIGURE) {
 		/* Check if the device was deactivated, and if so, delete_link.
 		 * Don't call delete_link synchronously because we are currently
 		 * handling a state change -- which is not reentrant. */
@@ -7921,7 +7929,7 @@ _cleanup_generic_post (NMDevice *self, gboolean deconfigure)
  *
  */
 static void
-nm_device_cleanup (NMDevice *self, NMDeviceStateReason reason, gboolean deconfigure)
+nm_device_cleanup (NMDevice *self, NMDeviceStateReason reason, CleanupType cleanup_type)
 {
 	NMDevicePrivate *priv;
 	int ifindex;
@@ -7936,10 +7944,10 @@ nm_device_cleanup (NMDevice *self, NMDeviceStateReason reason, gboolean deconfig
 	/* Save whether or not we tried IPv6 for later */
 	priv = NM_DEVICE_GET_PRIVATE (self);
 
-	_cleanup_generic_pre (self, deconfigure);
+	_cleanup_generic_pre (self, cleanup_type);
 
 	/* Turn off kernel IPv6 */
-	if (deconfigure) {
+	if (cleanup_type == CLEANUP_TYPE_DECONFIGURE) {
 		set_disable_ipv6 (self, "1");
 		nm_device_ipv6_sysctl_set (self, "accept_ra", "0");
 		nm_device_ipv6_sysctl_set (self, "use_tempaddr", "0");
@@ -7965,7 +7973,7 @@ nm_device_cleanup (NMDevice *self, NMDeviceStateReason reason, gboolean deconfig
 	}
 
 	nm_device_update_metered (self);
-	_cleanup_generic_post (self, deconfigure);
+	_cleanup_generic_post (self, cleanup_type);
 }
 
 static char *
@@ -8294,11 +8302,11 @@ _set_state_full (NMDevice *self,
 		nm_device_set_firmware_missing (self, FALSE);
 		if (old_state > NM_DEVICE_STATE_UNMANAGED) {
 			if (reason == NM_DEVICE_STATE_REASON_REMOVED) {
-				nm_device_cleanup (self, reason, FALSE);
+				nm_device_cleanup (self, reason, CLEANUP_TYPE_REMOVED);
 			} else {
 				/* Clean up if the device is now unmanaged but was activated */
 				if (nm_device_get_act_request (self))
-					nm_device_cleanup (self, reason, TRUE);
+					nm_device_cleanup (self, reason, CLEANUP_TYPE_DECONFIGURE);
 				nm_device_take_down (self, TRUE);
 				set_nm_ipv6ll (self, FALSE);
 				restore_ip6_properties (self);
@@ -8327,7 +8335,7 @@ _set_state_full (NMDevice *self,
 			 * Note that we "deactivate" the device even when coming from
 			 * UNMANAGED, to ensure that it's in a clean state.
 			 */
-			nm_device_cleanup (self, reason, TRUE);
+			nm_device_cleanup (self, reason, CLEANUP_TYPE_DECONFIGURE);
 		}
 		break;
 	case NM_DEVICE_STATE_DISCONNECTED:
@@ -8337,7 +8345,7 @@ _set_state_full (NMDevice *self,
 			 */
 			set_nm_ipv6ll (self, TRUE);
 
-			nm_device_cleanup (self, reason, TRUE);
+			nm_device_cleanup (self, reason, CLEANUP_TYPE_DECONFIGURE);
 		} else if (old_state < NM_DEVICE_STATE_DISCONNECTED) {
 			if (reason != NM_DEVICE_STATE_REASON_CONNECTION_ASSUMED) {
 				/* Ensure IPv6 is set up as it may not have been done when
@@ -8352,7 +8360,7 @@ _set_state_full (NMDevice *self,
 			/* Clean up any half-done IP operations if the device's layer2
 			 * finds out it needs authentication during IP config.
 			 */
-			_cleanup_ip_pre (self, TRUE);
+			_cleanup_ip_pre (self, CLEANUP_TYPE_DECONFIGURE);
 		}
 		break;
 	default:
@@ -8991,7 +8999,7 @@ dispose (GObject *object)
 
 	dispatcher_cleanup (self);
 
-	_cleanup_generic_pre (self, FALSE);
+	_cleanup_generic_pre (self, CLEANUP_TYPE_KEEP);
 
 	g_warn_if_fail (priv->slaves == NULL);
 	g_assert (priv->master_ready_id == 0);
@@ -8999,7 +9007,7 @@ dispose (GObject *object)
 	/* Let the kernel manage IPv6LL again */
 	set_nm_ipv6ll (self, FALSE);
 
-	_cleanup_generic_post (self, FALSE);
+	_cleanup_generic_post (self, CLEANUP_TYPE_KEEP);
 
 	g_hash_table_remove_all (priv->ip6_saved_properties);
 
diff --git a/src/dhcp-manager/nm-dhcp-client.c b/src/dhcp-manager/nm-dhcp-client.c
index aa5df9908a..67be01cf4f 100644
--- a/src/dhcp-manager/nm-dhcp-client.c
+++ b/src/dhcp-manager/nm-dhcp-client.c
@@ -590,9 +590,10 @@ nm_dhcp_client_stop_existing (const char *pid_file, const char *binary_name)
 	if ((errno == 0) && (tmp > 1)) {
 		guint64 start_time;
 		const char *exe;
+		pid_t ppid;
 
 		/* Ensure the process is a DHCP client */
-		start_time = nm_utils_get_start_time_for_pid (tmp, NULL);
+		start_time = nm_utils_get_start_time_for_pid (tmp, NULL, &ppid);
 		proc_path = g_strdup_printf ("/proc/%ld/cmdline", tmp);
 		if (   start_time
 		    && g_file_get_contents (proc_path, &proc_contents, NULL, NULL)) {
@@ -602,9 +603,15 @@ nm_dhcp_client_stop_existing (const char *pid_file, const char *binary_name)
 			else
 				exe = proc_contents;
 
-			if (!strcmp (exe, binary_name))
-				nm_utils_kill_process_sync (tmp, start_time, SIGTERM, LOGD_DHCP,
-				                            "dhcp-client", 1000 / 2, 1000 / 20, 2000);
+			if (!strcmp (exe, binary_name)) {
+				if (ppid == getpid ()) {
+					/* the process is our own child. */
+					nm_utils_kill_child_sync (tmp, SIGTERM, LOGD_DHCP, "dhcp-client", NULL, 1000 / 2, 1000 / 20);
+				} else {
+					nm_utils_kill_process_sync (tmp, start_time, SIGTERM, LOGD_DHCP,
+					                            "dhcp-client", 1000 / 2, 1000 / 20, 2000);
+				}
+			}
 		}
 	}
 
diff --git a/src/nm-auth-subject.c b/src/nm-auth-subject.c
index 3fc0e64a1d..c67424e788 100644
--- a/src/nm-auth-subject.c
+++ b/src/nm-auth-subject.c
@@ -360,7 +360,7 @@ constructed (GObject *object)
 		if (!priv->unix_process.dbus_sender || !*priv->unix_process.dbus_sender)
 			break;
 
-		priv->unix_process.start_time = nm_utils_get_start_time_for_pid (priv->unix_process.pid, NULL);
+		priv->unix_process.start_time = nm_utils_get_start_time_for_pid (priv->unix_process.pid, NULL, NULL);
 
 		if (!priv->unix_process.start_time) {
 			/* could not detect the process start time. The subject is invalid, but don't