Fix up debug and timeouts in agent helper

Signed-off-by: David Zeuthen <davidz@redhat.com>

3 files changed, 34 insertions, 26 deletions

diff --git a/src/polkitagent/polkitagenthelper-pam.c b/src/polkitagent/polkitagenthelper-pam.c
index 91b2b39..61fed4d 100644
--- a/src/polkitagent/polkitagenthelper-pam.c
+++ b/src/polkitagent/polkitagenthelper-pam.c
@@ -64,7 +64,6 @@ int
 main (int argc, char *argv[])
 {
   int rc;
-  int err_ret;
   const char *user_to_auth;
   const char *cookie;
   struct pam_conv pam_conversation;
@@ -73,7 +72,6 @@ main (int argc, char *argv[])
 
   rc = 0;
   pam_h = NULL;
-  err_ret = 1;
 
   /* clear the entire environment to avoid attacks using with libraries honoring environment variables */
   if (_polkit_clearenv () != 0)
@@ -85,11 +83,14 @@ main (int argc, char *argv[])
   /* check that we are setuid root */
   if (geteuid () != 0)
     {
+      gchar *s;
+
       fprintf (stderr, "polkit-agent-helper-1: needs to be setuid root\n");
-      /* Special-case a very common error triggered in jhbuild setups - see
-       * polkitagentsession.c:child_watch_func() for details
-       */
-      err_ret = 2;
+
+      /* Special-case a very common error triggered in jhbuild setups */
+      s = g_strdup_printf ("Incorrect permissions on %s (needs to be setuid root)", argv[0]);
+      send_to_helper ("PAM_ERROR_MSG ", s);
+      g_free (s);
       goto error;
     }
 
@@ -220,7 +221,7 @@ error:
 
   fprintf (stdout, "FAILURE\n");
   flush_and_wait();
-  return err_ret;
+  return 1;
 }
 
 static int
diff --git a/src/polkitagent/polkitagenthelperprivate.c b/src/polkitagent/polkitagenthelperprivate.c
index 8f0c878..4417e70 100644
--- a/src/polkitagent/polkitagenthelperprivate.c
+++ b/src/polkitagent/polkitagenthelperprivate.c
@@ -98,14 +98,12 @@ send_dbus_message (const char *cookie, const char *user)
   return ret;
 }
 
-/* fflush(3) stdin and stdout and wait a little bit.
- * This replaces the three-line commands at the bottom of
- * polkit-agent-helper-1's main() function.
- */
 void
 flush_and_wait ()
 {
   fflush (stdout);
   fflush (stderr);
-  usleep (10 * 1000); /* since fflush(3) seems buggy */
+  fdatasync (fileno(stdout));
+  fdatasync (fileno(stderr));
+  usleep (100 * 1000);
 }
diff --git a/src/polkitagent/polkitagentsession.c b/src/polkitagent/polkitagentsession.c
index 0308b65..9861b5d 100644
--- a/src/polkitagent/polkitagentsession.c
+++ b/src/polkitagent/polkitagentsession.c
@@ -97,6 +97,7 @@ struct _PolkitAgentSession
 
   gboolean success;
   gboolean helper_is_running;
+  gboolean have_emitted_completed;
 };
 
 struct _PolkitAgentSessionClass
@@ -404,9 +405,13 @@ complete_session (PolkitAgentSession *session,
                   gboolean            result)
 {
   kill_helper (session);
-  if (G_UNLIKELY (_show_debug ()))
-    g_print ("PolkitAgentSession: emitting ::completed(%s)\n", result ? "TRUE" : "FALSE");
-  g_signal_emit_by_name (session, "completed", result);
+  if (!session->have_emitted_completed)
+    {
+      if (G_UNLIKELY (_show_debug ()))
+        g_print ("PolkitAgentSession: emitting ::completed(%s)\n", result ? "TRUE" : "FALSE");
+      g_signal_emit_by_name (session, "completed", result);
+      session->have_emitted_completed = TRUE;
+    }
 }
 
 static void
@@ -416,19 +421,17 @@ child_watch_func (GPid     pid,
 {
   PolkitAgentSession *session = POLKIT_AGENT_SESSION (user_data);
 
-  /* kill all the watches we have set up, except for the child since it has exited already */
-  session->child_pid = 0;
-  kill_helper (session);
-
-  /* Special-case a very common error triggered in jhbuild setups */
-  if (WIFEXITED (status) && WEXITSTATUS (status) == 2)
+  if (G_UNLIKELY (_show_debug ()))
     {
-      const gchar *s = "Incorrect permissions on " PACKAGE_LIBEXEC_DIR "/polkit-agent-helper-1";
-      if (G_UNLIKELY (_show_debug ()))
-        g_print ("PolkitAgentSession: emitting ::show-error('%s')\n", s);
-      g_signal_emit_by_name (session, "show-error", s);
-      complete_session (session, FALSE);
+      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
@@ -469,6 +472,9 @@ io_watch_have_data (GIOChannel    *channel,
   if (strlen (line) > 0 && line[strlen (line) - 1] == '\n')
     line[strlen (line) - 1] = '\0';
 
+  if (G_UNLIKELY (_show_debug ()))
+    g_print ("PolkitAgentSession: read `%s' from helper\n", line);
+
   if (g_str_has_prefix (line, "PAM_PROMPT_ECHO_OFF "))
     {
       const gchar *s = line + sizeof "PAM_PROMPT_ECHO_OFF " - 1;
@@ -625,6 +631,9 @@ polkit_agent_session_initiate (PolkitAgentSession *session)
       goto error;
     }
 
+  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 ());