diff options
author | Edwin Wong <edwinwong@google.com> | 2021-02-04 00:03:08 -0800 |
---|---|---|
committer | Edwin Wong <edwinwong@google.com> | 2021-02-04 22:20:17 +0000 |
commit | 3c73391f6e8c4d28f4ed3d1fc7379acdebaa6fa6 (patch) | |
tree | 316fe13daf0dde6fa1fff702b136fbf9a817ae46 | |
parent | 4b86f1dbae158737cd05aa4d5b5634985534cfc3 (diff) |
Fix possible uaf of play policy state
Access to the play policy state may happen after
the state is freed in a race condition, which will
result in a SIGARBT.
SafetyNet logging is not added to avoid log spamming.
queryKeyStatus can be called often.
The crash was reproduced on the device before the fix.
Verified the test passes after the fix.
Test: sts-tradefed
sts-tradefed run sts-engbuild-no-spl-lock -m StsHostTestCases --test android.security.sts.Bug_176486806#testPocBug_176486806
Test: push to device with target_hwasan-userdebug build
adb shell /data/local/tmp/Bug-17648680664
Test: sts-tradefed
sts-tradefed run sts-engbuild-no-spl-lock -m StsHostTestCases --test android.security.sts.Bug_176444154#testPocBug_176444154
Test: push to device with target_hwasan-userdebug build
adb shell /data/local/tmp/Bug-17644415464
Bug: 176444154
Bug: 176486806
Change-Id: I07cc93c255942d56e866d0b08fb786f154f6e0d3
Merged-In: I07cc93c255942d56e866d0b08fb786f154f6e0d3
-rw-r--r-- | drm/mediadrm/plugins/clearkey/default/DrmPlugin.cpp | 1 | ||||
-rw-r--r-- | drm/mediadrm/plugins/clearkey/default/include/DrmPlugin.h | 2 | ||||
-rw-r--r-- | drm/mediadrm/plugins/clearkey/hidl/DrmPlugin.cpp | 3 |
3 files changed, 4 insertions, 2 deletions
diff --git a/drm/mediadrm/plugins/clearkey/default/DrmPlugin.cpp b/drm/mediadrm/plugins/clearkey/default/DrmPlugin.cpp index 6ac3510c7c..089eb1cdc9 100644 --- a/drm/mediadrm/plugins/clearkey/default/DrmPlugin.cpp +++ b/drm/mediadrm/plugins/clearkey/default/DrmPlugin.cpp @@ -207,6 +207,7 @@ status_t DrmPlugin::queryKeyStatus( } infoMap.clear(); + android::Mutex::Autolock lock(mPlayPolicyLock); for (size_t i = 0; i < mPlayPolicy.size(); ++i) { infoMap.add(mPlayPolicy.keyAt(i), mPlayPolicy.valueAt(i)); } diff --git a/drm/mediadrm/plugins/clearkey/default/include/DrmPlugin.h b/drm/mediadrm/plugins/clearkey/default/include/DrmPlugin.h index aa9b59ddbb..95f15caffe 100644 --- a/drm/mediadrm/plugins/clearkey/default/include/DrmPlugin.h +++ b/drm/mediadrm/plugins/clearkey/default/include/DrmPlugin.h @@ -262,7 +262,7 @@ private: void initProperties(); void setPlayPolicy(); - android::Mutex mPlayPolicyLock; + mutable android::Mutex mPlayPolicyLock; android::KeyedVector<String8, String8> mPlayPolicy; android::KeyedVector<String8, String8> mStringProperties; android::KeyedVector<String8, Vector<uint8_t>> mByteArrayProperties; diff --git a/drm/mediadrm/plugins/clearkey/hidl/DrmPlugin.cpp b/drm/mediadrm/plugins/clearkey/hidl/DrmPlugin.cpp index 15ebf78621..1ae4078288 100644 --- a/drm/mediadrm/plugins/clearkey/hidl/DrmPlugin.cpp +++ b/drm/mediadrm/plugins/clearkey/hidl/DrmPlugin.cpp @@ -563,7 +563,6 @@ Return<Status> DrmPlugin::setPropertyByteArray( Return<void> DrmPlugin::queryKeyStatus( const hidl_vec<uint8_t>& sessionId, queryKeyStatus_cb _hidl_cb) { - if (sessionId.size() == 0) { // Returns empty key status KeyValue pair _hidl_cb(Status::BAD_VALUE, hidl_vec<KeyValue>()); @@ -573,12 +572,14 @@ Return<void> DrmPlugin::queryKeyStatus( std::vector<KeyValue> infoMapVec; infoMapVec.clear(); + mPlayPolicyLock.lock(); KeyValue keyValuePair; for (size_t i = 0; i < mPlayPolicy.size(); ++i) { keyValuePair.key = mPlayPolicy[i].key; keyValuePair.value = mPlayPolicy[i].value; infoMapVec.push_back(keyValuePair); } + mPlayPolicyLock.unlock(); _hidl_cb(Status::OK, toHidlVec(infoMapVec)); return Void(); } |