diff --git a/libwvdrmengine/cdm/core/include/crypto_session.h b/libwvdrmengine/cdm/core/include/crypto_session.h index 7f6595e3..ba9c4d81 100644 --- a/libwvdrmengine/cdm/core/include/crypto_session.h +++ b/libwvdrmengine/cdm/core/include/crypto_session.h @@ -44,11 +44,11 @@ class CryptoSessionFactory; class CryptoSession { public: using HdcpCapability = OEMCrypto_HDCP_Capability; - typedef enum { + enum UsageDurationStatus { kUsageDurationsInvalid = 0, kUsageDurationPlaybackNotBegun = 1, kUsageDurationsValid = 2, - } UsageDurationStatus; + }; struct SupportedCertificateTypes { bool rsa_2048_bit; @@ -64,6 +64,17 @@ class CryptoSession { virtual ~CryptoSession(); + // This method will try to terminate OEMCrypto if |session_size_| is 0. + // A platform configured property |delay_oem_crypto_termination| will + // determine if termination occurs immediately or after a delay. + // If termination is delayed, a countdown mechanism is employed. + // Call |TryTerminate| periodically until it no longer returns true. + // To immediately terminate call |DisableDelayedTermination| before calling + // |TryTerminate|. + static bool TryTerminate(); + + static void DisableDelayedTermination(); + virtual CdmResponseType GetProvisioningToken(std::string* client_token); virtual CdmClientTokenType GetPreProvisionTokenType() { return pre_provision_token_type_; @@ -282,7 +293,6 @@ class CryptoSession { } void Init(); - void Terminate(); CdmResponseType GetTokenFromKeybox(std::string* token); CdmResponseType GetTokenFromOemCert(std::string* token); static bool ExtractSystemIdFromOemCert(const std::string& oem_cert, @@ -385,6 +395,7 @@ class CryptoSession { static bool initialized_; static int session_count_; + static int termination_counter_; metrics::CryptoMetrics* metrics_; metrics::TimerMetric life_span_; diff --git a/libwvdrmengine/cdm/core/src/crypto_session.cpp b/libwvdrmengine/cdm/core/src/crypto_session.cpp index 7024a0f7..4f1b13b7 100644 --- a/libwvdrmengine/cdm/core/src/crypto_session.cpp +++ b/libwvdrmengine/cdm/core/src/crypto_session.cpp @@ -60,6 +60,7 @@ const size_t kOemCryptoApiVersionSupportsSwitchingCipherMode = 14; // Constants and utility objects relating to OEM Certificates const char* const kWidevineSystemIdExtensionOid = "1.3.6.1.4.1.11129.4.1.1"; +constexpr int kMaxTerminateCountDown = 5; const std::string kStringNotAvailable = "NA"; @@ -96,6 +97,7 @@ shared_mutex CryptoSession::static_field_mutex_; shared_mutex CryptoSession::oem_crypto_mutex_; bool CryptoSession::initialized_ = false; int CryptoSession::session_count_ = 0; +int CryptoSession::termination_counter_ = 0; UsageTableHeader* CryptoSession::usage_table_header_l1_ = nullptr; UsageTableHeader* CryptoSession::usage_table_header_l3_ = nullptr; std::atomic CryptoSession::request_id_index_source_(0); @@ -189,7 +191,15 @@ CryptoSession::~CryptoSession() { if (open_) { Close(); } - Terminate(); + WithStaticFieldWriteLock("~CryptoSession", [&] { + if (session_count_ > 0) { + --session_count_; + } else { + LOGE("Invalid crypto session count: session_count_ = %d", session_count_); + } + }); + + TryTerminate(); M_RECORD(metrics_, crypto_session_life_span_, life_span_.AsMs()); } @@ -244,6 +254,9 @@ void CryptoSession::Init() { LOGE("OEMCrypto_Initialize failed: status = %d", static_cast(sts)); return; } + termination_counter_ = Properties::delay_oem_crypto_termination() + ? kMaxTerminateCountDown + : 0; initialized_ = true; initialized = true; } @@ -273,17 +286,20 @@ void CryptoSession::Init() { } } -void CryptoSession::Terminate() { +bool CryptoSession::TryTerminate() { LOGV("Terminating crypto session"); - WithStaticFieldWriteLock("Terminate", [&] { - LOGV("Terminating crypto session: initialized_ = %s, session_count_ = %d", - initialized_ ? "true" : "false", session_count_); - if (session_count_ > 0) { - session_count_ -= 1; - } else { - LOGE("Invalid crypto session count: session_count_ = %d", session_count_); + WithStaticFieldWriteLock("TryTerminate", [&] { + LOGV( + "Terminating crypto session: initialized_ = %s, session_count_ = %d, " + "termination_counter_ = %d", + initialized_ ? "true" : "false", session_count_, termination_counter_); + + if (termination_counter_ > 0) { + --termination_counter_; } - if (session_count_ > 0 || !initialized_) return; + if (session_count_ > 0 || termination_counter_ > 0 || !initialized_) + return false; + OEMCryptoResult sts; WithOecWriteLock("Terminate", [&] { sts = OEMCrypto_Terminate(); }); if (OEMCrypto_SUCCESS != sts) { @@ -300,7 +316,15 @@ void CryptoSession::Terminate() { } initialized_ = false; + return true; }); + return true; +} + +void CryptoSession::DisableDelayedTermination() { + LOGV("Disable delayed termination"); + WithStaticFieldWriteLock("DisableDelayedTermination", + [&] { termination_counter_ = 0; }); } CdmResponseType CryptoSession::GetTokenFromKeybox(std::string* token) { @@ -687,6 +711,11 @@ CdmResponseType CryptoSession::Open(SecurityLevel requested_security_level) { return MapOEMCryptoResult(sts, OPEN_CRYPTO_SESSION_ERROR, "Open"); } + WithStaticFieldWriteLock("Open() termination_counter", [&] { + termination_counter_ = + Properties::delay_oem_crypto_termination() ? kMaxTerminateCountDown : 0; + }); + oec_session_id_ = static_cast(sid); LOGV("Opened session: id = %u", oec_session_id_); open_ = true; diff --git a/libwvdrmengine/cdm/include/timer.h b/libwvdrmengine/cdm/include/timer.h index e9ebdfab..cce872a3 100644 --- a/libwvdrmengine/cdm/include/timer.h +++ b/libwvdrmengine/cdm/include/timer.h @@ -43,7 +43,10 @@ class Timer { ~Timer(); bool Start(TimerHandler* handler, uint32_t time_in_secs); - void Stop(); + // Enable |wait_for_exit| only if the method is being invoked from a thread + // other than the timer thread. Deadlock might occur if enabled and called + // from the timer thread. + void Stop(bool wait_for_exit); bool IsRunning(); private: diff --git a/libwvdrmengine/cdm/include/wv_content_decryption_module.h b/libwvdrmengine/cdm/include/wv_content_decryption_module.h index 39fc2c29..457c415e 100644 --- a/libwvdrmengine/cdm/include/wv_content_decryption_module.h +++ b/libwvdrmengine/cdm/include/wv_content_decryption_module.h @@ -184,14 +184,17 @@ class WvContentDecryptionModule : public android::RefBase, public TimerHandler { uint32_t GenerateSessionSharingId(); - // timer related methods to drive policy decisions - void EnablePolicyTimer(); - void DisablePolicyTimer(); + // Timer related methods to drive policy decisions + void EnableTimer(); + void DisableTimer(); + // Call this method only if invoked from a thread other than the timer thread. + // Otherwise this might result in deadlock. + void DisableTimerAndWaitForExit(); void OnTimerEvent(); static std::mutex session_sharing_id_generation_lock_; - std::mutex policy_timer_lock_; - Timer policy_timer_; + std::mutex timer_lock_; + Timer timer_; // instance variables // This manages the lifetime of the CDM instances. diff --git a/libwvdrmengine/cdm/src/timer.cpp b/libwvdrmengine/cdm/src/timer.cpp index 035a7070..429a1ddf 100644 --- a/libwvdrmengine/cdm/src/timer.cpp +++ b/libwvdrmengine/cdm/src/timer.cpp @@ -28,12 +28,13 @@ class Timer::Impl : virtual public android::RefBase { return run("wvcdm::Timer::Impl") == android::NO_ERROR; } - void Stop() { - { - android::Mutex::Autolock autoLock(lock_); - stop_condition_.signal(); + void Stop(bool wait_for_exit) { + stop_condition_.signal(); + if (wait_for_exit) { + requestExitAndWait(); + } else { + requestExit(); } - requestExitAndWait(); } private: @@ -63,8 +64,8 @@ class Timer::Impl : virtual public android::RefBase { return impl_thread_->Start(handler, time_in_secs); } - void Stop() { - impl_thread_->Stop(); + void Stop(bool wait_for_exit) { + impl_thread_->Stop(wait_for_exit); impl_thread_.clear(); } @@ -78,7 +79,7 @@ class Timer::Impl : virtual public android::RefBase { Timer::Timer() : impl_(new Timer::Impl()) {} Timer::~Timer() { - if (IsRunning()) Stop(); + if (IsRunning()) Stop(false); } bool Timer::Start(TimerHandler* handler, uint32_t time_in_secs) { @@ -87,7 +88,7 @@ bool Timer::Start(TimerHandler* handler, uint32_t time_in_secs) { return impl_->Start(handler, time_in_secs); } -void Timer::Stop() { impl_->Stop(); } +void Timer::Stop(bool wait_for_exit) { impl_->Stop(wait_for_exit); } bool Timer::IsRunning() { return impl_->IsRunning(); } diff --git a/libwvdrmengine/cdm/src/wv_content_decryption_module.cpp b/libwvdrmengine/cdm/src/wv_content_decryption_module.cpp index 6be9dcc7..3eb0697b 100644 --- a/libwvdrmengine/cdm/src/wv_content_decryption_module.cpp +++ b/libwvdrmengine/cdm/src/wv_content_decryption_module.cpp @@ -18,7 +18,7 @@ #include "wv_cdm_event_listener.h" namespace { -const int kCdmPolicyTimerDurationSeconds = 1; +const int kCdmTimerDurationSeconds = 1; } namespace wvcdm { @@ -28,8 +28,10 @@ std::mutex WvContentDecryptionModule::session_sharing_id_generation_lock_; WvContentDecryptionModule::WvContentDecryptionModule() {} WvContentDecryptionModule::~WvContentDecryptionModule() { + CryptoSession::DisableDelayedTermination(); CloseAllCdms(); - DisablePolicyTimer(); + CryptoSession::TryTerminate(); + DisableTimerAndWaitForExit(); } bool WvContentDecryptionModule::IsSupported(const std::string& init_data_type) { @@ -123,16 +125,9 @@ CdmResponseType WvContentDecryptionModule::GenerateKeyRequest( sts = cdm_engine->GenerateKeyRequest(session_id, key_set_id, initialization_data, license_type, app_parameters, key_request); - switch (license_type) { - case kLicenseTypeRelease: - if (sts != KEY_MESSAGE) { - cdm_engine->CloseKeySetSession(key_set_id); - cdm_by_session_id_.erase(key_set_id); - } - break; - default: - if (sts == KEY_MESSAGE) EnablePolicyTimer(); - break; + if (license_type == kLicenseTypeRelease && sts != KEY_MESSAGE) { + cdm_engine->CloseKeySetSession(key_set_id); + cdm_by_session_id_.erase(key_set_id); } return sts; } @@ -165,7 +160,6 @@ CdmResponseType WvContentDecryptionModule::RestoreKey( if (!cdm_engine) return SESSION_NOT_FOUND_4; CdmResponseType sts; sts = cdm_engine->RestoreKey(session_id, key_set_id); - if (sts == KEY_ADDED) EnablePolicyTimer(); return sts; } @@ -355,21 +349,28 @@ WvContentDecryptionModule::CdmInfo::CdmInfo() CdmEngine* WvContentDecryptionModule::EnsureCdmForIdentifier( const CdmIdentifier& identifier) { - std::unique_lock auto_lock(cdms_lock_); - if (cdms_.find(identifier) == cdms_.end()) { - // Accessing the map entry will create a new instance using the default - // constructor. We then need to provide it with two pieces of info: The - // origin provided by the app and an identifier that uniquely identifies - // this CDM. We concatenate all pieces of the CdmIdentifier in order to - // create an ID that is unique to that identifier. - cdms_[identifier].file_system.set_origin(identifier.origin); - cdms_[identifier].file_system.set_identifier(identifier.spoid + - identifier.origin); - cdms_[identifier].cdm_engine->SetAppPackageName( - identifier.app_package_name); - cdms_[identifier].cdm_engine->SetSpoid(identifier.spoid); + CdmEngine* cdm_engine; + bool enable_timer = false; + { + std::unique_lock auto_lock(cdms_lock_); + if (cdms_.size() == 0) enable_timer = true; + if (cdms_.find(identifier) == cdms_.end()) { + // Accessing the map entry will create a new instance using the default + // constructor. We then need to provide it with two pieces of info: The + // origin provided by the app and an identifier that uniquely identifies + // this CDM. We concatenate all pieces of the CdmIdentifier in order to + // create an ID that is unique to that identifier. + cdms_[identifier].file_system.set_origin(identifier.origin); + cdms_[identifier].file_system.set_identifier(identifier.spoid + + identifier.origin); + cdms_[identifier].cdm_engine->SetAppPackageName( + identifier.app_package_name); + cdms_[identifier].cdm_engine->SetSpoid(identifier.spoid); + } + cdm_engine = cdms_[identifier].cdm_engine.get(); } - CdmEngine* cdm_engine = cdms_[identifier].cdm_engine.get(); + // Do not enable timer while holding on to the |cdms_lock_| + if (enable_timer) EnableTimer(); return cdm_engine; } @@ -392,35 +393,23 @@ void WvContentDecryptionModule::CloseAllCdms() { CdmResponseType WvContentDecryptionModule::CloseCdm( const CdmIdentifier& cdm_identifier) { - // The policy timer ultimately calls OnTimerEvent (which wants to - // acquire cdms_lock_). Therefore, we cannot acquire cdms_lock_ and then the - // policy_timer_lock_ (via DisablePolicyTimer) at the same time. - // Acquire the cdms_lock_ first, in its own scope. - bool cdms_empty = false; - { - std::unique_lock auto_lock(cdms_lock_); - auto it = cdms_.find(cdm_identifier); - if (it == cdms_.end()) { - LOGE("WVContentDecryptionModule::Close. cdm_identifier not found."); - // TODO(blueeyes): Create a better error. - return UNKNOWN_ERROR; - } - // Remove any sessions that point to this engine. - for (auto session_it = cdm_by_session_id_.begin(); - session_it != cdm_by_session_id_.end();) { - if (session_it->second == it->second.cdm_engine.get()) { - session_it = cdm_by_session_id_.erase(session_it); - } else { - ++session_it; - } - } - cdms_.erase(it); - cdms_empty = cdms_.empty(); + std::unique_lock auto_lock(cdms_lock_); + auto it = cdms_.find(cdm_identifier); + if (it == cdms_.end()) { + LOGE("WVContentDecryptionModule::Close. cdm_identifier not found."); + // TODO(blueeyes): Create a better error. + return UNKNOWN_ERROR; } - - if (cdms_empty) { - DisablePolicyTimer(); + // Remove any sessions that point to this engine. + for (auto session_it = cdm_by_session_id_.begin(); + session_it != cdm_by_session_id_.end();) { + if (session_it->second == it->second.cdm_engine.get()) { + session_it = cdm_by_session_id_.erase(session_it); + } else { + ++session_it; + } } + cdms_.erase(it); return NO_ERROR; } @@ -462,23 +451,60 @@ CdmResponseType WvContentDecryptionModule::GetDecryptHashError( return cdm_engine->GetDecryptHashError(session_id, hash_error_string); } -void WvContentDecryptionModule::EnablePolicyTimer() { - std::unique_lock auto_lock(policy_timer_lock_); - if (!policy_timer_.IsRunning()) - policy_timer_.Start(this, kCdmPolicyTimerDurationSeconds); +void WvContentDecryptionModule::EnableTimer() { + std::unique_lock auto_lock(timer_lock_); + if (!timer_.IsRunning()) timer_.Start(this, kCdmTimerDurationSeconds); } -void WvContentDecryptionModule::DisablePolicyTimer() { - std::unique_lock auto_lock(policy_timer_lock_); - if (policy_timer_.IsRunning()) { - policy_timer_.Stop(); +void WvContentDecryptionModule::DisableTimer() { + std::unique_lock auto_lock(timer_lock_); + if (timer_.IsRunning()) { + timer_.Stop(false); + } +} + +void WvContentDecryptionModule::DisableTimerAndWaitForExit() { + std::unique_lock auto_lock(timer_lock_); + if (timer_.IsRunning()) { + timer_.Stop(true); } } void WvContentDecryptionModule::OnTimerEvent() { - std::unique_lock auto_lock(cdms_lock_); - for (auto it = cdms_.begin(); it != cdms_.end(); ++it) { - it->second.cdm_engine->OnTimerEvent(); + bool disable_timer = false; + { + std::unique_lock auto_lock(cdms_lock_); + for (auto it = cdms_.begin(); it != cdms_.end(); ++it) { + it->second.cdm_engine->OnTimerEvent(); + } + if (cdms_.empty()) { + if (CryptoSession::TryTerminate()) { + // If CryptoSession is in a state to be terminated, try acquiring the + // |timer_lock_| before deciding whether to disable the timer. If the + // lock cannot be acquired, there is no need to disable the timer. + // The |timer_lock_| is either being held by + // * EnableTimer - This does not appear possible, but if it did + // happen |OnTimerEvent| will be called again. The timer can be + // disabled during a future call. + // * DisableTimer - If so, allow that call to disable the timer. No + // need to call to disable it here. + // * DisableTimerAndWaitForExit - If so, allow that call to disable + // the timer. No need to call to disable it here. Note that + // |DisableTimerAndWaitForExit| will try to stop the timer but + // wait for it to exit. This might kick off the timer thread one + // last time, which will call into OnTimerEvent. Calling + // |DisableTimer| here will result in deadlock. + if (timer_lock_.try_lock()) { + disable_timer = true; + timer_lock_.unlock(); + } + } + } + } + + // Release |cdms_lock_| before attempting to disable the timer + if (disable_timer) { + DisableTimer(); } } diff --git a/libwvdrmengine/cdm/test/timer_unittest.cpp b/libwvdrmengine/cdm/test/timer_unittest.cpp index 3cc636b5..b1c65ab6 100644 --- a/libwvdrmengine/cdm/test/timer_unittest.cpp +++ b/libwvdrmengine/cdm/test/timer_unittest.cpp @@ -8,17 +8,46 @@ namespace wvcdm { +namespace { + +// duration in seconds after starting the timer +constexpr uint32_t kTestDuration = 5; +// duration in seconds after test completes, before evaluating the result +constexpr uint32_t kPostTestDuration = 3; + +} // namespace + class TestTimerHandler : public TimerHandler { public: - TestTimerHandler() : timer_events_(0){}; + TestTimerHandler() : timer_events_count_(0){}; virtual ~TestTimerHandler(){}; - virtual void OnTimerEvent() { timer_events_++; } + virtual void OnTimerEvent() { timer_events_count_++; } - uint32_t timer_events() { return timer_events_; } + uint32_t timer_events_count() { return timer_events_count_; } - private: - uint32_t timer_events_; + protected: + uint32_t timer_events_count_; +}; + +class TestStopTimerHandler : public TestTimerHandler { + public: + TestStopTimerHandler(uint32_t stop_at_timer_events_count, Timer* timer) + : stop_at_timer_events_count_(stop_at_timer_events_count), + timer_(timer) {} + virtual ~TestStopTimerHandler() {} + + virtual void OnTimerEvent() { + if (++timer_events_count_ >= stop_at_timer_events_count_) { + if (timer_ != nullptr) { + timer_->Stop(false); + } + } + } + + protected: + uint32_t stop_at_timer_events_count_; + Timer* timer_; }; TEST(TimerTest, ParametersCheck) { @@ -29,25 +58,58 @@ TEST(TimerTest, ParametersCheck) { EXPECT_FALSE(timer.Start(&handler, 0)); } -TEST(TimerTest, TimerCheck) { +TEST(TimerTest, TimerCheck_StopAndWaitForExit) { TestTimerHandler handler; Timer timer; - uint32_t duration = 10; - EXPECT_EQ(0u, handler.timer_events()); + EXPECT_EQ(0u, handler.timer_events_count()); EXPECT_FALSE(timer.IsRunning()); EXPECT_TRUE(timer.Start(&handler, 1)); EXPECT_TRUE(timer.IsRunning()); - sleep(duration); + sleep(kTestDuration); - EXPECT_LE(duration - 1, handler.timer_events()); - EXPECT_LE(handler.timer_events(), duration + 1); - timer.Stop(); + EXPECT_LE(kTestDuration - 1, handler.timer_events_count()); + EXPECT_LE(handler.timer_events_count(), kTestDuration + 1); + timer.Stop(true); EXPECT_FALSE(timer.IsRunning()); - sleep(duration); + sleep(kPostTestDuration); - EXPECT_LE(duration - 1, handler.timer_events()); - EXPECT_LE(handler.timer_events(), duration + 1); + EXPECT_LE(kTestDuration - 1, handler.timer_events_count()); + EXPECT_LE(handler.timer_events_count(), kTestDuration + 1); } + +TEST(TimerTest, TimerCheck_Stop) { + TestTimerHandler handler; + Timer timer; + + EXPECT_EQ(0u, handler.timer_events_count()); + EXPECT_FALSE(timer.IsRunning()); + + EXPECT_TRUE(timer.Start(&handler, 1)); + EXPECT_TRUE(timer.IsRunning()); + sleep(kTestDuration); + + EXPECT_LE(kTestDuration - 1, handler.timer_events_count()); + EXPECT_LE(handler.timer_events_count(), kTestDuration + 1); + timer.Stop(false); + sleep(kPostTestDuration); + + EXPECT_FALSE(timer.IsRunning()); +} + +TEST(TimerTest, StopOnTimerThread) { + Timer timer; + TestStopTimerHandler handler(kTestDuration - 2, &timer); + + EXPECT_EQ(0u, handler.timer_events_count()); + EXPECT_FALSE(timer.IsRunning()); + + EXPECT_TRUE(timer.Start(&handler, 1)); + EXPECT_TRUE(timer.IsRunning()); + sleep(kTestDuration + kPostTestDuration); + + EXPECT_FALSE(timer.IsRunning()); +} + } // namespace wvcdm