Fix Undefined Behavior Around Enums

(This is a cherry-pick of http://go/wvgerrit/104184.)

UBSan has detected several places where our code tripped over what is
technically Undefined Behavior when handling enums, although in practice
any compiler would still generate safe code.

Some of these were places a variable was not being initialized and thus
was filled with garbage data. These have been fixed.

Understanding the rest depends on a bit of C++ trivia I had certainly
never heard before: An enum that doesn't specify its backing type will
frequently have a gap between the range of values the compiler will let
it take (which is limited only by the size of the backing type assigned
by the C++ standard) and the range of values for which the C++ standard
defines the behavior. (which is limited by the minimum number of bits
needed to hold the largest valid enumeration entry) So, for example, an
enum containing ten entries numbered 0 through 9 would be stored in
memory as an int and could thus take any value in the range of an int.
But it only takes 4 bits to represent the numbers 0 through 9. The
largest number that can be represented in 4 bits is 15. So reading the
value of a variable of this enum type when its stored value is outside
the range 0 to 15 is undefined behavior.

An enum that specifies its backing type is not subject to this because
it is defined behavior to access any value representable in the backing
type if one was explicitly specified.

If you think this sounds a bit silly, you'll be happy to know it doesn't
apply from C++17 onwards and most compilers generate code that handles
the undefined behavior values correctly.

Nonetheless, to appease UBSan and protect us from any compilers that
actually rely on this undefined behavior for optimizations, I have
defined backing types for all our enums. I have defaulted to the type
the compiler was already using (int32) and have deviated only where an
enum exists to be compared to or filled from a protobuf field and that
field in the protobuf is unsigned, in which case I used a uint32.

In the case of the CE CDM exported API, this also required changing our
enums from C-style to C++-style.

Bug: 163080356
Test: CE CDM Build & Unit Tests Pass even with UBSan
Test: Android Build & Tests
Change-Id: Id7e0064129e7c4d2827bb4a94825d144eeaacec8
This commit is contained in:
John W. Bruce
2020-08-18 18:02:36 -07:00
parent fe06541507
commit 7f028d25c8
2 changed files with 25 additions and 22 deletions

View File

@@ -34,7 +34,7 @@ using CdmProvisioningResponse = std::string;
using CdmUsageTableHeader = std::string;
using CdmUsageEntry = std::string;
enum CdmKeyRequestType {
enum CdmKeyRequestType : uint32_t {
kKeyRequestTypeUnknown,
kKeyRequestTypeInitial,
kKeyRequestTypeRenewal,
@@ -43,13 +43,13 @@ enum CdmKeyRequestType {
kKeyRequestTypeUpdate,
};
enum CdmOfflineLicenseState {
enum CdmOfflineLicenseState : int32_t {
kLicenseStateActive,
kLicenseStateReleasing,
kLicenseStateUnknown,
};
enum CdmResponseType {
enum CdmResponseType : int32_t {
NO_ERROR = 0,
UNKNOWN_ERROR = 1,
KEY_ADDED = 2,
@@ -421,7 +421,7 @@ enum CdmResponseType {
// * android/include_hidl/mapErrors-inl.h
};
enum CdmKeyStatus {
enum CdmKeyStatus : int32_t {
kKeyStatusKeyUnknown,
kKeyStatusUsable,
kKeyStatusExpired,
@@ -432,7 +432,7 @@ enum CdmKeyStatus {
};
using CdmKeyStatusMap = std::map<KeyId, CdmKeyStatus>;
enum CdmLicenseType {
enum CdmLicenseType : int32_t {
kLicenseTypeOffline,
kLicenseTypeStreaming,
kLicenseTypeRelease,
@@ -446,48 +446,51 @@ enum CdmLicenseType {
// on code review comments from go/wvgerrit/41860 this license type should not
// be added. This type can be removed once it is no longer needed by
// GenerateKeyRequest.
kLicenseTypeEmbeddedKeyData
kLicenseTypeEmbeddedKeyData,
};
enum CdmLicenseKeyType { kLicenseKeyTypeContent, kLicenseKeyTypeEntitlement };
enum CdmLicenseKeyType : int32_t {
kLicenseKeyTypeContent,
kLicenseKeyTypeEntitlement,
};
enum SecurityLevel { kLevelDefault, kLevel3 };
enum SecurityLevel : uint32_t { kLevelDefault, kLevel3 };
enum CdmSecurityLevel {
enum CdmSecurityLevel : int32_t {
kSecurityLevelUninitialized,
kSecurityLevelL1,
kSecurityLevelL2,
kSecurityLevelL3,
kSecurityLevelUnknown
kSecurityLevelUnknown,
};
enum CdmCertificateType {
enum CdmCertificateType : int32_t {
kCertificateWidevine,
kCertificateX509,
};
enum CdmHlsMethod {
enum CdmHlsMethod : int32_t {
kHlsMethodNone,
kHlsMethodAes128,
kHlsMethodSampleAes,
};
enum CdmCipherMode {
enum CdmCipherMode : int32_t {
kCipherModeCtr,
kCipherModeCbc,
};
enum CdmEncryptionAlgorithm {
enum CdmEncryptionAlgorithm : int32_t {
kEncryptionAlgorithmUnknown,
kEncryptionAlgorithmAesCbc128
kEncryptionAlgorithmAesCbc128,
};
enum CdmSigningAlgorithm {
enum CdmSigningAlgorithm : int32_t {
kSigningAlgorithmUnknown,
kSigningAlgorithmHmacSha256
kSigningAlgorithmHmacSha256,
};
enum CdmClientTokenType {
enum CdmClientTokenType : int32_t {
kClientTokenKeybox,
kClientTokenDrmCert,
kClientTokenOemCert,
@@ -500,13 +503,13 @@ enum CdmClientTokenType {
// persisted in non-secure storage but are loaded and unloaded from
// the TEE during use (OEMCrypto v13+)
// kUnknownUsageSupport - usage support type is uninitialized or unavailable
enum CdmUsageSupportType {
enum CdmUsageSupportType : int32_t {
kNonSecureUsageSupport,
kUsageEntrySupport,
kUnknownUsageSupport,
};
enum CdmUsageEntryStorageType {
enum CdmUsageEntryStorageType : int32_t {
kStorageLicense,
kStorageUsageInfo,
kStorageTypeUnknown,
@@ -546,7 +549,7 @@ struct CdmUsageEntryInfo {
}
};
enum CdmKeySecurityLevel {
enum CdmKeySecurityLevel : int32_t {
kKeySecurityLevelUnset,
kSoftwareSecureCrypto,
kSoftwareSecureDecode,

View File

@@ -233,7 +233,7 @@ TEST_F(WvCdmEngineMetricsImplTest, GenerateKeyRequest) {
}
TEST_F(WvCdmEngineMetricsImplTest, AddKey) {
CdmLicenseType license_type;
CdmLicenseType license_type = {};
CdmKeySetId key_set_id;
EXPECT_CALL(*test_cdm_metrics_engine_,