admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix for use after free in simulated time controller.

Browse Source 
We need to keep the lock until we have finished using the task runner
returned by GetNextReadyRunner to ensure that we don't remove it from
another thread. Additionally, we must get only one runner at a time in
case the first runner removes the second runner.

Bug: webrtc:10538
Change-Id: Idbd5610b67666238545b3a321fb79f7e86fcac56
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/132342
Reviewed-by: Niels Moller <nisse@webrtc.org>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Commit-Queue: Sebastian Jansson <srte@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#27584}
This commit is contained in:
Sebastian Jansson 2019-04-11 17:48:30 +02:00 committed by Commit Bot
parent ff8cce37b6
commit 7654081e91
2 changed files with 22 additions and 20 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

33
test/time_controller/simulated_time_controller.cc
View File

@ -312,18 +312,6 @@ std::unique_ptr<ProcessThread> SimulatedTimeControllerImpl::CreateProcessThread(
return process_thread;
}
std::vector<SimulatedSequenceRunner*>
SimulatedTimeControllerImpl::GetNextReadyRunner(Timestamp current_time) {
rtc::CritScope lock(&lock_);
std::vector<SimulatedSequenceRunner*> ready;
for (auto* runner : runners_) {
if (yielded_.find(runner) == yielded_.end() &&
runner->GetNextRunTime() <= current_time) {
ready.push_back(runner);
}
}
return ready;
}
void SimulatedTimeControllerImpl::YieldExecution() {
if (rtc::CurrentThreadId() == thread_id_) {
@ -352,11 +340,23 @@ void SimulatedTimeControllerImpl::RunReadyRunners() {
// We repeat until we have no ready left to handle tasks posted by ready
// runners.
while (true) {
auto ready = GetNextReadyRunner(current_time);
if (ready.empty())
break;
for (auto* runner : ready) {
rtc::CritScope lock(&lock_);
RTC_DCHECK(ready_runners_.empty());
for (auto* runner : runners_) {
if (yielded_.find(runner) == yielded_.end() &&
runner->GetNextRunTime() <= current_time) {
ready_runners_.push_back(runner);
}
}
if (ready_runners_.empty())
return;
while (!ready_runners_.empty()) {
auto* runner = ready_runners_.front();
ready_runners_.pop_front();
runner->UpdateReady(current_time);
// Note that the Run function might indirectly cause a call to
// Unregister() which will recursively grab |lock_| again to remove items
// from |ready_runners_|.
runner->Run(current_time);
}
}
@ -390,6 +390,7 @@ void SimulatedTimeControllerImpl::Unregister(SimulatedSequenceRunner* runner) {
rtc::CritScope lock(&lock_);
bool removed = RemoveByValue(runners_, runner);
RTC_CHECK(removed);
RemoveByValue(ready_runners_, runner);
}
} // namespace sim_time_impl

9
test/time_controller/simulated_time_controller.h
View File

@ -60,16 +60,17 @@ class SimulatedTimeControllerImpl : public TaskQueueFactory,
void Unregister(SimulatedSequenceRunner* runner);
private:
// Returns runners in |runners_| that are ready for execution.
std::vector<SimulatedSequenceRunner*> GetNextReadyRunner(
Timestamp current_time) RTC_RUN_ON(thread_checker_);
const rtc::PlatformThreadId thread_id_;
rtc::ThreadChecker thread_checker_;
rtc::CriticalSection time_lock_;
Timestamp current_time_ RTC_GUARDED_BY(time_lock_);
rtc::CriticalSection lock_;
std::vector<SimulatedSequenceRunner*> runners_ RTC_GUARDED_BY(lock_);
// Used in RunReadyRunners() to keep track of ready runners that are to be
// processed in a round robin fashion. the reason it's a member is so that
// runners can removed from here by Unregister().
std::list<SimulatedSequenceRunner*> ready_runners_ RTC_GUARDED_BY(lock_);
// Task queues on which YieldExecution has been called.
std::unordered_set<TaskQueueBase*> yielded_ RTC_GUARDED_BY(thread_checker_);
};