Closed
Bug 1002702
Opened 10 years ago
Closed 10 years ago
"Assertion failure: !mWorkerPrivate" with port.close(), GC
Categories
(Core :: DOM: Workers, defect)
Tracking
()
VERIFIED
FIXED
mozilla33
Tracking | Status | |
---|---|---|
firefox30 | --- | wontfix |
firefox31 | --- | verified |
firefox32 | --- | verified |
firefox33 | --- | verified |
firefox-esr24 | --- | unaffected |
b2g-v1.3 | --- | unaffected |
b2g-v1.3T | --- | unaffected |
b2g-v1.4 | --- | fixed |
b2g-v2.0 | --- | fixed |
b2g-v2.1 | --- | fixed |
People
(Reporter: jruderman, Assigned: baku)
Details
(Keywords: assertion, sec-high, testcase, Whiteboard: [adv-main31+])
Attachments
(3 files, 3 obsolete files)
264 bytes,
text/html
|
Details | |
4.53 KB,
text/plain
|
Details | |
2.35 KB,
patch
|
khuey
:
review+
Sylvestre
:
approval-mozilla-aurora+
Sylvestre
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
1. Load the testcase 2. Use about:memory to force a GC OR 1. Install https://www.squarefree.com/extensions/domFuzzLite3.xpi 2. Load the testcase Assertion failure: !mWorkerPrivate, at dom/workers/SharedWorker.cpp:43 Security-sensitive because (1) it's similar to bug 998474 and (2) it involves GC.
Reporter | ||
Comment 1•10 years ago
|
||
Comment 2•10 years ago
|
||
It looks like this assertion means we've failed to run some kind of cleanup code, which sounds bad, so I'm going to set this to sec-high. Please adjust as appropriate.
Keywords: sec-high
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → amarchesini
Flags: needinfo?(amarchesini)
Assignee | ||
Comment 4•10 years ago
|
||
The reason of this crash is because we call "Close()" only when we Unlink is running becaue of GC. This is not enough for this test-case. This patch forces Close() in the dtor of SharedWorker.
Attachment #8424930 -
Flags: review?(nsm.nikhil)
Comment on attachment 8424930 [details] [diff] [review] sw.patch It's probably better to have khuey sanity-check this.
Attachment #8424930 -
Flags: review?(nsm.nikhil) → review?(khuey)
Assignee | ||
Comment 6•10 years ago
|
||
Mochitest added
Attachment #8424930 -
Attachment is obsolete: true
Attachment #8424930 -
Flags: review?(khuey)
Attachment #8424981 -
Flags: review?(khuey)
Updated•10 years ago
|
Group: dom-core-security
Comment on attachment 8424981 [details] [diff] [review] sw.patch Review of attachment 8424981 [details] [diff] [review]: ----------------------------------------------------------------- You haven't actually run the test ...
Attachment #8424981 -
Flags: review?(khuey) → review-
Assignee | ||
Comment 8•10 years ago
|
||
0:05.59 TEST-PASS | unknown test url | No crash \o/ I don't know why I lost an 's' in SpecialPowers saving the test file. Can you review it again, please?
Attachment #8424981 -
Attachment is obsolete: true
Attachment #8427920 -
Flags: review?(khuey)
Assignee | ||
Comment 9•10 years ago
|
||
Still the 's' was missing :)
Attachment #8427920 -
Attachment is obsolete: true
Attachment #8427920 -
Flags: review?(khuey)
Attachment #8427965 -
Flags: review?(khuey)
Comment 10•10 years ago
|
||
Kyle, can you review this patch, or should somebody else? It has been sitting here for over a month.
Flags: needinfo?(khuey)
Attachment #8427965 -
Flags: review?(khuey) → review+
Flags: needinfo?(khuey)
This is a bit weird, it means that error events stop being delivered to the SharedWorker once close() is called on the port object... Since there's no other way to disconnect the SharedWorker, and this is just error events, I guess this is ok for now.
Assignee | ||
Comment 12•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/9dafb9b56b5c
Comment 13•10 years ago
|
||
This appears to affect more than trunk, so it should have gotten sec-approval, though it is too late now. What versions does this affect?
Updated•10 years ago
|
Flags: needinfo?(amarchesini)
Assignee | ||
Comment 14•10 years ago
|
||
(In reply to Andrew McCreight [:mccr8] from comment #13) > This appears to affect more than trunk, so it should have gotten > sec-approval, though it is too late now. What versions does this affect? Sorry, I completely forgot the set-approval. I would say: ff 29 till now.
Flags: needinfo?(amarchesini)
Updated•10 years ago
|
status-firefox30:
--- → wontfix
status-firefox31:
--- → affected
status-firefox32:
--- → affected
status-firefox33:
--- → affected
status-firefox-esr24:
--- → unaffected
https://hg.mozilla.org/mozilla-central/rev/9dafb9b56b5c
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
Not only did this land without sec-approval, it landed with a test too.
Updated•10 years ago
|
Group: dom-core-security
Comment 17•10 years ago
|
||
Please try to get beta and aurora approval for this. We're almost at the final beta for this cycle.
Flags: needinfo?(amarchesini)
Comment on attachment 8427965 [details] [diff] [review] sw.patch Andrea is on PTO. Approval Request Comment [Feature/regressing bug #]: bug 924089 [User impact if declined]: It's probably a use-after-free opportunity, so crash or exploit is possible [Describe test coverage new/current, TBPL]: There's a test in the tree already... [Risks and why]: Should be low risk, this just removes a dangling pointer from a hashtable. [String/UUID change made/needed]: None
Attachment #8427965 -
Flags: approval-mozilla-beta?
Attachment #8427965 -
Flags: approval-mozilla-aurora?
Flags: needinfo?(amarchesini)
Updated•10 years ago
|
Attachment #8427965 -
Flags: approval-mozilla-beta?
Attachment #8427965 -
Flags: approval-mozilla-beta+
Attachment #8427965 -
Flags: approval-mozilla-aurora?
Attachment #8427965 -
Flags: approval-mozilla-aurora+
Comment 19•10 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/2bbc56ea0d3d https://hg.mozilla.org/releases/mozilla-beta/rev/cc906edb11c0 https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/32d95dfd0a3b
status-b2g-v1.3:
--- → unaffected
status-b2g-v1.3T:
--- → unaffected
status-b2g-v1.4:
--- → fixed
status-b2g-v2.0:
--- → fixed
status-b2g-v2.1:
--- → fixed
Flags: in-testsuite+
Comment 20•10 years ago
|
||
Reproduced the original issue on OSX 10.9.4, Ubuntu 13.10 and Win 8.1 using the STR from comment #0. Used build: * http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014/04/2014-04-28-03-02-03-mozilla-central/ Went through verification using the following builds: * fx-33.0a1 (Nightly) ** OSX 10.9.4 x64: [Passed] ** Ubuntu 13.10 x64: [Passed] ** Win 8.1 x64: [Passed] ** Build: http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014-07-08-03-02-03-mozilla-central/ fx-32.0a2 (Aurora) ** OSX 10.9.4 x64: [Passed] ** Ubuntu 13.10 x64: [Passed] ** Win 8.1 x64: [Passed] ** Build: http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014-07-08-00-40-01-mozilla-aurora/ * fx-31.0b8 (BETA) ** OSX 10.9.4 x64: [Passed] ** Ubuntu 13.10 x64: [Passed] ** Win 8.1 x64: [Passed] ** Build: http://ftp.mozilla.org/pub/mozilla.org/firefox/releases/31.0b8/ - opened the poc and waited a few minutes and ensured that the browser wasn't crashing - opened the poc using multiple tabs and windows and ensured that the browser wasn't crashing - opened several tabs and windows and invoked the GC through about:memory, ensured that the browser wasn't crashing
Status: RESOLVED → VERIFIED
Updated•10 years ago
|
Whiteboard: [adv-main31+]
Updated•9 years ago
|
Group: core-security → core-security-release
Updated•8 years ago
|
Group: core-security-release
You need to log in
before you can comment on or make changes to this bug.
Description
•