Closed
Bug 943444
Opened 11 years ago
Closed 10 years ago
Fatal CC asserts when using responseType="json" with worker XHR
Categories
(Core :: DOM: Workers, defect)
Tracking
()
RESOLVED
WORKSFORME
People
(Reporter: bzbarsky, Assigned: mccr8)
References
Details
Attachments
(1 file)
(deleted),
text/html
|
Details |
Security-sensitive for now.
STEPS TO REPRODUCE:
1) Load attached testcase in a debug build.
2) Hit reload button.
3) If by some chance you don't crash, hit reload button again.
All the testcase does is run this in a worker:
var xhr = new XMLHttpRequest;
xhr.open("GET", "data:text/plain,{}", false);
xhr.responseType = "json";
xhr.send();
what I get is:
Program received signal EXC_BAD_ACCESS, Could not access memory.
Reason: KERN_INVALID_ADDRESS at address: 0x0000000000000000
[Switching to process 28070 thread 0xb917]
AssertNoGcThing (aGCThing=0x14c7400a0, aName=0x107bbe80d "mStateData.mResponse", aClosure=0x0) at CycleCollectedJSRuntime.cpp:833
833 MOZ_ASSERT(!aGCThing);
(gdb) bt
#0 AssertNoGcThing (aGCThing=0x14c7400a0, aName=0x107bbe80d "mStateData.mResponse", aClosure=0x0) at CycleCollectedJSRuntime.cpp:833
#1 0x0000000103b34bbe in TraceCallbackFunc::Trace (this=0x14b1f8958, p=0x115e5db50, name=0x107bbe80d "mStateData.mResponse", closure=0x0) at nsCycleCollectionParticipant.cpp:72
#2 0x00000001058bd4ba in mozilla::dom::workers::XMLHttpRequest::cycleCollection::Trace (this=0x1094e1c90, p=0x115e5daa0, aCallbacks=@0x14b1f8958, aClosure=0x0) at XMLHttpRequest.cpp:1483
#3 0x0000000103b7422b in mozilla::CycleCollectedJSRuntime::AssertNoObjectsToTrace (this=0x14b200b28, aPossibleJSHolder=0x115e5daa0) at CycleCollectedJSRuntime.cpp:841
#4 0x0000000103b7a154 in nsCycleCollector::CollectWhite (this=0x149742000) at nsCycleCollector.cpp:2444
In particular, mStateData.mResponse is an object value in this case.
Reporter | ||
Updated•11 years ago
|
Summary: Fatal GC asserts when using responseType="json" with worker XHR → Fatal CC asserts when using responseType="json" with worker XHR
Reporter | ||
Comment 1•11 years ago
|
||
It looks like mStateData.mResponse is traced but not unlinked. Is that what the assert is about?
Comment 2•11 years ago
|
||
Yes, most probably.
Reporter | ||
Comment 3•11 years ago
|
||
Do we require unlinking all traced things? That's non-obvious to me, and probably others...
Is this a security issue, or can we just open it up?
Flags: needinfo?(bugs)
Assignee | ||
Comment 4•11 years ago
|
||
If a traced pointer isn't cleared in Unlink, and we have a bug somewhere else that makes an object reachable from content after it is unlinked (we've had a ton of these), then you have a use-after-free bug.
Reporter | ||
Comment 5•11 years ago
|
||
Assignee | ||
Comment 6•11 years ago
|
||
Well, potentially. That's assuming that we stop actually tracing the object after the unlink. If you do a DROP in the destructor, then you should still be okay, and at most you just have an unlink.
Assignee | ||
Comment 7•11 years ago
|
||
> and at most you just have an unlink.
have a leak, not an unlink...
Reporter | ||
Comment 8•11 years ago
|
||
Does this mean we have no responseType="json" coverage on workers, btw?
Reporter | ||
Updated•11 years ago
|
Flags: needinfo?(bent.mozilla)
Blocks: 928312
(In reply to Boris Zbarsky [:bz] from comment #8)
> Does this mean we have no responseType="json" coverage on workers, btw?
Probably.
Flags: needinfo?(bent.mozilla)
Happy to review a patch here if somebody knows what needs to be done.
Assignee | ||
Updated•11 years ago
|
Assignee: nobody → continuation
Assignee | ||
Comment 11•11 years ago
|
||
I think this isn't actually a security problem. The DropJSObjects call is only in the destructor, so we'll continue tracing this object pointer even after the object is unlinked. I'll unhide it in a few days if nobody objects.
Keywords: sec-high
Assignee | ||
Updated•11 years ago
|
Group: core-security
Flags: needinfo?(bugs)
Assignee | ||
Comment 12•11 years ago
|
||
I wrote a patch that does the simple thing of setting mStateData.mResponse to undefined in Unlink, but the test I added, which is just bz's test case, fails with this about 80% of the time:
Assertion failure: mRooted (Mismatched calls to Unpin!), at ../../../dom/workers/XMLHttpRequest.cpp:1757
TEST-UNEXPECTED-FAIL | /tests/dom/workers/test/test_bug943444.html | application terminated with exit code 11
PROCESS-CRASH | /tests/dom/workers/test/test_bug943444.html | application crashed [@ mozilla::dom::workers::XMLHttpRequest::Unpin()]
Return code: 1
I'll try to figure out what is supposed to be happening here. My theory would be that something is keying off the value of request somehow, but I don't really understand the behavior of all these interlocking unpinning runnables.
Comment 13•10 years ago
|
||
Is this still an issue?
Comment 14•10 years ago
|
||
Does not reproduce with the attached test case.
Assignee | ||
Updated•10 years ago
|
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → WORKSFORME
You need to log in
before you can comment on or make changes to this bug.
Description
•