Closed Bug 619004 Opened 13 years ago Closed 13 years ago

Crash [@ JSCompartment::wrap]

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
critical

Tracking

()

RESOLVED FIXED
Tracking Status
blocking2.0 --- betaN+

People

(Reporter: gkw, Assigned: dmandelin)

References

Details

(Keywords: crash, regression, testcase, Whiteboard: [ccbr][sg:critical?][hardblocker][fixed-in-tracemonkey])

Crash Data

Attachments

(3 files)

Attached file stack
gczeal(2)
evalcx('split')

crashes js debug shell on TM changeset 0343557b0c7a on ARM Ubuntu 9.04 without -m or -j at JSCompartment::wrap
Not sure if blocking-fennec is the way to go, please correct if not.
tracking-fennec: --- → ?
I managed to reproduce on x86 Windows, so this isn't ARM (or OS?) specific.
blocking2.0: --- → ?
OS: Linux → All
Hardware: ARM → All
Summary: Crash [@ JSCompartment::wrap] on ARM → Crash [@ JSCompartment::wrap]
Actually, upon further analysis and some IRC chat this seems a lot like a use-after-free error. Valgrind says that the address it tries to access is in a free block, and that doesn't seem like a false positive. In WinDBG when the js shell crashes, the EAX register is 0xDADADADA. Final evidence to support my hypothesis is call to "cmp dword ptr [eax+4ch],0", and the fact that the EIP of the crash is equal to eax+4ch. 

This seems like it should be security sensitive.
blocking2.0: ? → beta9+
Group: core-security
Whiteboard: [ccbr] → [ccbr][sg:critical?]
Attached file Windows stack trace
Notice the eax register
autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   54483:1c913526c597
user:        Gregor Wagner
date:        Fri Sep 24 10:54:39 2010 -0700
summary:     Bug 558861 - Compartmental GC (r=gal)
Blocks: 558861
This bug is most likely related to bug 621375, even if the crash signatures are different because of the splitting and gc().
(In reply to comment #6)
> This bug is most likely related to bug 621375, even if the crash signatures are
> different because of the splitting and gc().

Note that that bug seems to blame a different changeset and seems to require -m, which this one does not.
Whiteboard: [ccbr][sg:critical?] → [ccbr][sg:critical?][hardblocker]
(In reply to comment #7)
> (In reply to comment #6)
> > This bug is most likely related to bug 621375, even if the crash signatures are
> > different because of the splitting and gc().
> 
> Note that that bug seems to blame a different changeset and seems to require
> -m, which this one does not.

When I say related I mean that they are both issues with gc() only freeing up half of the split object.
Assignee: general → dmandelin
tracking-fennec: ? → ---
Affects only shell (it is a rooting/marking bug relating to the shell 'split' objects), so not s-s.
Group: core-security
blocking2.0: beta9+ → .x
Attached patch PatchSplinter Review
Not blocking, but throwing up a patch because it probably makes life harder for fuzzers. 

We crash because GC happens to define the 'lazy' property in NewSandbox and collects the outer object. This is because the return value from split_setup is the inner object, while the mark function marks the inner object iff its argument is the outer object. Thus, the conservative scanner sees only the inner object, which doesn't mark the outer.

Heap-marking outer from inner as well seems the simplest and most reliable solution.
Attachment #501227 - Flags: review?(jorendorff)
hardblocker, so moving to 2.0+
blocking2.0: .x → betaN+
(In reply to comment #12)
> hardblocker, so moving to 2.0+

I forgot to take the 'hardblocker' tag off when I unblocked it: this is a shell-only bug, so I think it should not block.
Attachment #501227 - Flags: review?(jorendorff) → review+
http://hg.mozilla.org/tracemonkey/rev/714d7e569fcd
Status: NEW → ASSIGNED
Whiteboard: [ccbr][sg:critical?][hardblocker] → [ccbr][sg:critical?][hardblocker][fixed-in-tracemonkey]
http://hg.mozilla.org/mozilla-central/rev/714d7e569fcd
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Crash Signature: [@ JSCompartment::wrap]
A testcase for this bug was automatically identified at js/src/jit-test/tests/basic/bug619004.js.
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.