rpms/gjs
7
0
Fork 0
Code Issues Pull Requests Releases Activity

Update to 1.49.3

Browse Source
This commit is contained in:
Kalev Lember 2017-06-25 17:37:12 +02:00
parent 2c93604b58
commit 04c2631ec9
5 changed files with 7 additions and 407 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
.gitignore vendored
View File

@ -64,3 +64,4 @@ gjs-0.7.1.tar.gz
/gjs-1.48.2.tar.xz
/gjs-1.48.3.tar.xz
/gjs-1.49.2.tar.xz
/gjs-1.49.3.tar.xz

236
0001-object-Prevent-use-after-free-in-signal-connections.patch
View File

@ -1,236 +0,0 @@
From 476cc84aa177c7c73e3f5d9568140294a20aaaf1 Mon Sep 17 00:00:00 2001
From: Philip Chimento <philip.chimento@gmail.com>
Date: Sun, 11 Jun 2017 12:29:27 -0700
Subject: [PATCH 1/2] object: Prevent use-after-free in signal connections
Objects trace their signal connections in order to keep the closures
alive during garbage collection. When invalidating a signal connection,
we must do so in an idle function, since it is illegal to stop tracing a
GC-thing in the middle of GC.
However, this caused a possible use-after-free if the signal connection
was invalidated, and then the object itself was finalized before the idle
function could be run.
This refactor avoids the use-after-free by cancelling any pending idle
invalidations in the object's finalizer, and invalidating any remaining
signal connections in such a way that no more idle functions are
scheduled.
https://bugzilla.gnome.org/show_bug.cgi?id=781799
---
gi/object.cpp | 124 +++++++++++++++++++++++++++++++++-------------------------
1 file changed, 71 insertions(+), 53 deletions(-)
diff --git a/gi/object.cpp b/gi/object.cpp
index d98c126..db5338b 100644
--- a/gi/object.cpp
+++ b/gi/object.cpp
@@ -55,6 +55,8 @@
#include <util/hash-x32.h>
#include <girepository.h>
+typedef struct _ConnectData ConnectData;
+
struct ObjectInstance {
GIObjectInfo *info;
GObject *gobj; /* NULL if we are the prototype and not an instance */
@@ -62,7 +64,7 @@ struct ObjectInstance {
GType gtype;
/* a list of all signal connections, used when tracing */
- GList *signals;
+ std::set<ConnectData *> signals;
/* the GObjectClass wrapped by this JS Object (only used for
prototypes) */
@@ -74,11 +76,11 @@ struct ObjectInstance {
unsigned js_object_finalized : 1;
};
-typedef struct {
+struct _ConnectData {
ObjectInstance *obj;
- GList *link;
GClosure *closure;
-} ConnectData;
+ unsigned idle_invalidate_id;
+};
static std::stack<JS::PersistentRootedObject> object_init_list;
static GHashTable *class_init_properties;
@@ -93,7 +95,7 @@ static std::set<ObjectInstance *> dissociate_list;
GJS_DEFINE_PRIV_FROM_JS(ObjectInstance, gjs_object_instance_class)
static void disassociate_js_gobject (GObject *gobj);
-static void invalidate_all_signals (ObjectInstance *priv);
+
typedef enum {
SOME_ERROR_OCCURRED = false,
NO_SUCH_G_PROPERTY,
@@ -1220,6 +1222,21 @@ associate_js_gobject (JSContext *context,
}
static void
+invalidate_all_signals(ObjectInstance *priv)
+{
+ /* Can't loop directly through the items, since invalidating an item's
+ * closure might have the effect of removing the item from the set in the
+ * invalidate notifier */
+ while (!priv->signals.empty()) {
+ /* This will also free cd, through the closure invalidation mechanism */
+ ConnectData *cd = *priv->signals.begin();
+ g_closure_invalidate(cd->closure);
+ /* Erase element if not already erased */
+ priv->signals.erase(cd);
+ }
+}
+
+static void
disassociate_js_gobject(GObject *gobj)
{
ObjectInstance *priv = get_object_qdata(gobj);
@@ -1369,43 +1386,44 @@ GJS_NATIVE_CONSTRUCTOR_DECLARE(object_instance)
}
static void
-invalidate_all_signals(ObjectInstance *priv)
-{
- GList *iter, *next;
-
- for (iter = priv->signals; iter; ) {
- ConnectData *cd = (ConnectData*) iter->data;
- next = iter->next;
-
- /* This will also free cd and iter, through
- the closure invalidation mechanism */
- g_closure_invalidate(cd->closure);
-
- iter = next;
- }
-}
-
-static void
object_instance_trace(JSTracer *tracer,
JSObject *obj)
{
ObjectInstance *priv;
- GList *iter;
priv = (ObjectInstance *) JS_GetPrivate(obj);
if (priv == NULL)
return;
- for (iter = priv->signals; iter; iter = iter->next) {
- ConnectData *cd = (ConnectData *) iter->data;
-
+ for (ConnectData *cd : priv->signals)
gjs_closure_trace(cd->closure, tracer);
- }
for (auto vfunc : priv->vfuncs)
vfunc->js_function.trace(tracer, "ObjectInstance::vfunc");
}
+/* Removing the signal connection data from the list means that the object stops
+ * tracing the JS function objects belonging to the closures. Incremental GC
+ * does not allow that in the middle of a garbage collection. Therefore, we must
+ * do it in an idle handler.
+ */
+static gboolean
+signal_connection_invalidate_idle(void *user_data)
+{
+ auto cd = static_cast<ConnectData *>(user_data);
+ cd->obj->signals.erase(cd);
+ g_slice_free(ConnectData, cd);
+ return G_SOURCE_REMOVE;
+}
+
+static void
+signal_connection_invalidated(void *data,
+ GClosure *closure)
+{
+ auto cd = static_cast<ConnectData *>(data);
+ cd->idle_invalidate_id = g_idle_add(signal_connection_invalidate_idle, cd);
+}
+
static void
object_instance_finalize(JSFreeOp *fop,
JSObject *obj)
@@ -1429,7 +1447,31 @@ object_instance_finalize(JSFreeOp *fop,
bool had_toggle_up;
bool had_toggle_down;
- invalidate_all_signals (priv);
+ /* We must invalidate all signal connections now, instead of in an idle
+ * handler, because the object will not exist anymore when we get
+ * around to the idle function. We originally needed to defer these
+ * invalidations to an idle function since the object needs to continue
+ * tracing its signal connections while GC is going on. However, once
+ * the object is finalized, it will not be tracing them any longer
+ * anyway, so it's safe to do them now.
+ *
+ * This is basically the same as invalidate_all_signals(), but does not
+ * defer the invalidation to an idle handler.
+ */
+ for (ConnectData *cd : priv->signals) {
+ /* First remove any pending invalidation, we are doing it now. */
+ if (cd->idle_invalidate_id > 0)
+ g_source_remove(cd->idle_invalidate_id);
+
+ /* We also have to remove the invalidate notifier, which would
+ * otherwise schedule a new pending invalidation. */
+ g_closure_remove_invalidate_notifier(cd->closure, cd,
+ signal_connection_invalidated);
+
+ g_closure_invalidate(cd->closure);
+ g_slice_free(ConnectData, cd);
+ }
+ priv->signals.clear();
if (G_UNLIKELY (priv->gobj->ref_count <= 0)) {
g_error("Finalizing proxy for an already freed object of type: %s.%s\n",
@@ -1563,29 +1605,6 @@ gjs_lookup_object_prototype(JSContext *context,
return proto;
}
-/* Removing the signal connection data from the list means that the object stops
- * tracing the JS function objects belonging to the closures. Incremental GC
- * does not allow that in the middle of a garbage collection. Therefore, we must
- * do it in an idle handler.
- */
-static gboolean
-signal_connection_invalidate_idle(void *user_data)
-{
- ConnectData *connect_data = (ConnectData *) user_data;
-
- connect_data->obj->signals = g_list_delete_link(connect_data->obj->signals,
- connect_data->link);
- g_slice_free(ConnectData, connect_data);
- return G_SOURCE_REMOVE;
-}
-
-static void
-signal_connection_invalidated(void *data,
- GClosure *closure)
-{
- g_idle_add(signal_connection_invalidate_idle, data);
-}
-
static bool
real_connect_func(JSContext *context,
unsigned argc,
@@ -1639,9 +1658,8 @@ real_connect_func(JSContext *context,
goto out;
connect_data = g_slice_new(ConnectData);
- priv->signals = g_list_prepend(priv->signals, connect_data);
+ priv->signals.insert(connect_data);
connect_data->obj = priv;
- connect_data->link = priv->signals;
/* This is a weak reference, and will be cleared when the closure is invalidated */
connect_data->closure = closure;
g_closure_add_invalidate_notifier(closure, connect_data, signal_connection_invalidated);
--
2.13.0

164
0002-util-root-Allow-GjsMaybeOwned-DestroyNotify-to-free.patch
View File

@ -1,164 +0,0 @@
From 834eede633dff16910ca69f3d0b4876a99c65ba2 Mon Sep 17 00:00:00 2001
From: Philip Chimento <philip.chimento@gmail.com>
Date: Sun, 11 Jun 2017 12:59:12 -0700
Subject: [PATCH 2/2] util-root: Allow GjsMaybeOwned::DestroyNotify to free
In the case of a closure, the GjsMaybeOwned object is embedded as part of
struct Closure. The context destroy notify callback will invalidate the
closure, which frees the GjsMaybeOwned object, causing a use-after-free
when the callback returns.
This patch gives the callback a boolean return value; it should return
true if it has freed the GjsMaybeOwned object and false if it does not.
If the callback returns true, then the GjsMaybeOwned object will be
considered invalid from then on.
https://bugzilla.gnome.org/show_bug.cgi?id=781799
---
gi/closure.cpp | 12 +++++++-----
gi/object.cpp | 3 ++-
gjs/jsapi-util-root.h | 10 ++++++----
test/gjs-test-rooting.cpp | 26 +++++++++++++++++++++++++-
4 files changed, 40 insertions(+), 11 deletions(-)
diff --git a/gi/closure.cpp b/gi/closure.cpp
index e3ef0ba..d3d5b2f 100644
--- a/gi/closure.cpp
+++ b/gi/closure.cpp
@@ -98,7 +98,7 @@ invalidate_js_pointers(GjsClosure *gc)
g_closure_invalidate(&gc->base);
}
-static void
+static bool
global_context_finalized(JS::HandleObject obj,
void *data)
{
@@ -109,11 +109,13 @@ global_context_finalized(JS::HandleObject obj,
"which calls object %p",
c, c->obj.get());
- if (c->obj != NULL) {
- g_assert(c->obj == obj);
+ if (!c->obj)
+ return false;
- invalidate_js_pointers(gc);
- }
+ g_assert(c->obj == obj);
+
+ invalidate_js_pointers(gc);
+ return true;
}
/* Closures have to drop their references to their JS functions in an idle
diff --git a/gi/object.cpp b/gi/object.cpp
index db5338b..0e38bc3 100644
--- a/gi/object.cpp
+++ b/gi/object.cpp
@@ -915,7 +915,7 @@ wrapped_gobj_dispose_notify(gpointer data,
#endif
}
-static void
+static bool
gobj_no_longer_kept_alive_func(JS::HandleObject obj,
void *data)
{
@@ -930,6 +930,7 @@ gobj_no_longer_kept_alive_func(JS::HandleObject obj,
priv->keep_alive.reset();
dissociate_list_remove(priv);
weak_pointer_list.erase(priv);
+ return false;
}
static void
diff --git a/gjs/jsapi-util-root.h b/gjs/jsapi-util-root.h
index 1bdf029..5e13177 100644
--- a/gjs/jsapi-util-root.h
+++ b/gjs/jsapi-util-root.h
@@ -110,7 +110,7 @@ struct GjsHeapOperation<JS::Value> {
template<typename T>
class GjsMaybeOwned {
public:
- typedef void (*DestroyNotify)(JS::Handle<T> thing, void *data);
+ using DestroyNotify = bool (*)(JS::Handle<T> thing, void *data);
private:
bool m_rooted; /* wrapper is in rooted mode */
@@ -170,9 +170,11 @@ private:
* to remove it. */
m_has_weakref = false;
- /* The object is still live across this callback. */
- if (m_notify)
- m_notify(handle(), m_data);
+ /* The object is still live entering this callback. The callback may
+ * destroy this wrapper if it's part of a larger struct, in which case
+ * it should return true so that we don't touch it */
+ if (m_notify && m_notify(handle(), m_data))
+ return;
reset();
}
diff --git a/test/gjs-test-rooting.cpp b/test/gjs-test-rooting.cpp
index 240455f..2c2dbdb 100644
--- a/test/gjs-test-rooting.cpp
+++ b/test/gjs-test-rooting.cpp
@@ -212,7 +212,7 @@ test_maybe_owned_switch_to_unrooted_allows_collection(GjsRootingFixture *fx,
delete obj;
}
-static void
+static bool
context_destroyed(JS::HandleObject obj,
void *data)
{
@@ -220,6 +220,16 @@ context_destroyed(JS::HandleObject obj,
g_assert_false(fx->notify_called);
g_assert_false(fx->finalized);
fx->notify_called = true;
+ return false;
+}
+
+static bool
+context_destroyed_trash_object(JS::HandleObject jsobj,
+ void *data)
+{
+ auto obj = static_cast<GjsMaybeOwned<JSObject *> *>(data);
+ memset(obj, -1, sizeof(GjsMaybeOwned<JSObject *>));
+ return true;
}
static void
@@ -253,6 +263,18 @@ test_maybe_owned_object_destroyed_after_notify(GjsRootingFixture *fx,
delete obj;
}
+static void
+test_maybe_owned_notify_callback_trashes_object_and_returns_true(GjsRootingFixture *fx,
+ gconstpointer unused)
+{
+ auto obj = new GjsMaybeOwned<JSObject *>();
+ obj->root(PARENT(fx)->cx, test_obj_new(fx), context_destroyed_trash_object,
+ obj);
+
+ gjs_unit_test_destroy_context(PARENT(fx));
+ /* No assertions, should crash if test fails */
+}
+
void
gjs_test_add_tests_for_rooting(void)
{
@@ -288,6 +310,8 @@ gjs_test_add_tests_for_rooting(void)
test_maybe_owned_notify_callback_called_on_context_destroy);
ADD_CONTEXT_DESTROY_TEST("maybe-owned/object-destroyed-after-notify",
test_maybe_owned_object_destroyed_after_notify);
+ ADD_CONTEXT_DESTROY_TEST("maybe-owned/notify-callback-trashes-object-and-returns-true",
+ test_maybe_owned_notify_callback_trashes_object_and_returns_true);
#undef ADD_CONTEXT_DESTROY_TEST
}
--
2.13.0

11
gjs.spec
View File

@ -4,8 +4,8 @@
%global mozjs38_version 38.8.0-4
Name: gjs
Version: 1.49.2
Release: 2%{?dist}
Version: 1.49.3
Release: 1%{?dist}
Summary: Javascript Bindings for GNOME
# The following files contain code from Mozilla which
@ -35,10 +35,6 @@ Requires: gobject-introspection%{?_isa} >= %{gobject_introspection_version}
Requires: gtk3%{?_isa} >= %{gtk3_version}
Requires: mozjs38%{?_isa} >= %{mozjs38_version}
# https://bugzilla.gnome.org/show_bug.cgi?id=781799
Patch0: 0001-object-Prevent-use-after-free-in-signal-connections.patch
Patch1: 0002-util-root-Allow-GjsMaybeOwned-DestroyNotify-to-free.patch
%description
Gjs allows using GNOME libraries from Javascript. It's based on the
Spidermonkey Javascript engine from Mozilla and the GObject introspection
@ -104,6 +100,9 @@ find %{buildroot} -name '*.la' -exec rm -f {} ';'
%{_datadir}/installed-tests
%changelog
* Sun Jun 25 2017 Kalev Lember <klember@redhat.com> - 1.49.3-1
- Update to 1.49.3
* Tue Jun 13 2017 Bastien Nocera <bnocera@redhat.com> - 1.49.2-2
+ gjs-1.49.2-2
- Add fix for possible use-after-free crasher (bgo #781799)

2
sources
View File

@ -1 +1 @@
SHA512 (gjs-1.49.2.tar.xz) = 741c7bbac144f1b1748038a6177040528d4519a90a2a73ae920f46816b9c74dd859c6e0f3aceae53521dd5010b39aabc495e58aeefe27e226d846786d1c03ec9
SHA512 (gjs-1.49.3.tar.xz) = 92a06841294a5e57e498756db6074b679f7ffd6f48daa506e9116abf522b30d85a5d8e2fd6e9cdf22c445d1ca4a1f8a26f53dba78b325b2303002dfe81a967bc