From 2bd5cf51b809f9593930d951af6cc7487c4330f8 Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Fri, 8 Aug 2014 12:00:30 -0500 Subject: [PATCH] dhcp: fix killing wrong process ID on dhclient release The prototype of dhclient_start() changed in 30cdd1248 but that commit didn't update stop() correctly. Clearly treating a gboolean as pid_t isn't going to work. Second, we don't want to watch the child process on DHCP release because we're just going to kill it shortly after. --- src/dhcp-manager/nm-dhcp-client.c | 5 +++-- src/dhcp-manager/nm-dhcp-dhclient.c | 18 +++++++++++------- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/src/dhcp-manager/nm-dhcp-client.c b/src/dhcp-manager/nm-dhcp-client.c index 29584b0662..dc23490a79 100644 --- a/src/dhcp-manager/nm-dhcp-client.c +++ b/src/dhcp-manager/nm-dhcp-client.c @@ -214,6 +214,8 @@ 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); + nm_utils_kill_child_sync (pid, SIGTERM, LOGD_DHCP, name ? name : "dhcp-client", NULL, 1000 / 2, 1000 / 20); g_free (name); @@ -232,9 +234,8 @@ stop (NMDhcpClient *self, gboolean release, const GByteArray *duid) /* Clean up the watch handler since we're explicitly killing the daemon */ watch_cleanup (self); nm_dhcp_client_stop_pid (priv->pid, priv->iface); - priv->pid = -1; } - + priv->pid = -1; priv->info_only = FALSE; } diff --git a/src/dhcp-manager/nm-dhcp-dhclient.c b/src/dhcp-manager/nm-dhcp-dhclient.c index 41f597f668..f2fe29057d 100644 --- a/src/dhcp-manager/nm-dhcp-dhclient.c +++ b/src/dhcp-manager/nm-dhcp-dhclient.c @@ -328,7 +328,8 @@ static gboolean dhclient_start (NMDhcpClient *client, const char *mode_opt, const GByteArray *duid, - gboolean release) + gboolean release, + pid_t *out_pid) { NMDhcpDhclientPrivate *priv = NM_DHCP_DHCLIENT_GET_PRIVATE (client); GPtrArray *argv = NULL; @@ -461,7 +462,8 @@ dhclient_start (NMDhcpClient *client, &dhclient_child_setup, NULL, &pid, &error)) { g_assert (pid > 0); nm_log_info (log_domain, "dhclient started with pid %d", pid); - nm_dhcp_client_watch_child (client, pid); + if (release == FALSE) + nm_dhcp_client_watch_child (client, pid); priv->pid_file = pid_file; } else { nm_log_warn (log_domain, "dhclient failed to start: '%s'", error->message); @@ -469,6 +471,9 @@ dhclient_start (NMDhcpClient *client, g_free (pid_file); } + if (out_pid) + *out_pid = pid; + g_ptr_array_free (argv, TRUE); g_free (system_bus_address_env); return pid > 0 ? TRUE : FALSE; @@ -492,7 +497,7 @@ ip4_start (NMDhcpClient *client, return FALSE; } - return dhclient_start (client, NULL, NULL, FALSE); + return dhclient_start (client, NULL, NULL, FALSE, NULL); } static gboolean @@ -515,7 +520,7 @@ ip6_start (NMDhcpClient *client, return FALSE; } - return dhclient_start (client, info_only ? "-S" : "-N", duid, FALSE); + return dhclient_start (client, info_only ? "-S" : "-N", duid, FALSE, NULL); } static void @@ -537,10 +542,9 @@ stop (NMDhcpClient *client, gboolean release, const GByteArray *duid) } if (release) { - pid_t rpid; + pid_t rpid = -1; - rpid = dhclient_start (client, NULL, duid, TRUE); - if (rpid > 0) { + if (dhclient_start (client, NULL, duid, TRUE, &rpid)) { /* Wait a few seconds for the release to happen */ nm_dhcp_client_stop_pid (rpid, nm_dhcp_client_get_iface (client)); }