[polkit] Fix a PolkitAgentSession race condition
Miloslav Trmac
mitr at fedoraproject.org
Tue Feb 11 15:09:57 UTC 2014
commit f3502e13349d97b0a5648cf92cf68e98dfc091c3
Author: Miloslav Trmač <mitr at redhat.com>
Date: Tue Feb 11 11:09:04 2014 +0100
Fix a PolkitAgentSession race condition
polkit-0.112-PolkitAgentSession-race.patch | 120 ++++++++++++++++++++++++++++
polkit.spec | 9 ++-
2 files changed, 128 insertions(+), 1 deletions(-)
---
diff --git a/polkit-0.112-PolkitAgentSession-race.patch b/polkit-0.112-PolkitAgentSession-race.patch
new file mode 100644
index 0000000..d6e16b2
--- /dev/null
+++ b/polkit-0.112-PolkitAgentSession-race.patch
@@ -0,0 +1,120 @@
+From 7650ad1e08ab13bdb461783c4995d186d9392840 Mon Sep 17 00:00:00 2001
+From: Rui Matos <tiagomatos at gmail.com>
+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.
+
+If we add G_IO_HUP and G_IO_ERR to the conditions we look for in the
+io watch and the child terminates we still run the io watch handler
+which will complete the session.
+
+This means that the child watch is in fact needless and we can remove
+it.
+
+https://bugs.freedesktop.org/show_bug.cgi?id=60847
+---
+ src/polkitagent/polkitagentsession.c | 47 +++++++++---------------------------
+ 1 file changed, 11 insertions(+), 36 deletions(-)
+
+diff --git a/src/polkitagent/polkitagentsession.c b/src/polkitagent/polkitagentsession.c
+index 1c7a2dc..f014773 100644
+--- a/src/polkitagent/polkitagentsession.c
++++ b/src/polkitagent/polkitagentsession.c
+@@ -92,7 +92,6 @@ struct _PolkitAgentSession
+ int child_stdout;
+ GPid child_pid;
+
+- GSource *child_watch_source;
+ GSource *child_stdout_watch_source;
+ GIOChannel *child_stdout_channel;
+
+@@ -377,13 +376,6 @@ kill_helper (PolkitAgentSession *session)
+ session->child_pid = 0;
+ }
+
+- if (session->child_watch_source != NULL)
+- {
+- g_source_destroy (session->child_watch_source);
+- g_source_unref (session->child_watch_source);
+- session->child_watch_source = NULL;
+- }
+-
+ if (session->child_stdout_watch_source != NULL)
+ {
+ g_source_destroy (session->child_stdout_watch_source);
+@@ -429,26 +421,6 @@ complete_session (PolkitAgentSession *session,
+ }
+ }
+
+-static void
+-child_watch_func (GPid pid,
+- gint status,
+- gpointer user_data)
+-{
+- PolkitAgentSession *session = POLKIT_AGENT_SESSION (user_data);
+-
+- if (G_UNLIKELY (_show_debug ()))
+- {
+- g_print ("PolkitAgentSession: in child_watch_func for pid %d (WIFEXITED=%d WEXITSTATUS=%d)\n",
+- (gint) pid,
+- WIFEXITED(status),
+- 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
+ io_watch_have_data (GIOChannel *channel,
+ GIOCondition condition,
+@@ -475,10 +447,13 @@ io_watch_have_data (GIOChannel *channel,
+ 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 +515,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;
+ }
+@@ -650,12 +628,9 @@ polkit_agent_session_initiate (PolkitAgentSession *session)
+ if (G_UNLIKELY (_show_debug ()))
+ g_print ("PolkitAgentSession: spawned helper with pid %d\n", (gint) session->child_pid);
+
+- session->child_watch_source = g_child_watch_source_new (session->child_pid);
+- g_source_set_callback (session->child_watch_source, (GSourceFunc) child_watch_func, session, NULL);
+- 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
+
diff --git a/polkit.spec b/polkit.spec
index 6efaeed..a1c9570 100644
--- a/polkit.spec
+++ b/polkit.spec
@@ -6,13 +6,15 @@
Summary: An authorization framework
Name: polkit
Version: 0.112
-Release: 2%{?dist}
+Release: 3%{?dist}
License: LGPLv2+
URL: http://www.freedesktop.org/wiki/Software/polkit
Source0: http://www.freedesktop.org/software/polkit/releases/%{name}-%{version}.tar.gz
Source1: http://www.freedesktop.org/software/polkit/releases/%{name}-%{version}.tar.gz.sign
# https://bugs.freedesktop.org/show_bug.cgi?id=71894
Patch0: polkit-0.112-XDG_RUNTIME_DIR.patch
+# https://bugs.freedesktop.org/show_bug.cgi?id=60847
+Patch1: polkit-0.112-PolkitAgentSession-race.patch
Group: System Environment/Libraries
BuildRequires: glib2-devel >= 2.30.0
BuildRequires: expat-devel
@@ -80,6 +82,7 @@ Development documentation for polkit.
%prep
%setup -q
%patch0 -p1 -b .XDG_RUNTIME_DIR
+%patch1 -p1 -b .PolkitAgentSession-race
%build
%if 0%{?enable_autoreconf}
@@ -166,6 +169,10 @@ exit 0
%{_datadir}/gtk-doc
%changelog
+* Mon Feb 10 2014 Miloslav Trmač <mitr at redhat.com> - 0.112-3
+- Fix a PolkitAgentSession race condition
+ Resolves: #1063193
+
* Sat Dec 7 2013 Miloslav Trmač <mitr at redhat.com> - 0.112-2
- Workaround pam_systemd setting broken XDG_RUNTIME_DIR
Resolves: #1033774
More information about the scm-commits
mailing list