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

Issue 733384 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Jul 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 2
Type: Bug



Sign in to add a comment

Lock-order-inversion in pthread_mutex_lock

Project Member Reported by ClusterFuzz, Jun 14 2017

Issue description

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

Fuzzer: lcamtuf_cross_fuzz
Job Type: linux_tsan_chrome_mp
Platform Id: linux

Crash Type: Lock-order-inversion
Crash Address: 
Crash State:
  pthread_mutex_lock
  base::internal::LockImpl::Lock
  midi::TaskService::RunTask
  
Sanitizer: thread (TSAN)

Regressed: https://clusterfuzz.com/revisions?job=linux_tsan_chrome_mp&range=479362:479374

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


Issue filed automatically.

See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information.
 
Cc: msrchandra@chromium.org
Labels: Test-Predator-Wrong-CLs M-61
Owner: bcwh...@chromium.org
Status: Assigned (was: Untriaged)
Predator and CL did not provide any possible suspects. 
Using Code Search for the file, "lock_impl_posix.cc" assigning to the concern owner.

Suspecting Commit#
https://chromium.googlesource.com/chromium/src/+/d9705964f525c278a5939b6fcd29a10f732149d0

@bcwhite -- Could you please look into the issue, kindly re-assign if this is not related to your changes.
Thank You.
Owner: toyoshim@chromium.org
My CL that you reference isn't in the "regressed range".  Probably better to randomly pick one of the CLs in that range that assign it against one 10 months old.

Clusterfuzz output lists MIDI so assigning against this:
c73f9dc095133071e0d3dd918a9135443aeb8032	

Web MIDI: use midi::TaskService in MidiManagerAlsa

This patch replaces existing MidiManagerAlsa implementation
to support asynchronous method posts with midi::TaskService
that should be used on all platforms now to support such
functionalities.

BUG=672793

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

I feel this is a false alert of https://github.com/google/sanitizers/issues/488 because three locks are carefully managed, and taking order is clearly defined as |instance_lock_| -> |lock_| -> |(one of) thread_task_locks_|.

I would check again next week.
Project Member

Comment 4 by ClusterFuzz, Jun 23 2017

ClusterFuzz has detected this issue as fixed in range 481537:481544.

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

Fuzzer: lcamtuf_cross_fuzz
Job Type: linux_tsan_chrome_mp
Platform Id: linux

Crash Type: Lock-order-inversion
Crash Address: 
Crash State:
  pthread_mutex_lock
  base::internal::LockImpl::Lock
  midi::TaskService::RunTask
  
Sanitizer: thread (TSAN)

Regressed: https://clusterfuzz.com/revisions?job=linux_tsan_chrome_mp&range=479362:479374
Fixed: https://clusterfuzz.com/revisions?job=linux_tsan_chrome_mp&range=481537:481544

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


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.
Labels: -Pri-1 Pri-2
We haven't make any related change, and reported revision range seems not containing any relevant fix. Let me keep this open for a while.
Project Member

Comment 6 by ClusterFuzz, Jun 23 2017

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

If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.
Labels: ClusterFuzz-Wrong
Status: Assigned (was: Verified)
Cc: toyoshim@chromium.org
Components: Blink>WebMIDI
Owner: msrchandra@chromium.org
In this report, M85140734437680512 can be identified as |thread_task_locks_| from callstacks because it was taken in TaskService::RunTask, and kept on holding in MidiManagerAlsa::EventLoop.

On the other hand, M1311 is also obtained in TaskService::RunTask. So this could be instance_lock_ or lock_.

But, they can not be hold when MidiManagerAlsa::EventLoop runs via TaskService::RunTask because the AutoLock is always out of scope at the point to run the task.
Cc: yhirano@chromium.org tzik@chromium.org
msrchandra@, can I have a chance to get advice from thread experts on this?
Cc: -toyoshim@chromium.org
Owner: toyoshim@chromium.org
I'll take ownership back.

Probably, I could make it simple by using ReadWriteLock so that tsan becomes happy.
Project Member

Comment 12 by bugdroid1@chromium.org, Jul 4 2017

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

commit c0c38f15e6afa44b2a3b9aa6b1e4de2da71a4cbc
Author: Takashi Toyoshima <toyoshim@chromium.org>
Date: Tue Jul 04 17:05:09 2017

Web MIDI: use ReadWriteLock to make TaskService simple

Use ReadWriteLock instead of std::vector<std::unique_ptr<base::Lock>>
to remove complicated lock acquisition. This makes TaskService
implementation much simpler.

BUG= 733384 

Change-Id: I3ac62974fa6b03cf992b47bb4b737d890e8a28b0
Reviewed-on: https://chromium-review.googlesource.com/544371
Commit-Queue: Takashi Toyoshima <toyoshim@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#484133}
[modify] https://crrev.com/c0c38f15e6afa44b2a3b9aa6b1e4de2da71a4cbc/media/midi/task_service.cc
[modify] https://crrev.com/c0c38f15e6afa44b2a3b9aa6b1e4de2da71a4cbc/media/midi/task_service.h
[modify] https://crrev.com/c0c38f15e6afa44b2a3b9aa6b1e4de2da71a4cbc/media/midi/task_service_unittest.cc

Project Member

Comment 13 by ClusterFuzz, Jul 5 2017

ClusterFuzz has detected this issue as fixed in range 481537:481544.

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

Fuzzer: lcamtuf_cross_fuzz
Job Type: linux_tsan_chrome_mp
Platform Id: linux

Crash Type: Lock-order-inversion
Crash Address: 
Crash State:
  pthread_mutex_lock
  base::internal::LockImpl::Lock
  midi::TaskService::RunTask
  
Sanitizer: thread (TSAN)

Regressed: https://clusterfuzz.com/revisions?job=linux_tsan_chrome_mp&range=479362:479374
Fixed: https://clusterfuzz.com/revisions?job=linux_tsan_chrome_mp&range=481537:481544

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


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.
484025 is the last revision I can see the error in the fuzzer's timeline.
I'd monitor it in another day, and will close if there is no error after 484133.
Status: Fixed (was: Assigned)
Labels: -ClusterFuzz-Wrong
We have made a bunch of changes on ClusterFuzz side, so resetting ClusterFuzz-Wrong label.

Sign in to add a comment