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)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED WORKSFORME

People

(Reporter: bzbarsky, Assigned: mccr8)

References

Details

Attachments

(1 file)

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.
Summary: Fatal GC asserts when using responseType="json" with worker XHR → Fatal CC asserts when using responseType="json" with worker XHR
It looks like mStateData.mResponse is traced but not unlinked. Is that what the assert is about?
Yes, most probably.
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)
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.
Attached file Testcase (deleted) —
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.
> and at most you just have an unlink. have a leak, not an unlink...
Does this mean we have no responseType="json" coverage on workers, btw?
Flags: needinfo?(bent.mozilla)
(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.
Keywords: sec-high
Assignee: nobody → continuation
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
Group: core-security
Flags: needinfo?(bugs)
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.
Is this still an issue?
Does not reproduce with the attached test case.
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.

Attachment

General

Created:
Updated:
Size: