Closed Bug 1467274 Opened Last year Closed Last year

Memory leak with OOM in js::JSONParser<CharT>::parse(JS::MutableHandle<JS::Value>)

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla62
Tracking Status
firefox62 --- fixed

People

(Reporter: anba, Assigned: Waldo)

Details

Attachments

(1 file)

|ElementVector* elements| resp. |PropertyVector* properties| is not deleted when |stack.append(...)| in these lines [1,2] return false.


[1] https://searchfox.org/mozilla-central/rev/cf464eabfeba64e866c1fa36b9fefd674dca9c51/js/src/vm/JSONParser.cpp#725,729
[2] https://searchfox.org/mozilla-central/rev/cf464eabfeba64e866c1fa36b9fefd674dca9c51/js/src/vm/JSONParser.cpp#747,751


Test case 1:
---
var n = 12;
var s = "[".repeat(n) + "0" + "]".repeat(n);
s = s.split("").join("");

oomAtAllocation(39);
JSON.parse(s);
---


Test case 2:
---
var n = 12;
var s = "{\"x\":".repeat(n) + "0" + "}".repeat(n);
s = s.split("").join("");

oomAtAllocation(39);
JSON.parse(s);
---




Configure flags: --enable-debug --disable-optimize --disable-tests --enable-valgrind --disable-jemalloc

Run with: valgrind --tool=memcheck --leak-check=yes ~/hg/mozilla-inbound/js/src/build-valgrind-debug-obj/dist/bin/js --no-baseline /tmp/t.js


Output 1:
---
==11042== Memcheck, a memory error detector
==11042== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
==11042== Using Valgrind-3.11.0 and LibVEX; rerun with -h for copyright info
==11042== Command: /home/andre/hg/mozilla-inbound/js/src/build-valgrind-debug-obj/dist/bin/js --no-baseline /tmp/t.js
==11042== 
==11042== Warning: set address range perms: large range [0x293b5c1f8000, 0x293b9c1f8000) (noaccess)
uncaught exception: out of memory
(Unable to print stack trace)
==11042== Warning: set address range perms: large range [0x293b5c1f8000, 0x293b9c1f8000) (noaccess)
==11042== 
==11042== HEAP SUMMARY:
==11042==     in use at exit: 72,987 bytes in 4 blocks
==11042==   total heap usage: 8,956 allocs, 8,952 frees, 6,041,043 bytes allocated
==11042== 
==11042== 208 bytes in 1 blocks are definitely lost in loss record 3 of 4
==11042==    at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==11042==    by 0x491E24: SystemMalloc::malloc(unsigned long) (malloc_decls.h:37)
==11042==    by 0x491D58: DummyArenaAllocator<SystemMalloc>::moz_arena_malloc(unsigned long, unsigned long) (malloc_decls.h:37)
==11042==    by 0x491C4C: moz_arena_malloc (malloc_decls.h:115)
==11042==    by 0x41E632: js_malloc(unsigned long) (Utility.h:388)
==11042==    by 0x4291C1: unsigned char* js_pod_malloc<unsigned char>(unsigned long) (Utility.h:578)
==11042==    by 0x45C073: unsigned char* js::MallocProvider<JSContext>::maybe_pod_malloc<unsigned char>(unsigned long) (MallocProvider.h:54)
==11042==    by 0x45BF2E: unsigned char* js::MallocProvider<JSContext>::pod_malloc<unsigned char>(unsigned long) (MallocProvider.h:87)
==11042==    by 0xFC0A22: mozilla::Vector<JS::Value, 20ul, js::TempAllocPolicy>* js::MallocProvider<JSContext>::new_<mozilla::Vector<JS::Value, 20ul, js::TempAllocPolicy>, JSContext* const&>(JSContext* const&) (in /home/andre/hg/mozilla-inbound/js/src/build-valgrind-debug-obj/dist/bin/js)
==11042==    by 0xFBE4F9: js::JSONParser<unsigned char>::parse(JS::MutableHandle<JS::Value>) (JSONParser.cpp:725)
==11042==    by 0x6D2B8F: js::MutableWrappedPtrOperations<js::JSONParser<unsigned char>, JS::Rooted<js::JSONParser<unsigned char> > >::parse(JS::MutableHandle<JS::Value>) (in /home/andre/hg/mozilla-inbound/js/src/build-valgrind-debug-obj/dist/bin/js)
==11042==    by 0x6D28C7: bool js::ParseJSONWithReviver<unsigned char>(JSContext*, mozilla::Range<unsigned char const>, JS::Handle<JS::Value>, JS::MutableHandle<JS::Value>) (JSON.cpp:895)
==11042== 
==11042== LEAK SUMMARY:
==11042==    definitely lost: 208 bytes in 1 blocks
==11042==    indirectly lost: 0 bytes in 0 blocks
==11042==      possibly lost: 0 bytes in 0 blocks
==11042==    still reachable: 72,779 bytes in 3 blocks
==11042==         suppressed: 0 bytes in 0 blocks
==11042== Reachable blocks (those to which a pointer was found) are not shown.
==11042== To see them, rerun with: --leak-check=full --show-leak-kinds=all
==11042== 
==11042== For counts of detected and suppressed errors, rerun with: -v
==11042== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)
---



