PolicyKit: Branch 'master' - 14 commits

Miloslav Trmac mitr at kemper.freedesktop.org
Thu Jul 2 10:44:43 PDT 2015


 NEWS                                                  |   93 ++++++++++++++++-
 README                                                |   18 +++
 src/polkitagent/polkitagentsession.c                  |    3 
 src/polkitbackend/polkitbackendactionpool.c           |    8 -
 src/polkitbackend/polkitbackendauthority.c            |    2 
 src/polkitbackend/polkitbackendinteractiveauthority.c |    5 
 src/polkitbackend/polkitbackendjsauthority.c          |   97 ++++++++++++++----
 7 files changed, 195 insertions(+), 31 deletions(-)

New commits:
commit 23519924f24fb80a5f33bb3a82058a6c025ddfa9
Author: Miloslav Trmač <mitr at redhat.com>
Date:   Fri Jun 19 23:07:05 2015 +0200

    Update NEWS for release.

diff --git a/NEWS b/NEWS
index e785172..8d7ce12 100644
--- a/NEWS
+++ b/NEWS
@@ -11,7 +11,27 @@ some security review. Use at your own risk.
 This is polkit 0.113.
 
 Highlights:
- TODO
+ Fixes CVE-2015-4625, a local privilege escalation due to predictable
+ authentication session cookie values. Thanks to Tavis Ormandy, Google Project
+ Zero for reporting this issue. For the future, authentication agents are
+ encouraged to use PolkitAgentSession instead of using the D-Bus agent response
+ API directly.
+
+ Fixes CVE-2015-3256, various memory corruption vulnerabilities in use of the
+ JavaScript interpreter, possibly leading to local privilege escalation.
+
+ Fixes CVE-2015-3255, a memory corruption vulnerability in handling duplicate
+ action IDs, possibly leading to local privilege escalation. Thanks to
+ Laurent Bigonville for reporting this issue.
+
+ Fixes CVE-2015-3218, which allowed any local user to crash polkitd. Thanks to
+ Tavis Ormandy, Google Project Zero, for reporting this issue.
+
+ On systemd-213 and later, the “active” state is shared across all sessions of
+ an user, instead of being tracked separately.
+
+ (pkexec), when not given a program to execute, runs the users’ shell by
+ default.
 
 Build requirements
 
@@ -23,12 +43,79 @@ Build requirements
 
 Changes since polkit 0.112:
 
- TODO
+Colin Walters (17):
+      PolkitSystemBusName: Add public API to retrieve Unix user
+      examples/cancel: Fix to securely lookup subject
+      sessionmonitor-systemd: Deduplicate code paths
+      PolkitSystemBusName: Retrieve both pid and uid
+      Port internals non-deprecated PolkitProcess API where possible
+      Use G_GNUC_BEGIN_IGNORE_DEPRECATIONS to avoid warning spam
+      pkexec: Work around systemd injecting broken XDG_RUNTIME_DIR
+      pkexec: Support just plain "pkexec" to run shell
+      .dir-locals: Style for Emacs - we don't use tabs
+      authority: Avoid cookie wrapping by using u64 counter
+      CVE-2015-3218: backend: Handle invalid object paths in RegisterAuthenticationAgent
+      build: Start using git.mk
+      Revert "authority: Avoid cookie wrapping by using u64 counter"
+      authority: Add a helper method for checking whether an identity is root
+      CVE-2015-4625: Use unpredictable cookie values, keep them secret
+      CVE-2015-4625: Bind use of cookies to specific uids
+      README: Note to send security reports via DBus's mechanism
+
+Kay Sievers (1):
+      sessionmonitor-systemd: prepare for D-Bus "user bus" model
+
+Lukasz Skalski (1):
+      polkitd: Fix problem with removing non-existent source
+
+Max A. Dednev (1):
+      authority: Fix memory leak in EnumerateActions call results handler
+
+Miloslav Trmač (24):
+      Post-release version bump to 0.113
+      Don't discard error data returned by polkit_system_bus_name_get_user_sync
+      Fix a memory leak
+      Refuse duplicate --user arguments to pkexec
+      Fix a possible NULL dereference.
+      Remove a redundant assignment.
+      Simplify forced error domain registration
+      Fix a typo, s/Evaluting/Evaluating/g
+      s/INCLUDES/AM_CPPFLAGS/g
+      Fix duplicate GError use when "uid" is missing
+      Fix a crash when two authentication requests are in flight.
+      docs: Update for changes to uid binding/AuthenticationAgentResponse2
+      Don't pass an uninitialized JS parameter
+      Don't add extra NULL group to subject.groups
+      Don't store unrooted jsvals on heap
+      Fix a per-authorization memory leak
+      Fix a memory leak when registering an authentication agent
+      Wrap all JS usage within “requests”
+      Register heap-based JSObject pointers to GC
+      Prevent builds against SpiderMonkey with exact stack rooting
+      Clear the JS operation callback before invoking JS in the callback
+      Fix spurious timeout exceptions on GC
+      Fix GHashTable usage.
+      Fix use-after-free in polkitagentsession.c
+
+Philip Withnall (1):
+      sessionmonitor-systemd: Use sd_uid_get_state() to check session activity
+
+Rui Matos (1):
+      PolkitAgentSession: fix race between child and io watches
+
+Simon McVittie (1):
+      Use libsystemd instead of older libsystemd-login if possible
+
+Ting-Wei Lan (1):
+      build: Fix several issues on FreeBSD
+
+Xabier Rodriguez Calvar (1):
+      Fixed compilation problem in the backend
 
 Thanks to our contributors.
 
 Colin Walters and Miloslav Trmač,
