New issue
Advanced search Search tips

Issue 820769 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Mar 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug-Security



Sign in to add a comment

Use-of-uninitialized-value in rtc::ClosureTask<webrtc::VideoStreamEncoder::OnEncodedImage

Project Member Reported by ClusterFuzz, Mar 11 2018

Issue description

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

Fuzzer: phoglund_webrtc_peerconnection
Job Type: linux_msan_chrome
Platform Id: linux

Crash Type: Use-of-uninitialized-value
Crash Address: 
Crash State:
  rtc::ClosureTask<webrtc::VideoStreamEncoder::OnEncodedImage
  rtc::TaskQueue::Impl::RunTask
  void base::internal::Invoker<base::internal::BindState<void
  
Sanitizer: memory (MSAN)

Recommended Security Severity: Medium

Regressed: https://clusterfuzz.com/revisions?job=linux_msan_chrome&range=542245:542262

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

Additional requirements: Requires HTTP

Issue filed automatically.

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

Comment 1 by ClusterFuzz, Mar 11 2018

Components: Blink>WebRTC Internals>Core
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.
Project Member

Comment 2 by ClusterFuzz, Mar 11 2018

Labels: Test-Predator-Auto-Owner
Owner: tommi@chromium.org
Status: Assigned (was: Untriaged)
Automatically assigning owner based on suspected regression changelist https://chromium.googlesource.com/chromium/src/+/5773863a93f2b00e2a40fa134ad32f1da48ae588 (Make WebRTC task queues run on chromium's SequencedTaskRunner.).

If this is incorrect, please let us know why and apply the Test-Predator-Wrong-CLs label. If you aren't the correct owner for this issue, please unassign yourself as soon as possible so it can be re-triaged.

Comment 3 by tommi@chromium.org, Mar 11 2018

Cc: tommi@chromium.org
Owner: nisse@chromium.org
Nisse - can you take a look?  Looks like this could be related to this change:
https://webrtc.googlesource.com/src.git/+/83dbeacb1ad128d2ad7fe476180f820b1499bb90

What seems to have triggered the discovery of this, is the fact that we're no longer running in 1:1 mode in Chrome, meaning 1x TQ : 1x dedicated thread.  TaskQueues in Chrome, now run on top of a thread pool.

Here's the stack from the report:

#0 0x563830176b32 in operator-> buildtools/third_party/libc++/trunk/include/memory:2568:19
#1 0x563830176b32 in webrtc::OveruseFrameDetector::FrameSent(unsigned int, long, long, rtc::Optional<int>) third_party/webrtc/video/overuse_frame_detector.cc:613
#2 0x563830196228 in operator() third_party/webrtc/video/video_stream_encoder.cc:850:28
 #3 0x563830196228 in rtc::ClosureTask<webrtc::VideoStreamEncoder::OnEncodedImage(webrtc::EncodedImage const&, webrtc::CodecSpecificInfo const*, webrtc::RTPFragmentationHeader const*)::$_7>::Run() third_party/webrtc/rtc_base/task_queue.h:53
 #4 0x56380f90d6e2 in rtc::TaskQueue::Impl::RunTask(std::__1::unique_ptr<rtc::QueuedTask, std::__1::default_delete<rtc::QueuedTask> >) third_party/webrtc_overrides/rtc_base/task_queue.cc:179:14
#5 0x56380f90ec3e in Invoke<scoped_refptr<rtc::TaskQueue::Impl>, std::__1::unique_ptr<rtc::QueuedTask, std::__1::default_delete<rtc::QueuedTask> > > base/bind_internal.h:447:12
#6 0x56380f90ec3e in MakeItSo<void (rtc::TaskQueue::Impl::*)(std::__1::unique_ptr<rtc::QueuedTask, std::__1::default_delete<rtc::QueuedTask> >), scoped_refptr<rtc::TaskQueue::Impl>, std::__1::unique_ptr<rtc::QueuedTask, std::__1::default_delete<rtc::QueuedTask> > > base/bind_internal.h:530
 #7 0x56380f90ec3e in void base::internal::Invoker<base::internal::BindState<void (rtc::TaskQueue::Impl::*)(std::__1::unique_ptr<rtc::QueuedTask, std::__1::default_delete<rtc::QueuedTask> >), scoped_refptr<rtc::TaskQueue::Impl>, base::internal::PassedWrapper<std::__1::unique_ptr<rtc::QueuedTask, std::__1::default_delete<rtc::QueuedTask> > > >, void ()>::RunImpl<void (rtc::TaskQueue::Impl::*)(std::__1::unique_ptr<rtc::QueuedTask, std::__1::default_delete<rtc::QueuedTask> >), std::__1::tuple<scoped_refptr<rtc::TaskQueue::Impl>, base::internal::PassedWrapper<std::__1::unique_ptr<rtc::QueuedTask, std::__1::default_delete<rtc::QueuedTask> > > >, 0ul, 1ul>(void (rtc::TaskQueue::Impl::*&&)(std::__1::unique_ptr<rtc::QueuedTask, std::__1::default_delete<rtc::QueuedTask> >), std::__1::tuple<scoped_refptr<rtc::TaskQueue::Impl>, base::internal::PassedWrapper<std::__1::unique_ptr<rtc::QueuedTask, std::__1::default_delete<rtc::QueuedTask> > > >&&, std::__1::integer_sequence<unsigned long, 0ul, 1ul>) base/bind_internal.h:604