Output 2:
---
==11076== Memcheck, a memory error detector
==11076== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
==11076== Using Valgrind-3.11.0 and LibVEX; rerun with -h for copyright info
==11076== Command: /home/andre/hg/mozilla-inbound/js/src/build-valgrind-debug-obj/dist/bin/js --no-baseline /tmp/t.js
==11076== 
==11076== Warning: set address range perms: large range [0x3fd6a28dc000, 0x3fd6e28dc000) (noaccess)
uncaught exception: out of memory
(Unable to print stack trace)
==11076== Warning: set address range perms: large range [0x3fd6a28dc000, 0x3fd6e28dc000) (noaccess)
==11076== 
==11076== HEAP SUMMARY:
==11076==     in use at exit: 72,987 bytes in 4 blocks
==11076==   total heap usage: 8,953 allocs, 8,949 frees, 6,040,148 bytes allocated
==11076== 
==11076== 208 bytes in 1 blocks are definitely lost in loss record 3 of 4
==11076==    at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==11076==    by 0x491E24: SystemMalloc::malloc(unsigned long) (malloc_decls.h:37)
==11076==    by 0x491D58: DummyArenaAllocator<SystemMalloc>::moz_arena_malloc(unsigned long, unsigned long) (malloc_decls.h:37)
==11076==    by 0x491C4C: moz_arena_malloc (malloc_decls.h:115)
==11076==    by 0x41E632: js_malloc(unsigned long) (Utility.h:388)
==11076==    by 0x4291C1: unsigned char* js_pod_malloc<unsigned char>(unsigned long) (Utility.h:578)
==11076==    by 0x45C073: unsigned char* js::MallocProvider<JSContext>::maybe_pod_malloc<unsigned char>(unsigned long) (MallocProvider.h:54)
==11076==    by 0x45BF2E: unsigned char* js::MallocProvider<JSContext>::pod_malloc<unsigned char>(unsigned long) (MallocProvider.h:87)
==11076==    by 0xFC10C2: mozilla::Vector<js::IdValuePair, 10ul, js::TempAllocPolicy>* js::MallocProvider<JSContext>::new_<mozilla::Vector<js::IdValuePair, 10ul, js::TempAllocPolicy>, JSContext* const&>(JSContext* const&) (in /home/andre/hg/mozilla-inbound/js/src/build-valgrind-debug-obj/dist/bin/js)
==11076==    by 0xFBE633: js::JSONParser<unsigned char>::parse(JS::MutableHandle<JS::Value>) (JSONParser.cpp:747)
==11076==    by 0x6D2B8F: js::MutableWrappedPtrOperations<js::JSONParser<unsigned char>, JS::Rooted<js::JSONParser<unsigned char> > >::parse(JS::MutableHandle<JS::Value>) (in /home/andre/hg/mozilla-inbound/js/src/build-valgrind-debug-obj/dist/bin/js)
==11076==    by 0x6D28C7: bool js::ParseJSONWithReviver<unsigned char>(JSContext*, mozilla::Range<unsigned char const>, JS::Handle<JS::Value>, JS::MutableHandle<JS::Value>) (JSON.cpp:895)
==11076== 
==11076== LEAK SUMMARY:
==11076==    definitely lost: 208 bytes in 1 blocks
==11076==    indirectly lost: 0 bytes in 0 blocks
==11076==      possibly lost: 0 bytes in 0 blocks
==11076==    still reachable: 72,779 bytes in 3 blocks
==11076==         suppressed: 0 bytes in 0 blocks
==11076== Reachable blocks (those to which a pointer was found) are not shown.
==11076== To see them, rerun with: --leak-check=full --show-leak-kinds=all
==11076== 
==11076== For counts of detected and suppressed errors, rerun with: -v
==11076== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)
---
Attached patch PatchSplinter Review
Attachment #8985430 - Flags: review?(andrebargull)
Assignee: nobody → jwalden+bmo
Status: NEW → ASSIGNED
Comment on attachment 8985430 [details] [diff] [review]
Patch

Review of attachment 8985430 [details] [diff] [review]:
-----------------------------------------------------------------

r+ with the mem-leak mentioned in the review comments fixed, too.

::: js/src/vm/JSONParser.cpp
@@ +721,5 @@
>                  if (!freeElements.empty()) {
>                      elements = freeElements.popCopy();
>                      elements->clear();
> +
> +                    if (!stack.append(elements))

|elements| needs to be deleted here, too.


Test case:
---
var n = 10;
var freeElements = "[".repeat(n-1) + "0" + "]".repeat(n-1);
var s = "[" + freeElements + "," + "{\"\":" + "[".repeat(n) + "0" + "]".repeat(n) + "}]";
s = s.split("").join("");

oomAtAllocation(78);
JSON.parse(s);
---

@@ +749,5 @@
>                  if (!freeProperties.empty()) {
>                      properties = freeProperties.popCopy();
>                      properties->clear();
> +
> +                    if (!stack.append(properties))

Same issue here.
Attachment #8985430 - Flags: review?(andrebargull) → review+
Pushed by jwalden@mit.edu:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7a56278c7b78
Don't leak ElementVector or PropertyVector on failure to append a raw-pointer instance of such to JSONParser's internal vectors.  r=anba
https://hg.mozilla.org/mozilla-central/rev/7a56278c7b78
Status: ASSIGNED → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
You need to log in before you can comment on or make changes to this bug.