-$DATE
+July 2, 2015
 
 --------------
 polkit 0.112
commit ccec766c509d16dab417582e94f43d906cefd4ae
Author: Colin Walters <walters at verbum.org>
Date:   Thu Jun 4 08:41:36 2015 -0400

    README: Note to send security reports via DBus's mechanism
    
    This avoids duplicating effort.

diff --git a/README b/README
index b075162..0723002 100644
--- a/README
+++ b/README
@@ -22,6 +22,22 @@ To verify the authenticity of the compressed tarball, use this command
 BUGS and DEVELOPMENT
 ====================
 
-Please report bugs via the freedesktop.org bugzilla at
+Please report non-security bugs via the freedesktop.org bugzilla at
 
  https://bugs.freedesktop.org/enter_bug.cgi?product=PolicyKit
+
+SECURITY ISSUES
+===============
+
+polkit uses the same mechanism for reporting security issues as dbus,
+the most recent copy of instructions can be found in the DBus git
+repository:
+
+http://cgit.freedesktop.org/dbus/dbus/tree/HACKING
+
+A copy of the instructions as of 2015-06-04:
+
+If you find a security vulnerability that is not known to the public,
+please report it privately to dbus-security at lists.freedesktop.org
+or by reporting a freedesktop.org bug that is marked as
+restricted to the "D-BUS security group".
commit efb6cd56a423ba15bb1f44ee3c4987aad5a5fd45
Author: Miloslav Trmač <mitr at redhat.com>
Date:   Tue Apr 14 22:27:41 2015 +0200

    Fix use-after-free in polkitagentsession.c
    
    PolkitAgentTextListener's "completed" handler drops the last reference
    to the session; in fact this is explicitly recommended in the signal's
    documentation.  So we must not access any members of session after
    emitting the signal.
    
    Found while dealing with
    https://bugs.freedesktop.org/show_bug.cgi?id=69501

diff --git a/src/polkitagent/polkitagentsession.c b/src/polkitagent/polkitagentsession.c
index 8b93ad0..895d75e 100644
--- a/src/polkitagent/polkitagentsession.c
+++ b/src/polkitagent/polkitagentsession.c
@@ -412,8 +412,9 @@ complete_session (PolkitAgentSession *session,
     {
       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;
+      /* Note that the signal handler may drop the last reference to session. */
+      g_signal_emit_by_name (session, "completed", result);
     }
 }
 
commit 9f5e0c731784003bd4d6fc75ab739ff8b2ea269f
Author: Miloslav Trmač <mitr at redhat.com>
Date:   Wed Apr 1 05:22:37 2015 +0200

    CVE-2015-3255 Fix GHashTable usage.
    
    Don't assume that the hash table with free both the key and the value
    at the same time, supply proper deallocation functions for the key
    and value separately.
    
    Then drop ParsedAction::action_id which is no longer used for anything.
    
    https://bugs.freedesktop.org/show_bug.cgi?id=69501
    and
    https://bugs.freedesktop.org/show_bug.cgi?id=83590
    
    CVE: CVE-2015-3255

