widevine-partner/android
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Usage Table Failure Creates Broken CryptoSession

Browse Source 
(This is a merge of http://go/wvgerrit/86383)

When Key Sessions were added to CryptoSession, the initialization of the
initial Key Session was placed at the end of the initialization of the
owning CryptoSession. That's all well and good except the block right
before that assumed that it was safe to abort initialization early in
order to swallow errors when setting up usage tables. As a result, if
anything caused usage table initialization to fail, it would leave the
CryptoSession without a Key Session, resulting in an inevitable segfault
further down the line.

There's no reason the Key Session can't be initialized first. This
change moves initialziation order around to avoid the bug.

Bug: 141021960
Test: CE CDM Unit Tests
Test: Android Unit Tests
Change-Id: Ic78005c831d2a24d7d6de22df54462b2bd7085f0
This commit is contained in:
John W. Bruce
2019-09-25 18:02:56 -07:00
parent 314d5a8745
commit d7b17c469b
1 changed files with 23 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files

26
libwvdrmengine/cdm/core/src/crypto_session.cpp
View File

@@ -674,6 +674,7 @@ CdmResponseType CryptoSession::Open(SecurityLevel requested_security_level) {
return LOAD_SYSTEM_ID_ERROR; return LOAD_SYSTEM_ID_ERROR;
} }
// Set up request ID
uint64_t request_id_base; uint64_t request_id_base;
OEMCryptoResult random_sts; OEMCryptoResult random_sts;
WithOecReadLock("Open() calling OEMCrypto_GetRandom", [&] { WithOecReadLock("Open() calling OEMCrypto_GetRandom", [&] {
@@ -688,6 +689,22 @@ CdmResponseType CryptoSession::Open(SecurityLevel requested_security_level) {
HexEncode(reinterpret_cast<uint8_t*>(&request_id_index), HexEncode(reinterpret_cast<uint8_t*>(&request_id_index),
sizeof(request_id_index)); sizeof(request_id_index));
// Initialize key session
WithOecSessionLock("Open() calling key_session_.reset()", [&] {
key_session_.reset(new ContentKeySession(oec_session_id_, metrics_));
});
// Set up Usage Table support
//
// This MUST be the last thing in the function because it contains the
// successful return paths of the function. It will attempt to initialize
// usage tables. However, whether they are employed is determined by
// information in the license, which will not be received until later. In case
// usage tables are not even needed, initialization errors here are ignored by
// returning successfully.
//
// TODO(b/141350978): Refactor this code so that it does not have this
// problem.
if (!GetApiVersion(&api_version_)) { if (!GetApiVersion(&api_version_)) {
LOGE("Failed to get API version"); LOGE("Failed to get API version");
return USAGE_SUPPORT_GET_API_FAILED; return USAGE_SUPPORT_GET_API_FAILED;
@@ -735,9 +752,12 @@ CdmResponseType CryptoSession::Open(SecurityLevel requested_security_level) {
metrics_->oemcrypto_usage_table_support_.SetError(result); metrics_->oemcrypto_usage_table_support_.SetError(result);
} }
WithOecSessionLock("Open() calling key_session_.reset()", [&] { // Do not add logic after this point as it may never get exercised. In the
key_session_.reset(new ContentKeySession(oec_session_id_, metrics_)); // event of errors initializing the usage tables, the code will return early,
}); // never reaching this point. See the comment above the "Set up Usage Table
// support" section for details.
//
// TODO(b/139973602): Refactor this code.
return NO_ERROR; return NO_ERROR;
} }