From ca4361bd5388a242f232dfa2caea60e5770fef30 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sun, 3 May 2015 14:08:31 +0200 Subject: [PATCH 1/4] utils: preserve errno in nm_utils_kill_child_sync() --- src/NetworkManagerUtils.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/NetworkManagerUtils.c b/src/NetworkManagerUtils.c index ae2e83e482..82c872175e 100644 --- a/src/NetworkManagerUtils.c +++ b/src/NetworkManagerUtils.c @@ -624,13 +624,15 @@ _sleep_duration_convert_ms_to_us (guint32 sleep_duration_msec) * sent unless the child already exited. If the child does not exit within @wait_before_kill_msec milliseconds, * the function will send %SIGKILL and waits for the child indefinitly. If @wait_before_kill_msec is zero, no * %SIGKILL signal will be sent. + * + * In case of error, errno is preserved to contain the last reason of failure. **/ gboolean nm_utils_kill_child_sync (pid_t pid, int sig, guint64 log_domain, const char *log_name, int *child_status, guint32 wait_before_kill_msec, guint32 sleep_duration_msec) { - int status = 0, errsv; + int status = 0, errsv = 0; pid_t ret; gboolean success = FALSE; gboolean was_waiting = FALSE, send_kill = FALSE; @@ -776,6 +778,7 @@ nm_utils_kill_child_sync (pid_t pid, int sig, guint64 log_domain, const char *lo out: if (child_status) *child_status = success ? status : -1; + errno = success ? 0 : errsv; return success; } From 6b646a1e375261d88a6caebc0b81408dd0eac30c Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sun, 3 May 2015 13:51:02 +0200 Subject: [PATCH 2/4] dns-manager: use nm_utils_kill_child_sync() to wait for netconfig to exit --- src/dns-manager/nm-dns-manager.c | 29 ++++++++++------------------- 1 file changed, 10 insertions(+), 19 deletions(-) diff --git a/src/dns-manager/nm-dns-manager.c b/src/dns-manager/nm-dns-manager.c index a912b17123..be7180e407 100644 --- a/src/dns-manager/nm-dns-manager.c +++ b/src/dns-manager/nm-dns-manager.c @@ -254,7 +254,7 @@ run_netconfig (GError **error, gint *stdin_fd) nm_log_dbg (LOGD_DNS, "spawning '%s'", tmp); g_free (tmp); - if (!g_spawn_async_with_pipes (NULL, argv, NULL, 0, NULL, + if (!g_spawn_async_with_pipes (NULL, argv, NULL, G_SPAWN_DO_NOT_REAP_CHILD, NULL, NULL, &pid, stdin_fd, NULL, NULL, error)) return -1; @@ -283,10 +283,10 @@ dispatch_netconfig (char **searches, char *str; GPid pid; gint fd; - int ret = 1; + int status; pid = run_netconfig (error, &fd); - if (pid < 0) + if (pid <= 0) return FALSE; /* NM is writing already-merged DNS information to netconfig, so it @@ -319,24 +319,15 @@ dispatch_netconfig (char **searches, close (fd); /* Wait until the process exits */ + if (!nm_utils_kill_child_sync (pid, 0, LOGD_DNS, "netconfig", &status, 1000, 0)) { + int errsv = errno; - again: - - if (waitpid (pid, NULL, 0) < 0) { - if (errno == EINTR) - goto again; - else if (errno == ECHILD) { - /* child already exited */ - ret = pid; - } else { - g_set_error (error, NM_MANAGER_ERROR, NM_MANAGER_ERROR_FAILED, - "Error waiting for netconfig to exit: %s", - strerror (errno)); - ret = 0; - } + g_set_error (error, NM_MANAGER_ERROR, NM_MANAGER_ERROR_FAILED, + "Error waiting for netconfig to exit: %s", + strerror (errsv)); + return FALSE; } - - return ret > 0; + return TRUE; } static gboolean From 5f0c23f1067a7f49d56a27592eb315aebb0b94cf Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sun, 3 May 2015 14:24:57 +0200 Subject: [PATCH 3/4] dns-manager: fail dns config if netconfig exits with non-zero status If netconfig does not exit with zero status signal, assume configuration failed and signal an error. --- src/dns-manager/nm-dns-manager.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/dns-manager/nm-dns-manager.c b/src/dns-manager/nm-dns-manager.c index be7180e407..160e6fb352 100644 --- a/src/dns-manager/nm-dns-manager.c +++ b/src/dns-manager/nm-dns-manager.c @@ -327,6 +327,13 @@ dispatch_netconfig (char **searches, strerror (errsv)); return FALSE; } + if (!WIFEXITED (status) || WEXITSTATUS (status) != EXIT_SUCCESS) { + g_set_error (error, NM_MANAGER_ERROR, NM_MANAGER_ERROR_FAILED, + "Error calling netconfig: %s %d", + WIFEXITED (status) ? "exited with status" : (WIFSIGNALED (status) ? "exited with signal" : "exited with unknown reason"), + WIFEXITED (status) ? WEXITSTATUS (status) : (WIFSIGNALED (status) ? WTERMSIG (status) : status)); + return FALSE; + } return TRUE; } From 22bfe2feb28f748235577da46f7f8a365f4ad607 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sun, 3 May 2015 13:57:16 +0200 Subject: [PATCH 4/4] dispatcher: repeat waitpid() call on EINTR Also, no use of first trying to kill() with signal zero. Just send SIGKILL right away. --- callouts/nm-dispatcher.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/callouts/nm-dispatcher.c b/callouts/nm-dispatcher.c index 6fb40cf9f3..77f2153d62 100644 --- a/callouts/nm-dispatcher.c +++ b/callouts/nm-dispatcher.c @@ -294,9 +294,12 @@ script_timeout_cb (gpointer user_data) g_warning ("Script '%s' took too long; killing it.", script->script); - if (kill (script->pid, 0) == 0) - kill (script->pid, SIGKILL); - (void) waitpid (script->pid, NULL, 0); + kill (script->pid, SIGKILL); +again: + if (waitpid (script->pid, NULL, 0) == -1) { + if (errno == EINTR) + goto again; + } script->error = g_strdup_printf ("Script '%s' timed out.", script->script); script->result = DISPATCH_RESULT_TIMEOUT;