From 381f879ff728a72a9653fb84971a9ffe9a65c2ce Mon Sep 17 00:00:00 2001 From: Alex Dale Date: Thu, 7 Jul 2022 19:12:14 -0700 Subject: [PATCH] Added mutex protection for session_property_set_. [ Merge of http://go/wvgerrit/154575 ] [ Cherry-pick of http://ag/19498242 ] There is a rare race condition experienced by some Android devices where the a new client property set is being added while another is being removed. The C++ stl library does not provided thread protection by default. This CL adds a new mutex for the client property set map which prevents multiple threads accessing the property sets concurrently. Bug: 235238226 Test: GtsMediaTestCases on redfin Change-Id: I32cf11bfb1332295ba1245071102ff0adc35259d (cherry picked from commit aaa97a5d60fab6d4a8776937c8553e693f1153f7) (cherry picked from commit 6109ec6d66a25970517bb61b8b2e97dc72504a40) --- libwvdrmengine/cdm/core/include/properties.h | 1 + libwvdrmengine/cdm/core/src/properties.cpp | 11 +++++++++++ libwvdrmengine/cdm/src/properties_android.cpp | 5 ++++- 3 files changed, 16 insertions(+), 1 deletion(-) diff --git a/libwvdrmengine/cdm/core/include/properties.h b/libwvdrmengine/cdm/core/include/properties.h index 72b8e376..360cac5c 100644 --- a/libwvdrmengine/cdm/core/include/properties.h +++ b/libwvdrmengine/cdm/core/include/properties.h @@ -142,6 +142,7 @@ class Properties { static void InitOnce(); static std::mutex init_mutex_; + static std::mutex session_mutex_; static bool is_initialized_; static bool oem_crypto_use_secure_buffers_; static bool oem_crypto_use_fifo_; diff --git a/libwvdrmengine/cdm/core/src/properties.cpp b/libwvdrmengine/cdm/core/src/properties.cpp index 1eeb8e17..746a437b 100644 --- a/libwvdrmengine/cdm/core/src/properties.cpp +++ b/libwvdrmengine/cdm/core/src/properties.cpp @@ -12,7 +12,10 @@ const char* kSecurityLevelDirs[] = {"L1/", "L3/"}; } // namespace namespace wvcdm { +using UniqueLock = std::unique_lock; + std::mutex Properties::init_mutex_; +std::mutex Properties::session_mutex_; bool Properties::is_initialized_ = false; bool Properties::oem_crypto_use_secure_buffers_; bool Properties::oem_crypto_use_fifo_; @@ -26,6 +29,7 @@ std::unique_ptr Properties::session_property_set_; bool Properties::AddSessionPropertySet(const CdmSessionId& session_id, CdmClientPropertySet* property_set) { + UniqueLock lock(session_mutex_); if (!session_property_set_) { return false; } @@ -37,6 +41,7 @@ bool Properties::AddSessionPropertySet(const CdmSessionId& session_id, } bool Properties::RemoveSessionPropertySet(const CdmSessionId& session_id) { + UniqueLock lock(session_mutex_); if (!session_property_set_) { return false; } @@ -45,6 +50,7 @@ bool Properties::RemoveSessionPropertySet(const CdmSessionId& session_id) { CdmClientPropertySet* Properties::GetCdmClientPropertySet( const CdmSessionId& session_id) { + // Call must obtain |session_mutex_|. if (session_property_set_) { CdmClientPropertySetMap::iterator it = session_property_set_->find(session_id); @@ -57,6 +63,7 @@ CdmClientPropertySet* Properties::GetCdmClientPropertySet( bool Properties::GetApplicationId(const CdmSessionId& session_id, std::string* app_id) { + UniqueLock lock(session_mutex_); const CdmClientPropertySet* property_set = GetCdmClientPropertySet(session_id); if (property_set == nullptr) { @@ -68,6 +75,7 @@ bool Properties::GetApplicationId(const CdmSessionId& session_id, bool Properties::GetServiceCertificate(const CdmSessionId& session_id, std::string* service_certificate) { + UniqueLock lock(session_mutex_); const CdmClientPropertySet* property_set = GetCdmClientPropertySet(session_id); if (property_set == nullptr) { @@ -79,6 +87,7 @@ bool Properties::GetServiceCertificate(const CdmSessionId& session_id, bool Properties::SetServiceCertificate(const CdmSessionId& session_id, const std::string& service_certificate) { + UniqueLock lock(session_mutex_); CdmClientPropertySet* property_set = GetCdmClientPropertySet(session_id); if (property_set == nullptr) { return false; @@ -88,6 +97,7 @@ bool Properties::SetServiceCertificate(const CdmSessionId& session_id, } bool Properties::UsePrivacyMode(const CdmSessionId& session_id) { + UniqueLock lock(session_mutex_); const CdmClientPropertySet* property_set = GetCdmClientPropertySet(session_id); if (property_set == nullptr) { @@ -97,6 +107,7 @@ bool Properties::UsePrivacyMode(const CdmSessionId& session_id) { } uint32_t Properties::GetSessionSharingId(const CdmSessionId& session_id) { + UniqueLock lock(session_mutex_); const CdmClientPropertySet* property_set = GetCdmClientPropertySet(session_id); if (property_set == nullptr) { diff --git a/libwvdrmengine/cdm/src/properties_android.cpp b/libwvdrmengine/cdm/src/properties_android.cpp index 2fa4839f..21a61223 100644 --- a/libwvdrmengine/cdm/src/properties_android.cpp +++ b/libwvdrmengine/cdm/src/properties_android.cpp @@ -52,7 +52,10 @@ void Properties::InitOnce() { allow_restore_of_offline_licenses_with_release_ = kAllowRestoreOfflineLicenseWithRelease; delay_oem_crypto_termination_ = kPropertyDelayOemCryptoTermination; - session_property_set_.reset(new CdmClientPropertySetMap()); + { + std::unique_lock lock(session_mutex_); + session_property_set_.reset(new CdmClientPropertySetMap()); + } } bool Properties::GetCompanyName(std::string* company_name) {