#8 0x5638165abdb9 in Run base/callback.h:95:12
#9 0x5638165abdb9 in base::debug::TaskAnnotator::RunTask(char const*, base::PendingTask*) base/debug/task_annotator.cc:61
#10 0x563816858c65 in base::internal::TaskTracker::RunOrSkipTask(base::internal::Task, base::internal::Sequence*, bool) base/task_scheduler/task_tracker.cc:460:23
#11 0x5638168611d4 in base::internal::TaskTrackerPosix::RunOrSkipTask(base::internal::Task, base::internal::Sequence*, bool) base/task_scheduler/task_tracker_posix.cc:25:16
#12 0x563816855b6a in base::internal::TaskTracker::RunAndPopNextTask(scoped_refptr<base::internal::Sequence>, base::internal::CanScheduleSequenceObserver*) base/task_scheduler/task_tracker.cc:353:3
#13 0x563816845e1a in base::internal::SchedulerWorker::Thread::ThreadMain() base/task_scheduler/scheduler_worker.cc:85:41
#14 0x563816892e73 in base::(anonymous namespace)::ThreadFunc(void*) base/threading/platform_thread_posix.cc:76:13
Project Member

Comment 4 by sheriffbot@chromium.org, Mar 11 2018

Labels: M-66
Project Member

Comment 5 by sheriffbot@chromium.org, Mar 11 2018

Labels: ReleaseBlock-Stable
This is a serious security regression. If you are not able to fix this quickly, please revert the change that introduced it.

If this doesn't affect a release branch, or has not been properly classified for severity, please update the Security_Impact or Security_Severity labels, and remove the ReleaseBlock label. To disable this altogether, apply ReleaseBlock-NA.

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

Comment 6 by sheriffbot@chromium.org, Mar 11 2018

Labels: Pri-1
Project Member

Comment 7 by sheriffbot@chromium.org, Mar 12 2018

Labels: -Security_Impact-Head Security_Impact-Beta
Project Member

Comment 8 by bugdroid1@chromium.org, Mar 12 2018

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

commit be53eac959ea4146e4fa7d9db046f5ec4c861307
Author: Tommi <tommi@chromium.org>
Date: Mon Mar 12 15:30:03 2018

Add call to TaskQueue::Impl::Stop() in TQ's dtor.

This call got removed in my last CL by mistake:
https://chromium-review.googlesource.com/c/chromium/src/+/890738

Bug:  808801 , 820936,  820769 ,  820830 ,  820827 
Change-Id: I6c6efb3d40d43bc564e18b013ddf5d5d3b09e1eb
Reviewed-on: https://chromium-review.googlesource.com/958218
Commit-Queue: Tommi <tommi@chromium.org>
Reviewed-by: Guido Urdaneta <guidou@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542486}
[modify] https://crrev.com/be53eac959ea4146e4fa7d9db046f5ec4c861307/third_party/webrtc_overrides/rtc_base/task_queue.cc

Comment 9 by tommi@chromium.org, Mar 12 2018

Cc: -tommi@chromium.org nisse@chromium.org
Owner: tommi@chromium.org
Project Member

Comment 10 by ClusterFuzz, Mar 13 2018

ClusterFuzz has detected this issue as fixed in range 542485:542486.

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

Fuzzer: phoglund_webrtc_peerconnection
Job Type: linux_msan_chrome
Platform Id: linux

Crash Type: Use-of-uninitialized-value
Crash Address: 
Crash State:
  rtc::ClosureTask<webrtc::VideoStreamEncoder::OnEncodedImage
  rtc::TaskQueue::Impl::RunTask
  void base::internal::Invoker<base::internal::BindState<void
  
Sanitizer: memory (MSAN)

Recommended Security Severity: Medium

Regressed: https://clusterfuzz.com/revisions?job=linux_msan_chrome&range=542245:542262
Fixed: https://clusterfuzz.com/revisions?job=linux_msan_chrome&range=542485:542486

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

Additional requirements: Requires HTTP

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 11 by ClusterFuzz, Mar 13 2018

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

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

Comment 12 by sheriffbot@chromium.org, Mar 13 2018

Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
Labels: -ReleaseBlock-Stable -M-66 M-67
Project Member

Comment 14 by sheriffbot@chromium.org, Jun 19 2018

Labels: -Restrict-View-SecurityNotify allpublic
This bug has been closed for more than 14 weeks. Removing security view restrictions.

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

Sign in to add a comment