New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 781455 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Nov 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug



Sign in to add a comment

Null-dereference READ in blink::LayoutObject::GetFrame

Project Member Reported by ClusterFuzz, Nov 3 2017

Issue description

Detailed report: https://clusterfuzz.com/testcase?key=4510245031510016

Fuzzer: inferno_layout_test_unmodified
Job Type: linux_ubsan_vptr_content_shell_drt
Platform Id: linux

Crash Type: Null-dereference READ
Crash Address: 0x000000000000
Crash State:
  blink::LayoutObject::GetFrame
  blink::AutoscrollController::ScheduleMainThreadAnimation
  blink::AutoscrollController::Animate
  
Sanitizer: undefined (UBSAN)

Regressed: https://clusterfuzz.com/revisions?job=linux_ubsan_vptr_content_shell_drt&range=508795:508862

Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=4510245031510016

Issue filed automatically.

See https://github.com/google/clusterfuzz-tools for more information.
 
Project Member

Comment 1 by ClusterFuzz, Nov 4 2017

Components: Blink>Layout
Labels: Test-Predator-AutoComponents
Automatically applying components based on crash stacktrace and information from OWNERS files.

If this is incorrect, please apply the Test-Predator-Wrong-Components label.
Cc: msrchandra@chromium.org pnangunoori@chromium.org
Labels: M-64 Test-Predator-Wrong
Owner: joelhockey@chromium.org
Status: Assigned (was: Untriaged)
Predator and CL could not provide any possible suspects.
Using the code search for the file, “AutoscrollController.cpp” assigning to concern owner from GIT blame.

Suspecting Commit#
https://chromium.googlesource.com/chromium/src/+/d3fb9e2435e2c3015314496c2afa5bf64c4ace56
@joelhockey -- Could you please look into this issue, kindly reassign if it has nothing to do with your changes.
Thank You.

Labels: Test-Predator-Auto-CC
Labels: -Test-Predator-AutoComponents Test-Predator-Auto-Components
Labels: -Test-Predator-Auto-CC
Cc: kenrb@chromium.org bokan@chromium.org
Owner: aelias@chromium.org
This error is not related to the refactoring that I did.

Reassigning to aelias@ who was most recently modifying this code.

The problem here is in https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/page/AutoscrollController.cpp?sq=package:chromium&rcl=1d17db84433d846eb3ad0b48fe6b7b749cae6fc2&l=335-337


335      event_handler.UpdateSelectionForMouseDrag();
336      ScheduleMainThreadAnimation();
337      autoscroll_layout_object_->Autoscroll(selection_point);

In line 335, |autoscroll_layout_object_| gets cleared when style and layout is updated.  Then the call to ScheduleMainThreadAnimation fails with null-dereference on it.

Perhaps a fix is:

      event_handler.UpdateSelectionForMouseDrag();
      if (autoscroll_layout_object_) {
        ScheduleMainThreadAnimation();
        autoscroll_layout_object_->Autoscroll(selection_point);
      }

Comment 7 by bokan@chromium.org, Nov 16 2017

Components: -Blink>Layout Blink>Scroll
Owner: ----
Status: Untriaged (was: Assigned)
aelias@ is no longer working on Chrome. Adding labels so scroll team can triage tomorrow.

Comment 8 by bokan@chromium.org, Nov 16 2017

Owner: chaopeng@chromium.org
Status: Assigned (was: Untriaged)
Chao, ptal. The fix is likely just to add a null check but you might want to poke a bit deeper and see if there's something else broken that's getting us into this state.
This issue looks related to auto scroll not cancel when scrollbar/scrollable destroy. Will try my old fix.

https://chromium-review.googlesource.com/c/chromium/src/+/616901/6/third_party/WebKit/Source/platform/scroll/Scrollbar.cpp

Comment 10 by bokan@chromium.org, Nov 16 2017

Yeah, it's not related to anything scrollbars...just load balancing onto you since there's been many autoscroll bugs on sunyunjia@'s plate recently :)
Cc: joelhockey@chromium.org
I got this crash dump for the test:

