Double free when realloc fails in JSTempVector::GrowTo

RESOLVED FIXED

Status

()

RESOLVED FIXED
10 years ago
6 years ago

People

(Reporter: jruderman, Assigned: luke)

Tracking

(Blocks: 1 bug, {regression, testcase})

Trunk
regression, testcase
Points:
---
Dependency tree / graph
Bug Flags:
wanted1.9.0.x -
in-testsuite -

Firefox Tracking Flags

(status1.9.1 unaffected)

Details

(Whiteboard: [sg:critical?] fixed-in-tracemonkey)

Attachments

(2 attachments)

(Reporter)

Description

10 years ago
Created attachment 388359 [details]
stack traces for realloc failure and second free

var a = [];
var s = "ABCDEFGHIJKLMNOPQRSTUVWXYZ123456";
for (var i = 0; i < 60000000; ++i)
  a.push(s);
"" + a;

takes about 20 seconds with -j, then causes malloc to complain: "pointer being freed was not allocated".

Looks like GrowTo's realloc fails, GrowTo deletes mBegin, and then the destructor deletes mBegin again.  Which should be responsible for deleting mBegin in an OOM situation, GrowTo or the destructor?
Assignee: general → lw
OS: Mac OS X → All
Hardware: x86 → All
(Assignee)

Comment 1

10 years ago
Created attachment 388368 [details] [diff] [review]
remove free on realloc

Oh dear.  I tested for this, but that was before I too-hastily slapped in the POD-handling special case.  Your diagnosis is correct; growTo should not be freeing on realloc failure, since the destructor will do that instead.  Thanks!
Status: NEW → ASSIGNED

Comment 2

10 years ago
Jesse, nice work.
Comment on attachment 388368 [details] [diff] [review]
remove free on realloc

Ugh, how'd I miss this...
Attachment #388368 - Flags: review+
http://hg.mozilla.org/tracemonkey/rev/d1b9ec46733f

with obscured commit message, not that it matters for anyone who ever takes a look at the change itself...
Whiteboard: [sg:critical] fixed-in-tracemonkey

Comment 5

10 years ago
http://hg.mozilla.org/mozilla-central/rev/d1b9ec46733f
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
(Reporter)

Updated

10 years ago
Group: core-security
status1.9.1: --- → unaffected
Flags: wanted1.9.0.x-
Whiteboard: [sg:critical] fixed-in-tracemonkey → [sg:critical?] fixed-in-tracemonkey
Flags: in-testsuite-
You need to log in before you can comment on or make changes to this bug.