Issue metadata
Sign in to add a comment
|
Crash in blink::FrameHost::globalRootScrollerController |
||||||||||||||||||||||
Issue descriptionDetailed report: https://cluster-fuzz.appspot.com/testcase?key=6574464155516928 Fuzzer: ifratric-browserfuzzer-v3 Job Type: mac_asan_chrome Platform Id: mac Crash Type: UNKNOWN READ Crash Address: 0x000000000038 Crash State: blink::FrameHost::globalRootScrollerController blink::FrameView::viewportSizeChanged blink::FrameView::setFrameRect Regressed: https://cluster-fuzz.appspot.com/revisions?job=mac_asan_chrome&range=434441:434442 Minimized Testcase (3.83 Kb): https://cluster-fuzz.appspot.com/download/AMIfv954cIK0p_enzSAZ7Ch42Ai-t_HdUCFWzF_fTTw9AQu4tIE6mA5c7wW5I221OYSC31kcy1UAyFvjvIisKUE8PHhJGR8EzkZhStlt8b5ZqbtELs-UdF_LuTjlEle6gAkA4d0o-68Su6l4SdQfbNUC3cn2z-lqHA?testcase_id=6574464155516928 Issue filed automatically. See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information.
,
Dec 6 2016
I don't completely understand the minimized test case but here's what I've gathered: 1. Calling body.innerText = "" causes us to remove all children which detaches the <iframe> and <embed> in the first part of WillRemoveChildren 2. Before we actually remove the children from the DOM we stop loading on the frame 3. loadFailed() leads to checkCompleted() which then calls checkCompleted() on the parent. 4. I'm guessing the parent has finished loading by now but hasn't had a chance to finalize yet. This causes implicitClose() on the parent which dispatches the load event. 5. This calls into the load handler which touches an Object's getter, causing the document to flush pending post layout tasks which updates the widgets on the page. 6. This calls updateWidget() on the detached <embed> which goes on to setFrameRect on a detached FrameView. Here's the relevant stack traces: [1] The detaching of frames: #1 0x7fdab4b0ccdb blink::LocalDOMWindow::dispatchWindowLoadEvent() #2 0x7fdab4b0d5b9 blink::LocalDOMWindow::documentWasClosed() #3 0x7fdab46a4b94 blink::Document::implicitClose() #4 0x7fdab524fc82 blink::FrameLoader::checkCompleted() #5 0x7fdab524feae blink::FrameLoader::checkCompleted() #6 0x7fdab5252f61 blink::FrameLoader::loadFailed() #7 0x7fdab524dc94 blink::FrameLoader::stopAllLoaders() #8 0x7fdab4b24326 blink::LocalFrame::detach() #9 0x7fdab4beaa13 blink::HTMLFrameOwnerElement::disconnectContentFrame() #10 0x7fdab46491b8 blink::ChildFrameDisconnector::disconnectCollectedFrameOwners() #11 0x7fdab4648cf7 blink::ChildFrameDisconnector::disconnect() #12 0x7fdab465ec88 blink::ContainerNode::willRemoveChildren() #13 0x7fdab465f27e blink::ContainerNode::removeChildren() #14 0x7fdab4bcaf3e blink::HTMLElement::setInnerText() #15 0x7fdab5839f8a blink::HTMLElementV8Internal::innerTextAttributeSetter() #16 0x7fdab5839e5f blink::HTMLElementV8Internal::innerTextAttributeSetterCallback() [2] The load handler on <body> leading to the crash: #4 0x7fdab4aba4e9 blink::FrameHost::globalRootScrollerController() #5 0x7fdab4ac9198 blink::FrameView::viewportSizeChanged() #6 0x7fdab4ac8965 blink::FrameView::setFrameRect() #7 0x7fdab50b44eb blink::LayoutPart::updateWidgetGeometryInternal() #8 0x7fdab50b4275 blink::LayoutPart::updateOnWidgetChange() #9 0x7fdab4bebf4d blink::LayoutPartItem::updateOnWidgetChange() #10 0x7fdab4beb1b4 blink::HTMLFrameOwnerElement::setWidget() #11 0x7fdab4c5ad44 blink::HTMLPlugInElement::loadPlugin() #12 0x7fdab4c5a693 blink::HTMLPlugInElement::requestObjectInternal() #13 0x7fdab4c5ca95 blink::HTMLPlugInElement::requestObject() #14 0x7fdab4bd1204 blink::HTMLEmbedElement::updateWidgetInternal() #15 0x7fdab4c5b92d blink::HTMLPlugInElement::updateWidget() #16 0x7fdab4ad2dca blink::FrameView::updateWidgets() #17 0x7fdab4ac616c blink::FrameView::updateWidgetsTimerFired() #18 0x7fdab4ad3027 blink::FrameView::flushAnyPendingPostLayoutTasks() #19 0x7fdab46a18d3 blink::Document::updateStyleAndLayoutIgnorePendingStylesheets() #20 0x7fdab4c5c71c blink::HTMLPlugInElement::layoutPartForJSBindings() #21 0x7fdab4c5b079 blink::HTMLPlugInElement::pluginWidget() #22 0x7fdab4c5c205 blink::HTMLPlugInElement::pluginWrapper() #23 0x7fdab4176d85 blink::(anonymous namespace)::getScriptableObjectProperty<>() #24 0x7fdab4176cdd blink::V8HTMLObjectElement::namedPropertyGetterCustom() #25 0x7fdab588eb2d blink::HTMLObjectElementV8Internal::namedPropertyGetterCallback()
,
Dec 6 2016
And here's the most minimized I could get the test case:
<script>
var fakestring = {toString: function() {
return "1" }}
function jsfuzzer() {
console.error("jsfuzzer");
/* HTMLObjectElement*/ var var00103 = document.createElement("object");
var00103.setCustomValidity();
}
function eventhandler1() {
console.error("handler1");
/* HTMLVideoElement*/ var var00024 = document.createElement("video");
var00024.poster = "" + String.fromCharCode( 79) + "";
htmlvar00011.align = "right";
setTimeout(eventhandler3, 0);
}
function eventhandler3() {
console.error("handler3");
htmlvar00011.height = "28";
htmlvar00023.srcdoc = "data:text/html,foo";
setTimeout(eventhandler4, 0);
}
function eventhandler4() {
console.error("handler4");
htmlvar00011.type = "JavaScript 1.1";
htmlvar00005.innerText = String.fromCharCode();
console.error("handler4DONE");
}
</script>
<body onload=jsfuzzer() id="htmlvar00005">
<embed id="htmlvar00011" onload="eventhandler1()" src="x">
<iframe id="htmlvar00023"></iframe>
<table id="htmlvar00043" </table>
</body>
,
Dec 6 2016
+dcheng@, is it expected that we can call into FrameView for detached Widgets? I added some code that assumed we have a frameHost in viewportSizeChanged. I can easily guard against that but just want to make sure I'm not papering over a problem higher up.
,
Dec 6 2016
Repros on Linux and doesn't seem OS dependent.
,
Dec 6 2016
Well, ideally, we shouldn't, but I guess that's not always true in practice... I think it's OK to guard here, if we have a test for it with a comment describing why we need to guard against this case I guess...
,
Dec 6 2016
Ok, thanks.
,
Dec 12 2016
Just to update, this is #5 renderer crash on the latest Dev(57.0.2946.0 - 9 crashes from 9 clients) of Mac.
,
Dec 12 2016
Users experienced this crash on the following builds: Mac Dev 57.0.2946.0 - 1.24 CPM, 4 reports, 4 clients (signature blink::FrameHost::globalRootScrollerController) If this update was incorrect, please add "Fracas-Wrong" label to prevent future updates. - Go/Fracas
,
Dec 14 2016
Friendly ping to get an update on this issue. Link to the list of the builds with crashes on Mac as seen on the crash server: =============================================================================== https://crash.corp.google.com/browse?q=custom_data.ChromeCrashProto.magic_signature_1.name%3D%27blink%3A%3AFrameHost%3A%3AglobalRootScrollerController%27%20AND%20product.name%3D%27Chrome_Mac%27&ignore_case=false&enable_rewrite=true&omit_field_name=&omit_field_value=&omit_field_opt=%3D#samplereports:5,productversion:1000
,
Dec 14 2016
Still waiting on review.
,
Dec 16 2016
More widget fun. I've been testing with bokan's test case in https://codereview.chromium.org/2560803002/. clearBody() looks like this: function clearBody() { embed.align = "right"; embed.height = "28"; embed.type = "foo"; // This will cause the iframe and embeds to detach and in doing so, // reevaluate whether the parent document has finished loading, and since // it has, call its load handler. body.innerText = ""; } Before clearBody() runs, the <embed> element has a content frame (0x15813d65e4d0) with a FrameView (0x272c309e47d0) Then the embed's onload is called. When we set the height property, we detach the layout tree (why?): [1:1:1216/112618.006080:152695227838:ERROR:V8HTMLPlugInElementCustom.cpp(126)] Setting property align for plugin. [1:1:1216/112618.006237:152695227967:ERROR:V8HTMLPlugInElementCustom.cpp(126)] Setting property height for plugin. [1:1:1216/112618.006441:152695228172:ERROR:HTMLPlugInElement.cpp(101)] ===== Maybe persisting a plugin widget... [1:1:1216/112618.006503:152695228229:ERROR:HTMLPlugInElement.cpp(114)] Persisting 0x272c309e47d0 #0 0x7f989d38ec3e base::debug::StackTrace::StackTrace() #1 0x7f98989f831e blink::HTMLPlugInElement::setPersistedPluginWidget() #2 0x7f98989f9aa0 blink::HTMLPlugInElement::detachLayoutTree() #3 0x7f98986acfc0 blink::Node::reattachLayoutTree() #4 0x7f9898662e64 blink::Element::rebuildLayoutTree() #5 0x7f98986625cf blink::Element::recalcOwnStyle() #6 0x7f9898662030 blink::Element::recalcStyle() #7 0x7f98985e928a blink::ContainerNode::recalcDescendantStyles() #8 0x7f98986620f2 blink::Element::recalcStyle() #9 0x7f98985e928a blink::ContainerNode::recalcDescendantStyles() #10 0x7f98986620f2 blink::Element::recalcStyle() #11 0x7f989861837b blink::Document::updateStyle() #12 0x7f98986140a1 blink::Document::updateStyleAndLayoutTree() #13 0x7f9898619965 blink::Document::updateStyleAndLayoutTreeIgnorePendingStylesheets() #14 0x7f989861935d blink::Document::updateStyleAndLayoutIgnorePendingStylesheets() #15 0x7f98989fa186 blink::HTMLPlugInElement::layoutPartForJSBindings() #16 0x7f98989f9e1b blink::HTMLPlugInElement::pluginWrapper() #17 0x7f98982d3397 blink::V8HTMLEmbedElement::namedPropertySetterCustom() #18 0x7f989917c2b3 blink::HTMLEmbedElementV8Internal::namedPropertySetterCallback() #19 0x7f989b063973 v8::internal::PropertyCallbackArguments::Call() #20 0x7f989b0f4054 v8::internal::(anonymous namespace)::SetPropertyWithInterceptorInternal() #21 0x7f989b10a7d2 v8::internal::Object::SetPropertyInternal() #22 0x7f989b10a554 v8::internal::Object::SetProperty() #23 0x7f989b054135 v8::internal::StoreIC::Store() #24 0x7f989b05be3f v8::internal::__RT_impl_Runtime_StoreIC_Miss() #25 0x2a23fb00426e <unknown> So now the <embed> element's m_persistedPluginWidget points to FrameView 0x272c309e47d0. Eventually, execution reaches body.innerHTML = "". This detaches the embed's content frame: [1:1:1216/112618.057959:152695279731:ERROR:HTMLFrameOwnerElement.cpp(178)] ===== Disconnecting content frame for EMBED id="embed" [1:1:1216/112618.058070:152695279800:ERROR:HTMLFrameOwnerElement.cpp(184)] Actually something to do for 0x15813d65e4d0 [1:1:1216/112618.058129:152695279855:ERROR:LocalFrame.cpp(389)] Detaching local frame 0x15813d65e4d0 [1:1:1216/112618.058303:152695280051:ERROR:HTMLFrameOwnerElement.cpp(246)] ====== setWidget on EMBED id="embed" ======= [1:1:1216/112618.058389:152695280116:ERROR:HTMLFrameOwnerElement.cpp(247)] I haz a LayoutObject: 0x48d4228010 [1:1:1216/112618.058442:152695280169:ERROR:HTMLFrameOwnerElement.cpp(248)] My document is #document [1:1:1216/112618.058499:152695280225:ERROR:HTMLFrameOwnerElement.cpp(249)] My frame is 0x15813d641ff8 [1:1:1216/112618.058551:152695280275:ERROR:HTMLFrameOwnerElement.cpp(251)] And my frame host is 0x15813d641bd8 [1:1:1216/112618.059475:152695281205:ERROR:LocalFrame.cpp(455)] Detached local frame 0x15813d65e4d0 (Note: the widget here is already null, so nothing interesting happens) While detaching the <iframe>, loading completes, calling bodyLoaded(), which looks like this: function bodyLoaded() { // This will run post layout tasks synchronously from // HTMLPluginElement::layoutPartForJSBindings() which will cause us to load // the detached <embed> element. This test passes if it doesn't crash. var element = document.createElement("object"); element.setCustomValidity(""); // Use testharness.js to prevent an -expected file. test(function () {}, "Make sure we didn't crash"); } And accessing setCustomValidity triggers a plugin reattach: [1:1:1216/112618.061543:152695283276:ERROR:V8HTMLPlugInElementCustom.cpp(118)] Getting property setCustomValidity for plugin. [1:1:1216/112618.062599:152695284331:ERROR:HTMLPlugInElement.cpp(525)] Loading the persisted plugin widget. [1:1:1216/112618.062665:152695284400:ERROR:HTMLFrameOwnerElement.cpp(246)] ====== setWidget on EMBED id="embed" ======= [1:1:1216/112618.062735:152695284462:ERROR:HTMLFrameOwnerElement.cpp(247)] I haz a LayoutObject: 0x48d4228010 [1:1:1216/112618.062789:152695284513:ERROR:HTMLFrameOwnerElement.cpp(248)] My document is #document [1:1:1216/112618.062843:152695284567:ERROR:HTMLFrameOwnerElement.cpp(249)] My frame is 0x15813d641ff8 [1:1:1216/112618.062893:152695284616:ERROR:HTMLFrameOwnerElement.cpp(251)] And my frame host is 0x15813d641bd8 [1:1:1216/112618.062942:152695284665:ERROR:HTMLFrameOwnerElement.cpp(256)] new widget is 0x272c309e47d0 [1:1:1216/112618.062991:152695284713:ERROR:HTMLFrameOwnerElement.cpp(273)] frame view's frame: 0x15813d65e4d0 [1:1:1216/112618.063039:152695284761:ERROR:HTMLFrameOwnerElement.cpp(274)] And the frame view is 0x272c309e47d0 [1:1:1216/112618.063098:152695284822:ERROR:FrameView.cpp(1546)] ==== Changing viewport size for frame 0x15813d65e4d0 with view 0x272c309e47d0 [1:1:1216/112618.063157:152695284880:ERROR:FrameView.cpp(1548)] Main frame? 1 [1:1:1216/112618.063207:152695284929:ERROR:FrameView.cpp(1549)] I haz a host? 0 [1:1:1216/112618.063255:152695284977:ERROR:FrameView.cpp(1551)] I haz a framehost? 0 And here we crash, because FrameView 0x272c309e47d0 is associated with Frame 0x15813d65e4d0, which is already detached. Received signal 11 SEGV_MAPERR 000000000068 #0 0x7f989d38e7d4 base::debug::(anonymous namespace)::StackDumpSignalHandler() #1 0x7f989f0c6330 <unknown> #2 0x7f98984b6b70 <unknown> #3 0x7f98988cf1dd blink::FrameView::viewportSizeChanged() #4 0x7f98988ce91f blink::FrameView::setFrameRect() #5 0x7f9898cc6df2 blink::LayoutPart::updateWidgetGeometryInternal() #6 0x7f9898cc6a2b blink::LayoutPart::updateOnWidgetChange() #7 0x7f98989a3a09 blink::HTMLFrameOwnerElement::setWidget() #8 0x7f98989f8b6e blink::HTMLPlugInElement::loadPlugin() #9 0x7f98989f849b blink::HTMLPlugInElement::requestObjectInternal() #10 0x7f98989fa3f8 blink::HTMLPlugInElement::requestObject() #11 0x7f9898990b77 blink::HTMLEmbedElement::updateWidgetInternal() #12 0x7f98989f95ad blink::HTMLPlugInElement::updateWidget() #13 0x7f98988d9a00 blink::FrameView::updateWidgets() #14 0x7f98988d9c28 blink::FrameView::flushAnyPendingPostLayoutTasks() #15 0x7f98989fa186 blink::HTMLPlugInElement::layoutPartForJSBindings() #16 0x7f98989f9e1b blink::HTMLPlugInElement::pluginWrapper() #17 0x7f98982d305e blink::V8HTMLObjectElement::namedPropertyGetterCustom() #18 0x7f9899196d66 blink::HTMLObjectElementV8Internal::namedPropertyGetterCallback() #19 0x7f989b0634e2 v8::internal::PropertyCallbackArguments::Call() #20 0x7f989b0f3427 v8::internal::(anonymous namespace)::GetPropertyWithInterceptorInternal() #21 0x7f989b0ef245 v8::internal::Object::GetProperty() #22 0x7f989b04ca69 v8::internal::LoadIC::Load() #23 0x7f989b05912c v8::internal::__RT_impl_Runtime_LoadIC_Miss() #24 0x2a23fb00426e <unknown> There seem to be at least two problems here: - Setting a detached FrameView as the widget is undesirable. Perhaps FrameView::dispose() should clear itself from m_persistedPluginWidget. It's unclear to me if we're allowed to reuse a widget that's disposed, since this triggers a bunch of one-way transformations. - Triggering loading of a <embed> element also seems buggy: we're about to remove this thing from the DOM. Crazy idea: FrameView::dispose clears the widget currently. Perhaps it should also clear persisted plugin widgets? Crazy idea #2: Perhaps plugin loads should also respect SubframeLoadDisabler?
,
Dec 17 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9485e0d3ee1ec3d8bd5578b7ed4493855cfc9898 commit 9485e0d3ee1ec3d8bd5578b7ed4493855cfc9898 Author: dcheng <dcheng@chromium.org> Date: Sat Dec 17 12:35:05 2016 Dispose a persisted plugin widget if the content frame is detached. HTMLPlugInElement::removedFrom() clears persisted plugin widgets, to prevent reuse of a widget associated with a destroyed object. However, when nodes containing connected subframes are removed from a Document, it is possible to synchronously trigger script after subframes are detached, but before the nodes themselves are actually removed. This can cause a FrameView for a detached frame to be incorrectly reused. BUG= 671331 Review-Url: https://codereview.chromium.org/2586633002 Cr-Commit-Position: refs/heads/master@{#439339} [add] https://crrev.com/9485e0d3ee1ec3d8bd5578b7ed4493855cfc9898/third_party/WebKit/LayoutTests/fast/plugins/update-plugin-after-detachment-crash.html [modify] https://crrev.com/9485e0d3ee1ec3d8bd5578b7ed4493855cfc9898/third_party/WebKit/Source/core/html/HTMLFrameOwnerElement.h [modify] https://crrev.com/9485e0d3ee1ec3d8bd5578b7ed4493855cfc9898/third_party/WebKit/Source/core/html/HTMLPlugInElement.cpp [modify] https://crrev.com/9485e0d3ee1ec3d8bd5578b7ed4493855cfc9898/third_party/WebKit/Source/core/html/HTMLPlugInElement.h
,
Dec 21 2016
,
Dec 21 2016
This should be fixed by the patch in #13 but Clusterfuzz is rerunning the test case with a rather old revision. I'll wait until Clusterfuzz confirms the fix before closing.
,
Dec 23 2016
Fuzzer seems stuck at an obsolete revision, I've filed a bug for that (issue 676841). I'm going to mark this as fixed since I can't repro locally.
,
Dec 23 2016
,
Dec 24 2016
ClusterFuzz has detected this issue as fixed in range 438498:440663. Detailed report: https://cluster-fuzz.appspot.com/testcase?key=6574464155516928 Fuzzer: ifratric-browserfuzzer-v3 Job Type: mac_asan_chrome Platform Id: mac Crash Type: UNKNOWN READ Crash Address: 0x000000000038 Crash State: blink::FrameHost::globalRootScrollerController blink::FrameView::viewportSizeChanged blink::FrameView::setFrameRect Regressed: https://cluster-fuzz.appspot.com/revisions?job=mac_asan_chrome&range=434441:434442 Fixed: https://cluster-fuzz.appspot.com/revisions?job=mac_asan_chrome&range=438498:440663 Minimized Testcase (3.83 Kb): https://cluster-fuzz.appspot.com/download/AMIfv954cIK0p_enzSAZ7Ch42Ai-t_HdUCFWzF_fTTw9AQu4tIE6mA5c7wW5I221OYSC31kcy1UAyFvjvIisKUE8PHhJGR8EzkZhStlt8b5ZqbtELs-UdF_LuTjlEle6gAkA4d0o-68Su6l4SdQfbNUC3cn2z-lqHA?testcase_id=6574464155516928 See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information. If you suspect that the result above is incorrect, try re-doing that job on the test case report page.
,
Jan 5 2017
|
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by dtapu...@chromium.org
, Dec 5 2016Owner: bokan@chromium.org
Status: Assigned (was: Untriaged)