Received signal 11 SEGV_MAPERR 000000000020
#0 0x7f176c4a3ecd base::debug::StackTrace::StackTrace()
#1 0x7f176c4a22fc base::debug::StackTrace::StackTrace()
#2 0x7f176c4a3885 base::debug::(anonymous namespace)::StackDumpSignalHandler()
#3 0x7f1771736330 <unknown>
#4 0x7f1763d309fc scoped_refptr<>::operator bool()
#5 0x7f17640341d9 blink::LayoutObject::GetDocument()
#6 0x7f1764589325 blink::LayoutObject::GetFrame()
#7 0x7f1764ffea49 blink::AutoscrollController::ScheduleMainThreadAnimation()
#8 0x7f1764fffa38 blink::AutoscrollController::Animate()
#9 0x7f1765037157 blink::PageWidgetDelegate::Animate()
#10 0x7f17646a565d blink::WebViewImpl::BeginFrame()
#11 0x7f17606b1458 test_runner::WebWidgetTestClient::AnimateNow()
#12 0x7f176061d1ff _ZN4base8internal13FunctorTraitsIMN11test_runner16MockColorChooserEFvvEvE6InvokeIRKNS_7WeakPtrIS3_EEJEEEvS5_OT_DpOT0_
#13 0x7f176061d17a _ZN4base8internal12InvokeHelperILb1EvE8MakeItSoIRKMN11test_runner16MockColorChooserEFvvERKNS_7WeakPtrIS5_EEJEEEvOT_OT0_DpOT1_
#14 0x7f176061d110 _ZN4base8internal7InvokerINS0_9BindStateIMN11test_runner16MockColorChooserEFvvEJNS_7WeakPtrIS4_EEEEEFvvEE7RunImplIRKS6_RKNSt3__15tupleIJS8_EEEJLm0EEEEvOT_OT0_NSF_16integer_sequenceImJXspT1_EEEE
#15 0x7f176061d05c _ZN4base8internal7InvokerINS0_9BindStateIMN11test_runner16MockColorChooserEFvvEJNS_7WeakPtrIS4_EEEEEFvvEE3RunEPNS0_13BindStateBaseE
#16 0x7f176c450061 _ZNO4base12OnceCallbackIFvvEE3RunEv
#17 0x7f176c4a86ba base::debug::TaskAnnotator::RunTask()
#18 0x7f1762aff7f3 blink::scheduler::TaskQueueManager::ProcessTaskFromWorkQueue()
#19 0x7f1762afa279 blink::scheduler::TaskQueueManager::DoWork()
#20 0x7f1762b08fd7 _ZN4base8internal13FunctorTraitsIMN5blink9scheduler16TaskQueueManagerEFvbEvE6InvokeIRKNS_7WeakPtrIS4_EEJRKbEEEvS6_OT_DpOT0_
#21 0x7f1762b08f35 _ZN4base8internal12InvokeHelperILb1EvE8MakeItSoIRKMN5blink9scheduler16TaskQueueManagerEFvbERKNS_7WeakPtrIS6_EEJRKbEEEvOT_OT0_DpOT1_
#22 0x7f1762b08ead _ZN4base8internal7InvokerINS0_9BindStateIMN5blink9scheduler16TaskQueueManagerEFvbEJNS_7WeakPtrIS5_EEbEEEFvvEE7RunImplIRKS7_RKNSt3__15tupleIJS9_bEEEJLm0ELm1EEEEvOT_OT0_NSG_16integer_sequenceImJXspT1_EEEE
#23 0x7f1762b08dbc _ZN4base8internal7InvokerINS0_9BindStateIMN5blink9scheduler16TaskQueueManagerEFvbEJNS_7WeakPtrIS5_EEbEEEFvvEE3RunEPNS0_13BindStateBaseE
#24 0x7f176c450061 _ZNO4base12OnceCallbackIFvvEE3RunEv
#25 0x7f176c4a86ba base::debug::TaskAnnotator::RunTask()
#26 0x7f176c5493e5 base::internal::IncomingTaskQueue::RunTask()
#27 0x7f176c552340 base::MessageLoop::RunTask()
#28 0x7f176c5525f6 base::MessageLoop::DeferOrRunPendingTask()
#29 0x7f176c552940 base::MessageLoop::DoWork()
#30 0x7f176c5554aa base::MessagePumpDefault::Run()
#31 0x7f176c551abc base::MessageLoop::Run()
#32 0x7f176c5feaab base::RunLoop::Run()
#33 0x7f177004579b content::RendererMain()
#34 0x7f17704c2584 content::RunZygote()
#35 0x7f17704c3261 content::RunNamedProcessTypeMain()
#36 0x7f17704c5caa content::ContentMainRunnerImpl::Run()
#37 0x7f17704bc16d content::ContentServiceManagerMainDelegate::RunEmbedderProcess()
#38 0x7f176727689d service_manager::Main()
#39 0x7f17704c1eff content::ContentMain()
#40 0x0000004f5161 main
#41 0x7f175dd1af45 __libc_start_main
#42 0x0000004f502a _start

