Crash [@ JSCompartment::wrap]

RESOLVED FIXED

Status

()

--
critical
RESOLVED FIXED
8 years ago
6 years ago

People

(Reporter: gkw, Assigned: dmandelin)

Tracking

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

Trunk
crash, regression, testcase
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(blocking2.0 betaN+)

Details

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

Attachments

(3 attachments)

(Reporter)

Description

8 years ago
Created attachment 497429 [details]
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
(Reporter)

Comment 1

8 years ago
Not sure if blocking-fennec is the way to go, please correct if not.
tracking-fennec: --- → ?

Comment 2

8 years ago
I managed to reproduce on x86 Windows, so this isn't ARM (or OS?) specific.
(Reporter)

Updated

8 years ago
blocking2.0: --- → ?
OS: Linux → All
Hardware: ARM → All
Summary: Crash [@ JSCompartment::wrap] on ARM → Crash [@ JSCompartment::wrap]

Comment 3

8 years ago
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.

Updated

8 years ago
blocking2.0: ? → beta9+

Updated

8 years ago
Group: core-security

Updated

8 years ago
Whiteboard: [ccbr] → [ccbr][sg:critical?]

Comment 4

8 years ago
Created attachment 497439 [details]
Windows stack trace

Notice the eax register
(Reporter)

Comment 5

8 years ago
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

Comment 6

8 years ago
This bug is most likely related to bug 621375, even if the crash signatures are different because of the splitting and gc().
(Reporter)

Comment 7

8 years ago
(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.
(Assignee)

Updated

8 years ago
Whiteboard: [ccbr][sg:critical?] → [ccbr][sg:critical?][hardblocker]

Comment 8

8 years ago
(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)

Updated

8 years ago
Duplicate of this bug: 621375
(Assignee)

Updated

8 years ago
Assignee: general → dmandelin
tracking-fennec: ? → ---
(Assignee)

Comment 10

8 years ago
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
(Assignee)

Comment 11

8 years ago
Created attachment 501227 [details] [diff] [review]
Patch

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)

Comment 12

8 years ago
hardblocker, so moving to 2.0+
blocking2.0: .x → betaN+
(Assignee)

Comment 13

8 years ago
(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+
(Assignee)

Comment 14

8 years ago
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
Last Resolved: 8 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.