Bugzilla@Mozilla – Bug 619255
GC hazard in JO() from json.cpp due to reuse of root (1.9.[12]-only)
Last modified: 2011-03-29 19:27:27 PDT
Summon comment box
On 1.9.2 branch JO() from json.cpp incorrectly reuse the passed vp parameter for rooting an iterator leading to a GC hazard. The following example demonstrates this for a debug build of JS shell: function iter() { ({}); gc(); yield 1; } function create_iter() { return iter(); } JSON.stringify({ get o() { var obj = {}; obj.__iterator__ = create_iter; return obj; }}); ~/m/192/js/src> ~/b/js/192dbg32/js ~/s/x.js Segmentation fault (core dumped) On trunk we are safe due to the conservative GC scanner.
Created attachment 497715 [details] [review] fix The patch copies the initial value of *vp into an extra root.
Comment on attachment 497715 [details] [review] fix >diff --git a/js/src/json.cpp b/js/src/json.cpp >--- a/js/src/json.cpp >+++ b/js/src/json.cpp >@@ -241,20 +241,20 @@ WriteIndent(JSContext *cx, StringifyCont > > static JSBool > JO(JSContext *cx, jsval *vp, StringifyContext *scx) > { > JSObject *obj = JSVAL_TO_OBJECT(*vp); > > if (!scx->cb.append('{')) > return JS_FALSE; > >- jsval vec[3] = {JSVAL_NULL, JSVAL_NULL, JSVAL_NULL}; >- JSAutoTempValueRooter tvr(cx, 3, vec); >+ jsval vec[] = {JSVAL_NULL, JSVAL_NULL, JSVAL_NULL, *vp}; >+ JSAutoTempValueRooter tvr(cx, JS_ARRAY_LENGTH(vec), vec); > jsval& key = vec[0]; > jsval& outputValue = vec[1]; > > JSObject *iterObj = NULL; > jsval *keySource = vp; > bool usingWhitelist = false; > > // if the replacer is an array, we use the keys from it > if (scx->replacer && JS_IsArrayObject(cx, scx->replacer)) {
Nominating for 1.9.2 due to the critical status.
Does this affect 1.9.1 as well?
(waiting to approve until we know 1.9.1 is (un)affected.
(In reply to comment #4) > Does this affect 1.9.1 as well? Yes, this affects 1.9.1 - I forgot that the native JSON were introduced on that branch first.
Created attachment 498868 [details] [review] fix for 191 191 version of the patch needed a trivial merge. Here is a plain diff between patches: /home/igor/s/> diff json_bug619255.patch json_bug619255_191.patch 4,5c4 < @@ -241,20 +241,20 @@ WriteIndent(JSContext *cx, StringifyCont < --- > @@ -261,20 +261,20 @@ WriteIndent(JSContext *cx, StringifyCont 11c10,11 < if (!scx->cb.append('{')) --- > jschar c = jschar('{'); > if (!scx->callback(&c, 1, scx->data))
Comment on attachment 497715 [details] [review] fix a=LegNeato for 1.9.2.14
Comment on attachment 498868 [details] [review] fix for 191 a=LegNeato for 1.9.1.17
*** This bug has been marked as a duplicate of bug 616009 ***
Verified for branches by verifying bug 616009.