diff --git a/src/polkitbackend/polkitbackendactionpool.c b/src/polkitbackend/polkitbackendactionpool.c
index bc14381..3894fe9 100644
--- a/src/polkitbackend/polkitbackendactionpool.c
+++ b/src/polkitbackend/polkitbackendactionpool.c
@@ -40,7 +40,6 @@
 
 typedef struct
 {
-  gchar *action_id;
   gchar *vendor_name;
   gchar *vendor_url;
   gchar *icon_name;
@@ -62,7 +61,6 @@ typedef struct
 static void
 parsed_action_free (ParsedAction *action)
 {
-  g_free (action->action_id);
   g_free (action->vendor_name);
   g_free (action->vendor_url);
   g_free (action->icon_name);
@@ -134,7 +132,7 @@ polkit_backend_action_pool_init (PolkitBackendActionPool *pool)
 
   priv->parsed_actions = g_hash_table_new_full (g_str_hash,
                                                 g_str_equal,
-                                                NULL,
+                                                g_free,
                                                 (GDestroyNotify) parsed_action_free);
 
   priv->parsed_files = g_hash_table_new_full (g_str_hash,
@@ -988,7 +986,6 @@ _end (void *data, const char *el)
           icon_name = pd->global_icon_name;
 
         action = g_new0 (ParsedAction, 1);
-        action->action_id = g_strdup (pd->action_id);
         action->vendor_name = g_strdup (vendor);
         action->vendor_url = g_strdup (vendor_url);
         action->icon_name = g_strdup (icon_name);
@@ -1003,7 +1000,8 @@ _end (void *data, const char *el)
         action->implicit_authorization_inactive = pd->implicit_authorization_inactive;
         action->implicit_authorization_active = pd->implicit_authorization_active;
 
-        g_hash_table_insert (priv->parsed_actions, action->action_id, action);
+        g_hash_table_insert (priv->parsed_actions, g_strdup (pd->action_id),
+                             action);
 
         /* we steal these hash tables */
         pd->annotations = NULL;
commit d7da6a23766e9c95fa333a0a9c742f7397c0ad22
Author: Miloslav Trmač <mitr at redhat.com>
Date:   Tue Jul 1 20:00:48 2014 +0200

    Fix spurious timeout exceptions on GC
    
    The JS “Operation callback” can be called by the runtime for other
    reasons, not only when we trigger it by a timeout—notably as part of GC.
    So, make sure to only raise an exception if there actually was a
    timeout.
    
    Adding a whole extra mutex to protect a single boolean is somewhat of an
    overkill, but better than worrying about “subtle bugs and occasionally
    undefined behaviour” the g_atomic_* API is warning about.
    
    https://bugs.freedesktop.org/show_bug.cgi?id=69501
    also
    https://bugs.freedesktop.org/show_bug.cgi?id=77524

diff --git a/src/polkitbackend/polkitbackendjsauthority.c b/src/polkitbackend/polkitbackendjsauthority.c
index 8a0a097..097dcc5 100644
--- a/src/polkitbackend/polkitbackendjsauthority.c
+++ b/src/polkitbackend/polkitbackendjsauthority.c
@@ -80,6 +80,8 @@ struct _PolkitBackendJsAuthorityPrivate
   GMainContext *rkt_context;
   GMainLoop *rkt_loop;
   GSource *rkt_source;
+  GMutex rkt_timeout_pending_mutex;
+  gboolean rkt_timeout_pending;
 
   /* A list of JSObject instances */
   GList *scripts;
@@ -528,6 +530,7 @@ polkit_backend_js_authority_constructed (GObject *object)
 
   g_mutex_init (&authority->priv->rkt_init_mutex);
   g_cond_init (&authority->priv->rkt_init_cond);
+  g_mutex_init (&authority->priv->rkt_timeout_pending_mutex);
 
   authority->priv->runaway_killer_thread = g_thread_new ("runaway-killer-thread",
                                                          runaway_killer_thread_func,
@@ -563,6 +566,7 @@ polkit_backend_js_authority_finalize (GObject *object)
 
   g_mutex_clear (&authority->priv->rkt_init_mutex);
   g_cond_clear (&authority->priv->rkt_init_cond);
+  g_mutex_clear (&authority->priv->rkt_timeout_pending_mutex);
 
   /* shut down the killer thread */
   g_assert (authority->priv->rkt_loop != NULL);
@@ -957,6 +961,18 @@ js_operation_callback (JSContext *cx)
   JSString *val_str;
   jsval val;
 
+  /* This callback can be called by the runtime at any time without us causing
+   * it by JS_TriggerOperationCallback().
+   */
+  g_mutex_lock (&authority->priv->rkt_timeout_pending_mutex);
+  if (!authority->priv->rkt_timeout_pending)
+    {
+      g_mutex_unlock (&authority->priv->rkt_timeout_pending_mutex);
+      return JS_TRUE;
+    }
+  authority->priv->rkt_timeout_pending = FALSE;
+  g_mutex_unlock (&authority->priv->rkt_timeout_pending_mutex);
+
   /* Log that we are terminating the script */
   polkit_backend_authority_log (POLKIT_BACKEND_AUTHORITY (authority), "Terminating runaway script");
 
@@ -974,6 +990,10 @@ rkt_on_timeout (gpointer user_data)
 {
   PolkitBackendJsAuthority *authority = POLKIT_BACKEND_JS_AUTHORITY (user_data);
 
+  g_mutex_lock (&authority->priv->rkt_timeout_pending_mutex);
+  authority->priv->rkt_timeout_pending = TRUE;
+  g_mutex_unlock (&authority->priv->rkt_timeout_pending_mutex);
+
   /* Supposedly this is thread-safe... */
 #if JS_VERSION == 186
   JS_TriggerOperationCallback (authority->priv->rt);
@@ -993,6 +1013,9 @@ runaway_killer_setup (PolkitBackendJsAuthority *authority)
   g_assert (authority->priv->rkt_source == NULL);
 
   /* set-up timer for runaway scripts, will be executed in runaway_killer_thread */
+  g_mutex_lock (&authority->priv->rkt_timeout_pending_mutex);
+  authority->priv->rkt_timeout_pending = FALSE;
+  g_mutex_unlock (&authority->priv->rkt_timeout_pending_mutex);
   authority->priv->rkt_source = g_timeout_source_new_seconds (15);
   g_source_set_callback (authority->priv->rkt_source, rkt_on_timeout, authority, NULL);
   g_source_attach (authority->priv->rkt_source, authority->priv->rkt_context);
commit b544f10dd469ae3cfedc026db71ee76e9ef511a2
Author: Miloslav Trmač <mitr at redhat.com>
Date:   Tue Jul 1 20:00:48 2014 +0200

    Clear the JS operation callback before invoking JS in the callback
    
    Setting the callback to NULL is required by
    https://developer.mozilla.org/en-US/docs/SpiderMonkey/JSAPI_Reference/JS_SetOperationCallback
    to avoid the possibility of recursion.
    
    https://bugs.freedesktop.org/show_bug.cgi?id=69501

diff --git a/src/polkitbackend/polkitbackendjsauthority.c b/src/polkitbackend/polkitbackendjsauthority.c
index 22812a6..8a0a097 100644
--- a/src/polkitbackend/polkitbackendjsauthority.c
+++ b/src/polkitbackend/polkitbackendjsauthority.c
@@ -961,9 +961,11 @@ js_operation_callback (JSContext *cx)
   polkit_backend_authority_log (POLKIT_BACKEND_AUTHORITY (authority), "Terminating runaway script");
 
   /* Throw an exception - this way the JS code can ignore the runaway script handling */
+  JS_SetOperationCallback (authority->priv->cx, NULL);
   val_str = JS_NewStringCopyZ (cx, "Terminating runaway script");
   val = STRING_TO_JSVAL (val_str);
   JS_SetPendingException (authority->priv->cx, val);
+  JS_SetOperationCallback (authority->priv->cx, js_operation_callback);
   return JS_FALSE;
 }
 
commit 2881f8b260c03df29afb0e35e6d1707240f95ad7
Author: Miloslav Trmač <mitr at redhat.com>
Date:   Tue Jul 1 20:00:48 2014 +0200

    Prevent builds against SpiderMonkey with exact stack rooting
    
    “Exact stack rooting” means that every on-stack pointer to a JavaScript
    value needs to be registered with the runtime.  The current code doesn't
    do this, so it is not safe to use against a runtime with this
    configuration.  Luckily this configuration is not default.
    
    See
    https://developer.mozilla.org/en-US/docs/SpiderMonkey/Internals/GC/Exact_Stack_Rooting
    and other pages in the wiki for what the conversion would require.
    
    https://bugs.freedesktop.org/show_bug.cgi?id=69501

diff --git a/src/polkitbackend/polkitbackendjsauthority.c b/src/polkitbackend/polkitbackendjsauthority.c
index 39f7060..22812a6 100644
--- a/src/polkitbackend/polkitbackendjsauthority.c
+++ b/src/polkitbackend/polkitbackendjsauthority.c
@@ -43,6 +43,13 @@
 
 #include "initjs.h" /* init.js */
 
+#ifdef JSGC_USE_EXACT_ROOTING
+/* See https://developer.mozilla.org/en-US/docs/SpiderMonkey/Internals/GC/Exact_Stack_Rooting
+ * for more information about exact stack rooting.
+ */
+#error "This code is not safe in SpiderMonkey exact stack rooting configurations"
+#endif
+
 /**
  * SECTION:polkitbackendjsauthority
  * @title: PolkitBackendJsAuthority
commit 5c668722320eb363f713a0998934aa48fecd56cb
Author: Miloslav Trmač <mitr at redhat.com>
Date:   Tue Jul 1 20:00:48 2014 +0200

    Register heap-based JSObject pointers to GC
    
    This is necessary so that the GC can move the objects (though I haven't
    so far encountered this in testing).
    
    https://bugs.freedesktop.org/show_bug.cgi?id=69501

diff --git a/src/polkitbackend/polkitbackendjsauthority.c b/src/polkitbackend/polkitbackendjsauthority.c
index 88f31bd..39f7060 100644
--- a/src/polkitbackend/polkitbackendjsauthority.c
+++ b/src/polkitbackend/polkitbackendjsauthority.c
@@ -482,6 +482,7 @@ polkit_backend_js_authority_constructed (GObject *object)
 
   if (authority->priv->js_global == NULL)
     goto fail;
+  JS_AddObjectRoot (authority->priv->cx, &authority->priv->js_global);
 
   if (!JS_InitStandardClasses (authority->priv->cx, authority->priv->js_global))
     goto fail;
@@ -494,6 +495,7 @@ polkit_backend_js_authority_constructed (GObject *object)
                                                 JSPROP_ENUMERATE);
   if (authority->priv->js_polkit == NULL)
     goto fail;
+  JS_AddObjectRoot (authority->priv->cx, &authority->priv->js_polkit);
 
   if (!JS_DefineFunctions (authority->priv->cx,
                            authority->priv->js_polkit,
@@ -572,6 +574,11 @@ polkit_backend_js_authority_finalize (GObject *object)
   g_free (authority->priv->dir_monitors);
   g_strfreev (authority->priv->rules_dirs);
 
+  JS_BeginRequest (authority->priv->cx);
+  JS_RemoveObjectRoot (authority->priv->cx, &authority->priv->js_polkit);
+  JS_RemoveObjectRoot (authority->priv->cx, &authority->priv->js_global);
+  JS_EndRequest (authority->priv->cx);
+
   JS_DestroyContext (authority->priv->cx);
   JS_DestroyRuntime (authority->priv->rt);
   /* JS_ShutDown (); */
commit 57e2d86edc2630cac1812a3285715dad795a4bd6
Author: Miloslav Trmač <mitr at redhat.com>
Date:   Tue Jul 1 20:00:48 2014 +0200

    Wrap all JS usage within “requests”
    
    Required by
    https://developer.mozilla.org/en-US/docs/Mozilla/Projects/SpiderMonkey/JSAPI_reference/JS_THREADSAFE
    ; lack of requests causes assertion failures with a debug build of
    mozjs17.
    
    https://bugs.freedesktop.org/show_bug.cgi?id=69501

diff --git a/src/polkitbackend/polkitbackendjsauthority.c b/src/polkitbackend/polkitbackendjsauthority.c
index d02e5e3..88f31bd 100644
--- a/src/polkitbackend/polkitbackendjsauthority.c
+++ b/src/polkitbackend/polkitbackendjsauthority.c
@@ -239,6 +239,7 @@ rules_file_name_cmp (const gchar *a,
   return ret;
 }
 
+/* authority->priv->cx must be within a request */
 static void
 load_scripts (PolkitBackendJsAuthority  *authority)
 {
@@ -339,6 +340,8 @@ reload_scripts (PolkitBackendJsAuthority *authority)
   jsval argv[1] = {JSVAL_NULL};
   jsval rval = JSVAL_NULL;
 
+  JS_BeginRequest (authority->priv->cx);
+
   if (!JS_CallFunctionName(authority->priv->cx,
                            authority->priv->js_polkit,
                            "_deleteRules",
@@ -364,7 +367,7 @@ reload_scripts (PolkitBackendJsAuthority *authority)
   /* Let applications know we have new rules... */
   g_signal_emit_by_name (authority, "changed");
  out:
-  ;
+  JS_EndRequest (authority->priv->cx);
 }
 
 static void
@@ -447,6 +450,7 @@ static void
 polkit_backend_js_authority_constructed (GObject *object)
 {
   PolkitBackendJsAuthority *authority = POLKIT_BACKEND_JS_AUTHORITY (object);
+  gboolean entered_request = FALSE;
 
   authority->priv->rt = JS_NewRuntime (8L * 1024L * 1024L);
   if (authority->priv->rt == NULL)
@@ -466,6 +470,9 @@ polkit_backend_js_authority_constructed (GObject *object)
   JS_SetErrorReporter(authority->priv->cx, report_error);
   JS_SetContextPrivate (authority->priv->cx, authority);
 
+  JS_BeginRequest(authority->priv->cx);
+  entered_request = TRUE;
+
   authority->priv->js_global =
 #if JS_VERSION == 186
     JS_NewGlobalObject (authority->priv->cx, &js_global_class, NULL);
@@ -526,10 +533,15 @@ polkit_backend_js_authority_constructed (GObject *object)
   setup_file_monitors (authority);
   load_scripts (authority);
 
+  JS_EndRequest (authority->priv->cx);
+  entered_request = FALSE;
+
   G_OBJECT_CLASS (polkit_backend_js_authority_parent_class)->constructed (object);
   return;
 
  fail:
+  if (entered_request)
+    JS_EndRequest (authority->priv->cx);
   g_critical ("Error initializing JavaScript environment");
   g_assert_not_reached ();
 }
@@ -642,6 +654,7 @@ polkit_backend_js_authority_class_init (PolkitBackendJsAuthorityClass *klass)
 
 /* ---------------------------------------------------------------------------------------------------- */
 
+/* authority->priv->cx must be within a request */
 static void
 set_property_str (PolkitBackendJsAuthority  *authority,
                   JSObject                  *obj,
@@ -655,6 +668,7 @@ set_property_str (PolkitBackendJsAuthority  *authority,
   JS_SetProperty (authority->priv->cx, obj, name, &value_jsval);
 }
 
+/* authority->priv->cx must be within a request */
 static void
 set_property_strv (PolkitBackendJsAuthority  *authority,
                    JSObject                  *obj,
@@ -681,7 +695,7 @@ set_property_strv (PolkitBackendJsAuthority  *authority,
   JS_SetProperty (authority->priv->cx, obj, name, &value_jsval);
 }
 
-
+/* authority->priv->cx must be within a request */
 static void
 set_property_int32 (PolkitBackendJsAuthority  *authority,
                     JSObject                  *obj,
@@ -693,6 +707,7 @@ set_property_int32 (PolkitBackendJsAuthority  *authority,
   JS_SetProperty (authority->priv->cx, obj, name, &value_jsval);
 }
 
+/* authority->priv->cx must be within a request */
 static void
 set_property_bool (PolkitBackendJsAuthority  *authority,
                    JSObject                  *obj,
@@ -706,6 +721,7 @@ set_property_bool (PolkitBackendJsAuthority  *authority,
 
 /* ---------------------------------------------------------------------------------------------------- */
 
+/* authority->priv->cx must be within a request */
 static gboolean
 subject_to_jsval (PolkitBackendJsAuthority  *authority,
                   PolkitSubject             *subject,
@@ -838,6 +854,7 @@ subject_to_jsval (PolkitBackendJsAuthority  *authority,
 
 /* ---------------------------------------------------------------------------------------------------- */
 
+/* authority->priv->cx must be within a request */
 static gboolean
 action_and_details_to_jsval (PolkitBackendJsAuthority  *authority,
                              const gchar               *action_id,
@@ -1041,6 +1058,8 @@ polkit_backend_js_authority_get_admin_auth_identities (PolkitBackendInteractiveA
   gchar *ret_str = NULL;
   gchar **ret_strs = NULL;
 
+  JS_BeginRequest (authority->priv->cx);
+
   if (!action_and_details_to_jsval (authority, action_id, details, &argv[0], &error))
     {
       polkit_backend_authority_log (POLKIT_BACKEND_AUTHORITY (authority),
@@ -1120,6 +1139,8 @@ polkit_backend_js_authority_get_admin_auth_identities (PolkitBackendInteractiveA
 
   JS_MaybeGC (authority->priv->cx);
 
+  JS_EndRequest (authority->priv->cx);
+
   return ret;
 }
 
@@ -1146,6 +1167,8 @@ polkit_backend_js_authority_check_authorization_sync (PolkitBackendInteractiveAu
   gchar *ret_str = NULL;
   gboolean good = FALSE;
 
+  JS_BeginRequest (authority->priv->cx);
+
   if (!action_and_details_to_jsval (authority, action_id, details, &argv[0], &error))
     {
       polkit_backend_authority_log (POLKIT_BACKEND_AUTHORITY (authority),
@@ -1222,6 +1245,8 @@ polkit_backend_js_authority_check_authorization_sync (PolkitBackendInteractiveAu
 
   JS_MaybeGC (authority->priv->cx);
 
+  JS_EndRequest (authority->priv->cx);
+
   return ret;
 }
 
commit ec039f9d7ede5b839f5511e26d5cd6ae9107cb2e
Author: Miloslav Trmač <mitr at redhat.com>
Date:   Tue Jul 1 20:00:48 2014 +0200

    Fix a memory leak when registering an authentication agent
    
    https://bugs.freedesktop.org/show_bug.cgi?id=69501

diff --git a/src/polkitbackend/polkitbackendauthority.c b/src/polkitbackend/polkitbackendauthority.c
index 14eea99..64560e1 100644
--- a/src/polkitbackend/polkitbackendauthority.c
+++ b/src/polkitbackend/polkitbackendauthority.c
@@ -900,6 +900,7 @@ server_handle_register_authentication_agent (Server                 *server,
   g_dbus_method_invocation_return_value (invocation, g_variant_new ("()"));
 
  out:
+  g_variant_unref (subject_gvariant);
   if (subject != NULL)
     g_object_unref (subject);
 }
commit 0f5852a4bdabe377ddcdbed09a0c1f95710e17fe
Author: Miloslav Trmač <mitr at redhat.com>
Date:   Tue Jul 1 20:00:48 2014 +0200

    Fix a per-authorization memory leak
    
    We were leaking PolkitAuthorizationResult on every request, primarily on
    the success path, but also on various error paths as well.
    
    https://bugs.freedesktop.org/show_bug.cgi?id=69501

diff --git a/src/polkitbackend/polkitbackendauthority.c b/src/polkitbackend/polkitbackendauthority.c
index a09d667..14eea99 100644
--- a/src/polkitbackend/polkitbackendauthority.c
+++ b/src/polkitbackend/polkitbackendauthority.c
@@ -714,6 +714,7 @@ check_auth_cb (GObject      *source_object,
       g_variant_ref_sink (value);
       g_dbus_method_invocation_return_value (data->invocation, g_variant_new ("(@(bba{ss}))", value));
       g_variant_unref (value);
+      g_object_unref (result);
     }
 
   check_auth_data_free (data);
diff --git a/src/polkitbackend/polkitbackendinteractiveauthority.c b/src/polkitbackend/polkitbackendinteractiveauthority.c
index 96725f7..7019356 100644
--- a/src/polkitbackend/polkitbackendinteractiveauthority.c
+++ b/src/polkitbackend/polkitbackendinteractiveauthority.c
@@ -1022,7 +1022,7 @@ polkit_backend_interactive_authority_check_authorization (PolkitBackendAuthority
 
   /* Otherwise just return the result */
   g_simple_async_result_set_op_res_gpointer (simple,
-                                             result,
+                                             g_object_ref (result),
                                              g_object_unref);
   g_simple_async_result_complete (simple);
   g_object_unref (simple);
@@ -1039,6 +1039,9 @@ polkit_backend_interactive_authority_check_authorization (PolkitBackendAuthority
   g_free (subject_str);
   g_free (user_of_caller_str);
   g_free (user_of_subject_str);
+
+  if (result != NULL)
+    g_object_unref (result);
 }
 
 /* ---------------------------------------------------------------------------------------------------- */
commit cbad0d5721804a4b7c2d998b00da9e70dc623820
Author: Miloslav Trmač <mitr at redhat.com>
Date:   Tue Jul 1 20:00:48 2014 +0200

    Don't store unrooted jsvals on heap
    
    Don't create a temporary array of jsvals on heap; the GC is not looking
    for GC roots there.
    
    Compare
    https://developer.mozilla.org/en-US/docs/SpiderMonkey/GC_Rooting_Guide
    and
    https://web.archive.org/web/20140305233124/https://developer.mozilla.org/en-US/docs/SpiderMonkey_Garbage_Collection_Tips
    .
    
    https://bugs.freedesktop.org/show_bug.cgi?id=69501

diff --git a/src/polkitbackend/polkitbackendjsauthority.c b/src/polkitbackend/polkitbackendjsauthority.c
index efb07a9..d02e5e3 100644
--- a/src/polkitbackend/polkitbackendjsauthority.c
+++ b/src/polkitbackend/polkitbackendjsauthority.c
@@ -663,23 +663,22 @@ set_property_strv (PolkitBackendJsAuthority  *authority,
 {
   jsval value_jsval;
   JSObject *array_object;
-  jsval *jsvals;
   guint n;
 
-  jsvals = g_new0 (jsval, value->len);
+  array_object = JS_NewArrayObject (authority->priv->cx, 0, NULL);
+
   for (n = 0; n < value->len; n++)
     {
       JSString *jsstr;
+      jsval val;
+
       jsstr = JS_NewStringCopyZ (authority->priv->cx, g_ptr_array_index(value, n));
-      jsvals[n] = STRING_TO_JSVAL (jsstr);
+      val = STRING_TO_JSVAL (jsstr);
+      JS_SetElement (authority->priv->cx, array_object, n, &val);
     }
 
-  array_object = JS_NewArrayObject (authority->priv->cx, value->len, jsvals);
-
   value_jsval = OBJECT_TO_JSVAL (array_object);
   JS_SetProperty (authority->priv->cx, obj, name, &value_jsval);
-
-  g_free (jsvals);
 }
 
 
commit a97672540c66c03ed392fc072f0c682281f08989
Author: Miloslav Trmač <mitr at redhat.com>
Date:   Tue Jul 1 20:00:48 2014 +0200

    Don't add extra NULL group to subject.groups
    
    The NULL “terminator” of ‘groups’ was being passed to JavaScript.  Drop
    it, and simplify by leting set_property_strv use the GPtrArray directly
    instead of the extra conversions “into” a strv and a completely dead
    g_strv_length().
    
    https://bugs.freedesktop.org/show_bug.cgi?id=69501

diff --git a/src/polkitbackend/polkitbackendjsauthority.c b/src/polkitbackend/polkitbackendjsauthority.c
index c7a29e0..efb07a9 100644
--- a/src/polkitbackend/polkitbackendjsauthority.c
+++ b/src/polkitbackend/polkitbackendjsauthority.c
@@ -659,26 +659,22 @@ static void
 set_property_strv (PolkitBackendJsAuthority  *authority,
                    JSObject                  *obj,
                    const gchar               *name,
-                   const gchar *const        *value,
-                   gssize                     len)
+                   GPtrArray                 *value)
 {
   jsval value_jsval;
   JSObject *array_object;
   jsval *jsvals;
   guint n;
 
-  if (len < 0)
-    len = g_strv_length ((gchar **) value);
-
-  jsvals = g_new0 (jsval, len);
-  for (n = 0; n < len; n++)
+  jsvals = g_new0 (jsval, value->len);
+  for (n = 0; n < value->len; n++)
     {
       JSString *jsstr;
-      jsstr = JS_NewStringCopyZ (authority->priv->cx, value[n]);
+      jsstr = JS_NewStringCopyZ (authority->priv->cx, g_ptr_array_index(value, n));
       jsvals[n] = STRING_TO_JSVAL (jsstr);
     }
 
-  array_object = JS_NewArrayObject (authority->priv->cx, (gint32) len, jsvals);
+  array_object = JS_NewArrayObject (authority->priv->cx, value->len, jsvals);
 
   value_jsval = OBJECT_TO_JSVAL (array_object);
   JS_SetProperty (authority->priv->cx, obj, name, &value_jsval);
@@ -818,11 +814,9 @@ subject_to_jsval (PolkitBackendJsAuthority  *authority,
         }
     }
 
-  g_ptr_array_add (groups, NULL);
-
   set_property_int32 (authority, obj, "pid", pid);
   set_property_str (authority, obj, "user", user_name);
-  set_property_strv (authority, obj, "groups", (const gchar* const *) groups->pdata, groups->len);
+  set_property_strv (authority, obj, "groups", groups);
   set_property_str (authority, obj, "seat", seat_str);
   set_property_str (authority, obj, "session", session_str);
   set_property_bool (authority, obj, "local", subject_is_local);
commit 983e8ec37b0ec1cc5114cb9ca49cf558dedfb31e
Author: Miloslav Trmač <mitr at redhat.com>
Date:   Tue Jul 1 20:00:48 2014 +0200

    Don't pass an uninitialized JS parameter
    
    Don't pass argc==3 when using a 2-member array in
    polkit_backend_js_authority_check_authorization_sync .  To avoid such
    problems in the future, use G_N_ELEMENTS in both similar callers.
    
    https://bugs.freedesktop.org/show_bug.cgi?id=69501

diff --git a/src/polkitbackend/polkitbackendjsauthority.c b/src/polkitbackend/polkitbackendjsauthority.c
index c232573..c7a29e0 100644
--- a/src/polkitbackend/polkitbackendjsauthority.c
+++ b/src/polkitbackend/polkitbackendjsauthority.c
@@ -1074,7 +1074,7 @@ polkit_backend_js_authority_get_admin_auth_identities (PolkitBackendInteractiveA
 
   if (!call_js_function_with_runaway_killer (authority,
                                              "_runAdminRules",
-                                             2,
+                                             G_N_ELEMENTS (argv),
                                              argv,
                                              &rval))
     {
@@ -1179,7 +1179,7 @@ polkit_backend_js_authority_check_authorization_sync (PolkitBackendInteractiveAu
 
   if (!call_js_function_with_runaway_killer (authority,
                                              "_runRules",
-                                             3,
+                                             G_N_ELEMENTS (argv),
                                              argv,
                                              &rval))
     {


More information about the hal-commit mailing list