Finer-Grained OEMCrypto Locking

(This is a merge of http://go/wvgerrit/72867)

This patch replaces the previous static std::mutexes in CryptoSession
with shared_mutexes, allowing multiple readers to access the resources
they protect. For the shared fields, this means only Initialize(),
Terminate(), and the code that sets up the usage table headers needs
exclusive access. All other CryptoSession code is able to read these
fields in parallel.

For OEMCrypto, the static OEMCrypto lock is joined by a per-session
std::mutex, which are used in concert to enforce the OEMCrypto v15
threading guarantees.

On my machine this results in a noticeable increase in performance for
the parallel unit tests.

Bug: 70889998
Bug: 118584039
Test: CE CDM Unit Tests
Test: Android Unit Tests
Test: Jenkins Tests
Change-Id: Ie6332ae4926ed4f14af897685d37bfe63831b14f
This commit is contained in:
John W. Bruce
2019-02-19 13:58:07 -08:00
parent d925048c35
commit e10ac3b465
2 changed files with 181 additions and 118 deletions

View File

@@ -17,6 +17,7 @@
#include "key_session.h"
#include "metrics_collections.h"
#include "oemcrypto_adapter.h"
#include "rw_lock.h"
#include "timer_metric.h"
#include "wv_cdm_types.h"
@@ -256,7 +257,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_);
std::unique_lock<std::mutex> auto_lock(factory_mutex_);
factory_.reset(factory);
}
@@ -311,25 +312,59 @@ 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.
// These methods should be used to take the various CryptoSession mutexes in
// preference to taking the mutexes 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.)
// A lock should be taken on the Static Field Mutex before accessing any of
// CryptoSession's non-atomic static fields. It can be taken as a reader or as
// a writer, depending on how you will be accessing the static fields.
//
// Before calling into OEMCrypto, code must take locks on the OEMCrypto Mutex
// and/or the OEMCrypto Session Mutex. Which of them should be taken and how
// depends on the OEMCrypto function being called; consult the OEMCrypto
// specification's threading guarantees before making any calls. The OEMCrypto
// specification defines several classes of functions for the purposes of
// parallelism. The methods below lock the OEMCrypto Mutex and OEMCrypto
// Session Mutex in the correct order and manner to fulfill the guarantees in
// the specification.
//
// For this function class... | ...use this locking method
// ------------------------------+---------------------------
// Initialization & Termination | WithOecWriteLock()
// Property | WithOecReadLock()
// Session Initialization | WithOecWriteLock()
// Usage Table | WithOecWriteLock()
// Session | WithOecSessionLock()
//
// Note that accessing |key_session_| often accesses the OEMCrypto session, so
// WithOecSessionLock() should be used before accessing |key_session_| as
// well.
//
// If a function needs to take a lock on both the Static Field Mutex and some
// of the OEMCrypto mutexes simultaneously, it must *always* lock the Static
// Field Mutex before the OEMCrypto mutexes.
//
// In general, all locks should only be held for the minimum time necessary
// (e.g. a lock on the OEMCrypto mutexes 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)
static auto WithStaticFieldWriteLock(const char* tag, Func body)
-> decltype(body());
template <class Func>
static auto WithOecLock(const char* tag, Func body) -> decltype(body());
static auto WithStaticFieldReadLock(const char* tag, Func body)
-> decltype(body());
template <class Func>
static auto WithOecWriteLock(const char* tag, Func body) -> decltype(body());
template <class Func>
static auto WithOecReadLock(const char* tag, Func body) -> decltype(body());
template <class Func>
auto WithOecSessionLock(const char* tag, Func body) -> decltype(body());
static bool IsInitialized();
@@ -337,11 +372,12 @@ class CryptoSession {
static const size_t kAes128BlockSize = 16; // Block size for AES_CBC_128
static const size_t kSignatureSize = 32; // size for HMAC-SHA256 signature
// 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_;
// The locking methods above should be used in preference to taking these
// mutexes directly. If code takes these manually and needs to take more
// than one, it must *always* take them in the order they are defined here.
static shared_mutex static_field_mutex_;
static shared_mutex oem_crypto_mutex_;
std::mutex oem_crypto_session_mutex_;
static bool initialized_;
static int session_count_;
@@ -374,9 +410,9 @@ class CryptoSession {
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_;
// of the CryptoSession static mutexes, |factory_| is protected by its own
// mutex that is only used in the two funtions that interact with it.
static std::mutex factory_mutex_;
static std::unique_ptr<CryptoSessionFactory> factory_;
CORE_DISALLOW_COPY_AND_ASSIGN(CryptoSession);