Split CryptoSession Lock into Three
(This is a merge of http://go/wvgerrit/71324) This patch increases the granularity of the locking in CryptoSession without substantially changing its locking semantics. Where before there was a single |crypto_lock_| performing multiple duties, now there are three locks: 1) |static_field_lock_|, which is used when needing to access the non-atomic static member fields of CryptoSession. 2) |oem_crypto_lock_|, which is used when needing to call into OEMCrypto. 3) |factory_lock_|, used only by the functions that interact with the CryptoSession factory. All the code in CryptoSession has been updated to use these locks. It has also been updated to only hold them for the minimal amount of time necessary, as opposed to holding them for a whole function. This should help some with the ability of CryptoSession calls to happen concurrently. To assist in taking locks in a consistent manner, two helper functions, |WithStaticFieldLock()| and |WithOecLock()| have been added. Also, for the very common case of reading |initialized_|, the accessor |IsInitialized()| will read the value safely. While changing all the code to lock differently, I found that some places in CryptoSession were *not* locking before accessing static state or calling into OEMCrypto. I have made these callsites consistent with the rest of CryptoSession. As a result of taking locks for only the minimum time necessary, it is no longer necessary for functions to make assumptions about whether the lock will already be held before they are called. Locks should not be held while calling helper functions, and code should always take a lock for the brief time it is necessary to do so. In tests, including the concurrent unit tests coming in the following patch, this code did not perform substantially better or worse than the code that preceded it, but the hope is that it will experience less contention on devices that are more resource-constrained than my desktop, such as older game consoles. This patch appears to address some real threading issues. Hopefully, it will also make it easier to maintain soundness in the future and to reason about when code in CryptoSession needs to take a lock. This is the first step to implementing the "Finer-Grained Locking in CryptoSession" specification. A future patch will make some of these locks reader-writer locks, to allow even greater parallelism. Bug: 70889998 Bug: 118584039 Bug: 123319961 Test: CE CDM Unit Tests Test: Android Unit Tests Test: GTS Test: Play Movies Test: Netflix Change-Id: I346c04a5d9875723db54af33ee91772bf49ca12f
This commit is contained in:
@@ -158,7 +158,7 @@ class CryptoSession {
|
||||
HdcpCapability* max);
|
||||
virtual CdmResponseType GetHdcpCapabilities(SecurityLevel security_level,
|
||||
HdcpCapability* current,
|
||||
HdcpCapability* max);
|
||||
HdcpCapability* max);
|
||||
virtual bool GetResourceRatingTier(uint32_t* tier);
|
||||
virtual bool GetResourceRatingTier(SecurityLevel security_level,
|
||||
uint32_t* tier);
|
||||
@@ -180,11 +180,10 @@ class CryptoSession {
|
||||
virtual uint32_t IsDecryptHashSupported(SecurityLevel security_level);
|
||||
|
||||
virtual CdmResponseType SetDecryptHash(uint32_t frame_number,
|
||||
const std::string& hash);
|
||||
const std::string& hash);
|
||||
|
||||
virtual CdmResponseType GetDecryptHashError(std::string* error_string);
|
||||
|
||||
|
||||
virtual CdmResponseType GenericEncrypt(const std::string& in_buffer,
|
||||
const std::string& key_id,
|
||||
const std::string& iv,
|
||||
@@ -257,6 +256,7 @@ class CryptoSession {
|
||||
// OEMCrypto to use a test keybox.
|
||||
// Ownership of the object is transfered to CryptoSession.
|
||||
static void SetCryptoSessionFactory(CryptoSessionFactory* factory) {
|
||||
std::unique_lock<std::mutex> auto_lock(factory_lock_);
|
||||
factory_.reset(factory);
|
||||
}
|
||||
|
||||
@@ -311,9 +311,38 @@ class CryptoSession {
|
||||
size_t max_chunk_size);
|
||||
static void IncrementIV(uint64_t increase_by, std::vector<uint8_t>* iv_out);
|
||||
|
||||
// These methods should be used to take the various CryptoSession locks in
|
||||
// preference to taking the locks directly.
|
||||
//
|
||||
// The Static Field Lock should be taken before accessing any of
|
||||
// CryptoSession's non-atomic static fields. The OEMCrypto Lock should be
|
||||
// taken before calling into OEMCrypto. Note that accessing |key_session_|
|
||||
// often accesses OEMCrypto, so this lock should be held before calling into
|
||||
// |key_session_| as well. If a function needs to take both locks
|
||||
// simultaneously, it must *always* take the Static Field Lock first. In
|
||||
// general, locks should only be held for the minimum time necessary.
|
||||
// (e.g. |oem_crypto_lock_| should only be held for the duration of a single
|
||||
// call into OEMCrypto, unless there is a compelling argument otherwise such
|
||||
// as making two calls into OEMCrypto immediately after each other.)
|
||||
template <class Func>
|
||||
static auto WithStaticFieldLock(const char* tag, Func body)
|
||||
-> decltype(body());
|
||||
|
||||
template <class Func>
|
||||
static auto WithOecLock(const char* tag, Func body) -> decltype(body());
|
||||
|
||||
static bool IsInitialized();
|
||||
|
||||
// Constants
|
||||
static const size_t kAes128BlockSize = 16; // Block size for AES_CBC_128
|
||||
static const size_t kSignatureSize = 32; // size for HMAC-SHA256 signature
|
||||
static std::mutex crypto_lock_;
|
||||
|
||||
// The |WithStaticFieldLock| and |WithOecLock| methods should be used in
|
||||
// preference to taking these locks directly. When taken, the rules of
|
||||
// ordering documented with those functions must still be upheld.
|
||||
static std::mutex static_field_lock_;
|
||||
static std::mutex oem_crypto_lock_;
|
||||
|
||||
static bool initialized_;
|
||||
static int session_count_;
|
||||
|
||||
@@ -344,6 +373,10 @@ class CryptoSession {
|
||||
CdmCipherMode cipher_mode_;
|
||||
uint32_t api_version_;
|
||||
|
||||
// In order to avoid creating a deadlock if instantiation needs to take any
|
||||
// of the CryptoSession static locks, |factory_| is protected by its own lock
|
||||
// that is only used in the two funtions that interact with it.
|
||||
static std::mutex factory_lock_;
|
||||
static std::unique_ptr<CryptoSessionFactory> factory_;
|
||||
|
||||
CORE_DISALLOW_COPY_AND_ASSIGN(CryptoSession);
|
||||
|
||||
Reference in New Issue
Block a user