Null-dereference READ in blink::Canvas2DLayerBridge::Hibernate |
|||||||||
Issue descriptionDetailed report: https://clusterfuzz.com/testcase?key=4542402992537600 Fuzzer: inferno_twister_c Job Type: linux_asan_chrome_mp Platform Id: linux Crash Type: Null-dereference READ Crash Address: 0x000000000000 Crash State: blink::Canvas2DLayerBridge::Hibernate base::internal::Invoker<base::internal::BindState<void blink::scheduler::MainThreadSchedulerImpl::RunIdleTask Sanitizer: address (ASAN) Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=4542402992537600 Issue filed automatically. See https://github.com/google/clusterfuzz-tools for more information.
,
Aug 7
I don't see how my patch changing the type of an unused parameter could have caused this. Not able to reproduce locally, and not seeing any other likely culprits, bouncing to canvas team for triage.
,
Aug 8
Unable to find actual suspect through code search and also observing no CL's under regression range, hence adding appropriate label and requesting someone from dev team to look in to this issue.
,
Aug 13
fserb@, this has passed our 7-day triage window. Could you please look at it an re-triage?
,
Aug 14
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b34b7be96c8e4026ab5f17598b4c0682b70b532f commit b34b7be96c8e4026ab5f17598b4c0682b70b532f Author: Miguel Casas <mcasas@chromium.org> Date: Tue Aug 14 14:47:21 2018 lowLatency canvas2D: fix hibernation tab crash lowLatency 2D contexts would crash the tab when sent to the background; this is because of the callstack HTMLCanvasElement::PageVisibilityChanged() CanvasRenderingContext2D::SetIsHidden() canvas()->GetCanvas2DLayerBridge()->SetIsHidden(hidden) ... Canvas2DLayerBridge::Hibernate() and that tries to destroy its Layer, which has not been created (because in lowLatency mode we don't use it). This CL fixes the issue by cutting at SetIsHidden() if there is no |layer_|. Test: open https://codepen.io/miguelao/full/ZjJNNw (or any other trivial sample canvas lowLatency demo), draw something , then open a new tab. The former tab crashes on ToT, doesn't with this CL. Bug: 839970, 870873 Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel Change-Id: I6657a72009438cb31fac2b2d874c807b1719d16a Reviewed-on: https://chromium-review.googlesource.com/1173344 Reviewed-by: Fernando Serboncini <fserb@chromium.org> Commit-Queue: Miguel Casas <mcasas@chromium.org> Cr-Commit-Position: refs/heads/master@{#582913} [modify] https://crrev.com/b34b7be96c8e4026ab5f17598b4c0682b70b532f/third_party/blink/renderer/modules/canvas/canvas2d/canvas_rendering_context_2d.cc [modify] https://crrev.com/b34b7be96c8e4026ab5f17598b4c0682b70b532f/third_party/blink/renderer/platform/graphics/canvas_2d_layer_bridge.cc
,
Aug 21
,
Aug 21
This bug requires manual review: We are only 13 days from stable. Please contact the milestone owner if you have questions. Owners: amineer@(Android), kariahda@(iOS), cindyb@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Aug 21
How safe is the change to merge to M69? And why it is critical to merge to M69 this late in release cycle?
,
Aug 21
it's super safe (just a null check before using a variable). We got a clusterfuzz crash on this as well as a ChromeOS Hibernation crash. I think we should do it.
,
Aug 21
Rejecting merge to M69 as it is not must merge per chat with fserb@.
,
Aug 21
Ok. We are dropping this for M-69, but it's fixed on M70.
,
Aug 28
ClusterFuzz testcase 4542402992537600 is still reproducing on tip-of-tree build (trunk). Please re-test your fix against this testcase and if the fix was incorrect or incomplete, please re-open the bug. Otherwise, ignore this notification and add ClusterFuzz-Wrong label. |
|||||||||
►
Sign in to add a comment |
|||||||||
Comment 1 by kkaluri@chromium.org
, Aug 6Labels: M-69 Test-Predator-Wrong
Owner: jbroman@chromium.org
Status: Assigned (was: Untriaged)