New issue
Advanced search Search tips

Issue 788809 link

Starred by 1 user

Issue metadata

Status: Duplicate
Merged: issue 788802
Owner:
Closed: Nov 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug



Sign in to add a comment

Null-dereference READ in blink::ThreadState::FreePersistentNode

Project Member Reported by ClusterFuzz, Nov 27 2017

Issue description

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

Fuzzer: phoglund_webrtc_peerconnection
Job Type: linux_msan_chrome
Platform Id: linux

Crash Type: Null-dereference READ
Crash Address: 0x000000000010
Crash State:
  blink::ThreadState::FreePersistentNode
  blink::WebRTCVoidRequest::Reset
  content::RTCPeerConnectionHandler::WebRtcSetRemoteDescriptionObserverImpl::~WebR
  
Sanitizer: memory (MSAN)

Regressed: https://clusterfuzz.com/revisions?job=linux_msan_chrome&range=519268:519278

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

Additional requirements: Requires HTTP

Issue filed automatically.

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

Comment 1 by ClusterFuzz, Nov 27 2017

Labels: Test-Predator-Auto-Owner
Owner: hbos@chromium.org
Status: Assigned (was: Untriaged)
Automatically assigning owner based on suspected regression changelist https://chromium.googlesource.com/chromium/src/+/27d21eb0af100f86a2dfca5bccecf60e2d71f70e (Unify SetRemoteDescription track and SRD resolve events in one callback.).

If this is incorrect, please remove the owner and apply the Test-Predator-Wrong-CLs label.
Project Member

Comment 2 by ClusterFuzz, Nov 27 2017

Components: Blink>MemoryAllocator>GarbageCollection Platform
Labels: Test-Predator-Auto-Components
Automatically applying components based on crash stacktrace and information from OWNERS files.

If this is incorrect, please apply the Test-Predator-Wrong-Components label.

Comment 3 by hbos@chromium.org, Nov 27 2017

Status: Started (was: Assigned)
Hmm, there is a race between the webrtc signaling thread invoking OnComplete before releasing its reference to the observer, and the observer jumping to the main thread where the bind's reference is released.

As such there is a risk that the blink::WebRTCVoidRequest is destroyed on the webrtc signaling thread instead of the main thread. This could cause problems. Not sure if this is that crash or something else.

I'll create a patch to see if the flaky crashes goes away. Here's another recently reported one blaming the same CL:
 https://crbug.com/788802 

The repro test cases of both bugs look quite random. I think it's a flake, suspecting the race of the above explanation. This only relates to calling setRemoteDescription(), nothing else the repro cases are doing.
I couldn't run the repro case though, it crashes on an unrelated permissions related thing when I click "accept" on the popup...
Project Member

Comment 4 by bugdroid1@chromium.org, Nov 27 2017

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

commit 1aa2887ffca8e6ce3a74891e71720bc3e0ad809c
Author: Henrik Boström <hbos@chromium.org>
Date: Mon Nov 27 18:55:09 2017

WebRtcSetRemoteDescriptionObserverImpl: Reset WebRTCVoidRequest on main thread

This prevents a race between the main thread and the webrtc signaling
thread dictating on which thread the observer is destroyed, and by
extension which thread destroys WebRTCVoidRequest.

It is not safe to reset the request on the webrtc signaling thread, by
explicitly calling Reset in the callback we ensure it has already been
reset when the destructor runs.

The referenced bugs are suspected to be caused by this bug.

NOTRY=True
TBR=guidou@chromium.org

Bug:  788809 ,  788802 
Change-Id: Ie725fc9af85fbb4e925f1410f1c728da7173350e
Reviewed-on: https://chromium-review.googlesource.com/791350
Commit-Queue: Henrik Boström <hbos@chromium.org>
Reviewed-by: Henrik Boström <hbos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#519354}
[modify] https://crrev.com/1aa2887ffca8e6ce3a74891e71720bc3e0ad809c/content/renderer/media/rtc_peer_connection_handler.cc

Comment 5 by hbos@chromium.org, Nov 27 2017

Status: Fixed (was: Started)
Speculatively marking this as Fixed, without having been able to verify because I couldn't repro.

Comment 6 by hbos@chromium.org, Dec 5 2017

Mergedinto: 788802
Status: Duplicate (was: Fixed)

Sign in to add a comment