It looks like a data racing issue since we have checked autoscroll_layout_object_->GetFrame() in AutoscrollController::Animate before call AutoscrollController::ScheduleMainThreadAnimation. Also I tried to add a log to LocalFrame::GetDocument then no crash happened. But the stack shows it calls from BeginFrame (main thread?). How can the data racing happen?
See my information in #c6.

The stack traces can be misleading.  There is no race condition.  The layout object |autoscroll_layout_object_| is set to null in the call to  event_handler.UpdateSelectionForMouseDrag() just before the call to AutoscrollController::ScheduleMainThreadAnimation.

Project Member

Comment 14 by bugdroid1@chromium.org, Nov 18 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b5360385ee24c3ba0594ba7f9758377f81944802

commit b5360385ee24c3ba0594ba7f9758377f81944802
Author: chaopeng <chaopeng@chromium.org>
Date: Sat Nov 18 07:36:27 2017

Check auto scroll not canceled before schedule animation

This issue is caused by AutoscrollForSelection call
UpdateSelectionForMouseDrag may cancel auto scroll by layout.

In this patch we check auto scroll state before schedule animation.

Bug:  781455 
Change-Id: I904ae86aaaa4694a5ce56f9daca7daf778d8a6c8
Reviewed-on: https://chromium-review.googlesource.com/776622
Commit-Queue: Jianpeng Chao <chaopeng@chromium.org>
Reviewed-by: David Bokan <bokan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#517711}
[modify] https://crrev.com/b5360385ee24c3ba0594ba7f9758377f81944802/third_party/WebKit/Source/core/BUILD.gn
[modify] https://crrev.com/b5360385ee24c3ba0594ba7f9758377f81944802/third_party/WebKit/Source/core/page/AutoscrollController.cpp
[modify] https://crrev.com/b5360385ee24c3ba0594ba7f9758377f81944802/third_party/WebKit/Source/core/page/AutoscrollController.h
[add] https://crrev.com/b5360385ee24c3ba0594ba7f9758377f81944802/third_party/WebKit/Source/core/page/AutoscrollControllerTest.cpp

Project Member

Comment 15 by ClusterFuzz, Nov 19 2017

ClusterFuzz has detected this issue as fixed in range 517702:517712.

Detailed report: https://clusterfuzz.com/testcase?key=4510245031510016

Fuzzer: inferno_layout_test_unmodified
Job Type: linux_ubsan_vptr_content_shell_drt
Platform Id: linux

Crash Type: Null-dereference READ
Crash Address: 0x000000000000
Crash State:
  blink::LayoutObject::GetFrame
  blink::AutoscrollController::ScheduleMainThreadAnimation
  blink::AutoscrollController::Animate
  
Sanitizer: undefined (UBSAN)

Regressed: https://clusterfuzz.com/revisions?job=linux_ubsan_vptr_content_shell_drt&range=508795:508862
Fixed: https://clusterfuzz.com/revisions?job=linux_ubsan_vptr_content_shell_drt&range=517702:517712

Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=4510245031510016

See https://github.com/google/clusterfuzz-tools for more information.

If you suspect that the result above is incorrect, try re-doing that job on the test case report page.
Project Member

Comment 16 by ClusterFuzz, Nov 19 2017

Labels: ClusterFuzz-Verified
Status: Verified (was: Assigned)
ClusterFuzz testcase 4510245031510016 is verified as fixed, so closing issue as verified.

If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.

Sign in to add a comment