From 928d235b72678c24d5aff5b0b4738e0a4def4ac4 Mon Sep 17 00:00:00 2001 From: Rui Matos Date: Thu, 6 Feb 2014 18:41:18 +0100 Subject: [PATCH] PolkitAgentSession: fix race between child and io watches The helper flushes and fdatasyncs stdout and stderr before terminating but this doesn't guarantee that our io watch is called before our child watch. This means that we can end up with a successful return from the helper which we still report as a failure. To fix this, instead of reporting a failure from the child watch handler, we'll just unset child_pid. In addition, we add G_IO_HUP and G_IO_ERR to the conditions we look for in the io watch so that if the child terminates abnormally we still run the io watch handler which will complete the session reporting a failure. --- src/polkitagent/polkitagentsession.c | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/src/polkitagent/polkitagentsession.c b/src/polkitagent/polkitagentsession.c index 1c7a2dc..0d35e95 100644 --- a/src/polkitagent/polkitagentsession.c +++ b/src/polkitagent/polkitagentsession.c @@ -444,9 +444,7 @@ child_watch_func (GPid pid, WEXITSTATUS(status)); } - /* kill all the watches we have set up, except for the child since it has exited already */ session->child_pid = 0; - complete_session (session, FALSE); } static gboolean @@ -470,15 +468,24 @@ io_watch_have_data (GIOChannel *channel, goto out; } + if (!(condition & G_IO_IN)) + { + complete_session (session, FALSE); + goto out; + } + g_io_channel_read_line (channel, &line, NULL, NULL, &error); - if (error != NULL) + if (error != NULL || line == NULL) { - g_warning ("Error reading line from helper: %s", error->message); - g_error_free (error); + /* In case we get just G_IO_HUP, line is NULL but error is + unset.*/ + g_warning ("Error reading line from helper: %s", + error ? error->message : "nothing to read"); + g_clear_error (&error); complete_session (session, FALSE); goto out; @@ -540,6 +547,9 @@ io_watch_have_data (GIOChannel *channel, g_free (line); g_free (unescaped); + if (condition & (G_IO_ERR | G_IO_HUP)) + complete_session (session, FALSE); + /* keep the IOChannel around */ return TRUE; } @@ -655,7 +665,8 @@ polkit_agent_session_initiate (PolkitAgentSession *session) g_source_attach (session->child_watch_source, g_main_context_get_thread_default ()); session->child_stdout_channel = g_io_channel_unix_new (session->child_stdout); - session->child_stdout_watch_source = g_io_create_watch (session->child_stdout_channel, G_IO_IN); + session->child_stdout_watch_source = g_io_create_watch (session->child_stdout_channel, + G_IO_IN | G_IO_ERR | G_IO_HUP); g_source_set_callback (session->child_stdout_watch_source, (GSourceFunc) io_watch_have_data, session, NULL); g_source_attach (session->child_stdout_watch_source, g_main_context_get_thread_default ()); -- 1.8.3.1