New issue
Advanced search Search tips

Issue 620442 link

Starred by 3 users

Issue metadata

Status: Verified
Owner:
Closed: Jul 2016
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug



Sign in to add a comment

Lock-order-inversion in pthread_mutex_lock

Project Member Reported by ClusterFuzz, Jun 15 2016

Issue description

Detailed report: https://cluster-fuzz.appspot.com/testcase?key=4620105287991296

Fuzzer: ochang_domfuzzer
Job Type: linux_tsan_chrome_mp
Platform Id: linux

Crash Type: Lock-order-inversion
Crash Address: 
Crash State:
  pthread_mutex_lock
  blink::WorkerThread::postTask
  blink::WorkerGlobalScope::postTask
  

Unminimized Testcase: https://cluster-fuzz.appspot.com/download/AMIfv94SW3UqKr4mebeuz4J-U3ubx_9lPtqyxGqhleYGk3ixgt51biRhwfgYJPksPDn2EtQ5X0YFZUbcxbyyA-wqI8y4klZAyXHxR5I4syrWX-yCL4DsNMz9TsGHXrXqvCgFVxPtdA84t0gxR8Fa5nyhi26OfwqBhw


Additional requirements: Requires Gestures

Filer: mmohammad

See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information.
 
Owner: yutak@chromium.org
Status: Assigned (was: Available)
looks this is the suspected CL:
https://chromium.googlesource.com/chromium/src/+/482d8313491ff6cc881dd38a0714e61966200624%5E%21/third_party/WebKit/Source/core/workers/WorkerGlobalScope.cpp


yutak@ Could you please look into this and reassign if needed.Thank you 

Comment 2 by yutak@chromium.org, Jun 16 2016

Components: Blink>Workers
Owner: ----
Status: Untriaged (was: Assigned)
Status: Available (was: Untriaged)

Comment 4 by sigbjo...@opera.com, Jun 20 2016

Owner: sigbjo...@opera.com
Status: Assigned (was: Available)

Comment 5 by sigbjo...@opera.com, Jun 20 2016

Cc: nhiroki@chromium.org
Owner: ----
Status: Available (was: Assigned)
Testcase tries to re-lock m_threadStateMutex in WorkerThread::postTask() while already holding the lock in WorkerThread::initializeOnWorkerThread().

postTask() locking was added by https://codereview.chromium.org/2041753002

Comment 6 by falken@chromium.org, Jul 11 2016

Cc: -nhiroki@chromium.org
Labels: M-53
Owner: nhiroki@chromium.org
nhiroki: can you look at this? Last time we had a recursive lock bug was not good times.
Project Member

Comment 7 by sheriffbot@chromium.org, Jul 11 2016

Labels: -M-53 M-54 MovedFrom-53
Moving this nonessential bug to the next milestone.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 8 by falken@chromium.org, Jul 12 2016

Labels: -M-54 -MovedFrom-53 M-53
This should be handled in M53.
Status: Started (was: Available)
Project Member

Comment 10 by bugdroid1@chromium.org, Jul 15 2016

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

commit 18d1be3c48e503e5d080361f8fe6df7cfd2b503e
Author: nhiroki <nhiroki@chromium.org>
Date: Fri Jul 15 07:29:56 2016

Worker: Remove locking from WorkerThread::postTask()

WorkerThread::postTask() can be called from various places and locking in the
function may lead to deadlock. To avoid the problem, this CL removes locking
from WorkerThread::postTask() and makes it possible to safely access to shared
flags without locking.

BUG= 620442 

Review-Url: https://codereview.chromium.org/2142273004
Cr-Commit-Position: refs/heads/master@{#405715}

[modify] https://crrev.com/18d1be3c48e503e5d080361f8fe6df7cfd2b503e/third_party/WebKit/Source/core/workers/WorkerThread.cpp
[modify] https://crrev.com/18d1be3c48e503e5d080361f8fe6df7cfd2b503e/third_party/WebKit/Source/core/workers/WorkerThread.h

Project Member

Comment 11 by ClusterFuzz, Jul 16 2016

ClusterFuzz has detected this issue as fixed in range 405656:405727.

Detailed report: https://cluster-fuzz.appspot.com/testcase?key=4620105287991296

Fuzzer: ochang_domfuzzer
Job Type: linux_tsan_chrome_mp
Platform Id: linux

Crash Type: Lock-order-inversion
Crash Address: 
Crash State:
  pthread_mutex_lock
  blink::WorkerThread::postTask
  blink::WorkerGlobalScope::postTask
  
Regressed: https://cluster-fuzz.appspot.com/revisions?job=linux_tsan_chrome_mp&range=398598:398917
Fixed: https://cluster-fuzz.appspot.com/revisions?job=linux_tsan_chrome_mp&range=405656:405727

Unminimized Testcase: https://cluster-fuzz.appspot.com/download/AMIfv94SW3UqKr4mebeuz4J-U3ubx_9lPtqyxGqhleYGk3ixgt51biRhwfgYJPksPDn2EtQ5X0YFZUbcxbyyA-wqI8y4klZAyXHxR5I4syrWX-yCL4DsNMz9TsGHXrXqvCgFVxPtdA84t0gxR8Fa5nyhi26OfwqBhw?testcase_id=4620105287991296


Additional requirements: Requires Gestures

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.
Project Member

Comment 12 by ClusterFuzz, Jul 16 2016

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

If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.
Labels: Merge-Request-53
This is a regression in M53. The fix to be merged was verified on ClusterFuzz.

Comment 14 by dimu@google.com, Jul 18 2016

Labels: -Merge-Request-53 Merge-Approved-53 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M53 (branch: 2785)
Project Member

Comment 15 by bugdroid1@chromium.org, Jul 20 2016

Labels: -merge-approved-53 merge-merged-2785
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b70891a44868d73034b78417cad991307819e5a5

commit b70891a44868d73034b78417cad991307819e5a5
Author: Hiroki Nakagawa <nhiroki@chromium.org>
Date: Wed Jul 20 01:41:01 2016

[Merge to M53] Worker: Remove locking from WorkerThread::postTask()

WorkerThread::postTask() can be called from various places and locking in the
function may lead to deadlock. To avoid the problem, this CL removes locking
from WorkerThread::postTask() and makes it possible to safely access to shared
flags without locking.

BUG= 620442 

Review-Url: https://codereview.chromium.org/2142273004
Cr-Commit-Position: refs/heads/master@{#405715}
(cherry picked from commit 18d1be3c48e503e5d080361f8fe6df7cfd2b503e)

Review URL: https://codereview.chromium.org/2164873002 .

Cr-Commit-Position: refs/branch-heads/2785@{#234}
Cr-Branched-From: 68623971be0cfc492a2cb0427d7f478e7b214c24-refs/heads/master@{#403382}

[modify] https://crrev.com/b70891a44868d73034b78417cad991307819e5a5/third_party/WebKit/Source/core/workers/WorkerThread.cpp
[modify] https://crrev.com/b70891a44868d73034b78417cad991307819e5a5/third_party/WebKit/Source/core/workers/WorkerThread.h

Nice fix!
Project Member

Comment 17 by sheriffbot@chromium.org, Nov 22 2016

Labels: -Restrict-View-EditIssue
Removing EditIssue view restrictions from ClusterFuzz filed bugs. If you believe that this issue should still be restricted, please reapply the label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Sign in to add a comment