From 32562250ca97ccee5ef00adb85aae206092d8d0b Mon Sep 17 00:00:00 2001 From: Karl Wiberg Date: Thu, 21 Feb 2019 13:38:30 +0100 Subject: [PATCH] "Remove" loophole in rtc::Thread::ScopedDisallowBlockingCalls It was previously possible to escape the sandbox by calling rtc::Thread::SetAllowBlockingCalls(true). This CL only removes the loophole on non-Android builds, because we still have old Android code that relies on it. We expect that code to go away soon-ish, though. Bug: webrtc:9987 Change-Id: Ida96400d0abe430af4c2046284795d37d64f6613 Reviewed-on: https://webrtc-review.googlesource.com/c/123523 Commit-Queue: Karl Wiberg Reviewed-by: Tommi Cr-Commit-Position: refs/heads/master@{#26792} --- pc/channel_manager.cc | 2 +- rtc_base/thread.h | 19 +++++++++++++++---- .../src/jni/android_media_codec_common.h | 2 +- 3 files changed, 17 insertions(+), 6 deletions(-) diff --git a/pc/channel_manager.cc b/pc/channel_manager.cc index 49da430297..0a1c8b4876 100644 --- a/pc/channel_manager.cc +++ b/pc/channel_manager.cc @@ -127,7 +127,7 @@ bool ChannelManager::Init() { if (!network_thread_->IsCurrent()) { // Do not allow invoking calls to other threads on the network thread. network_thread_->Invoke( - RTC_FROM_HERE, [&] { network_thread_->SetAllowBlockingCalls(false); }); + RTC_FROM_HERE, [&] { network_thread_->DisallowBlockingCalls(); }); } if (media_engine_) { diff --git a/rtc_base/thread.h b/rtc_base/thread.h index e2d37a07db..3ba3010fd9 100644 --- a/rtc_base/thread.h +++ b/rtc_base/thread.h @@ -219,10 +219,6 @@ class RTC_LOCKABLE Thread : public MessageQueue { // of whatever code is conditionally executing because of the return value! bool RunningForTest() { return IsRunning(); } - // Sets the per-thread allow-blocking-calls flag and returns the previous - // value. Must be called on this thread. - bool SetAllowBlockingCalls(bool allow); - // These functions are public to avoid injecting test hooks. Don't call them // outside of tests. // This method should be called when thread is created using non standard @@ -232,6 +228,17 @@ class RTC_LOCKABLE Thread : public MessageQueue { bool WrapCurrent(); void UnwrapCurrent(); + // Sets the per-thread allow-blocking-calls flag to false; this is + // irrevocable. Must be called on this thread. + void DisallowBlockingCalls() { SetAllowBlockingCalls(false); } + +#ifdef WEBRTC_ANDROID + // Sets the per-thread allow-blocking-calls flag to true, sidestepping the + // invariants upheld by DisallowBlockingCalls() and + // ScopedDisallowBlockingCalls. Must be called on this thread. + void DEPRECATED_AllowBlockingCalls() { SetAllowBlockingCalls(true); } +#endif + protected: // Same as WrapCurrent except that it never fails as it does not try to // acquire the synchronization access of the thread. The caller should never @@ -251,6 +258,10 @@ class RTC_LOCKABLE Thread : public MessageQueue { Runnable* runnable; }; + // Sets the per-thread allow-blocking-calls flag and returns the previous + // value. Must be called on this thread. + bool SetAllowBlockingCalls(bool allow); + #if defined(WEBRTC_WIN) static DWORD WINAPI PreRun(LPVOID context); #else diff --git a/sdk/android/src/jni/android_media_codec_common.h b/sdk/android/src/jni/android_media_codec_common.h index 55955b0d41..be2eb19ba6 100644 --- a/sdk/android/src/jni/android_media_codec_common.h +++ b/sdk/android/src/jni/android_media_codec_common.h @@ -66,7 +66,7 @@ enum { kMaxEncodedLogFrames = 10 }; static inline void AllowBlockingCalls() { rtc::Thread* current_thread = rtc::Thread::Current(); if (current_thread != NULL) - current_thread->SetAllowBlockingCalls(true); + current_thread->DEPRECATED_AllowBlockingCalls(); } // Checks for any Java exception, prints stack backtrace and clears