commit 983e8ec37b0ec1cc5114cb9ca49cf558dedfb31e
Author: Miloslav Trmač <mitr@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))
{
commit a97672540c66c03ed392fc072f0c682281f08989
Author: Miloslav Trmač <mitr@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 cbad0d5721804a4b7c2d998b00da9e70dc623820
Author: Miloslav Trmač <mitr@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 0f5852a4bdabe377ddcdbed09a0c1f95710e17fe
Author: Miloslav Trmač <mitr@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 ec039f9d7ede5b839f5511e26d5cd6ae9107cb2e
Author: Miloslav Trmač <mitr@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 57e2d86edc2630cac1812a3285715dad795a4bd6
Author: Miloslav Trmač <mitr@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 5c668722320eb363f713a0998934aa48fecd56cb
Author: Miloslav Trmač <mitr@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 2881f8b260c03df29afb0e35e6d1707240f95ad7
Author: Miloslav Trmač <mitr@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 b544f10dd469ae3cfedc026db71ee76e9ef511a2
Author: Miloslav Trmač <mitr@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 d7da6a23766e9c95fa333a0a9c742f7397c0ad22
Author: Miloslav Trmač <mitr@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);