summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorEdwin Wong <edwinwong@google.com>2021-02-04 00:03:08 -0800
committerEdwin Wong <edwinwong@google.com>2021-02-04 22:20:17 +0000
commit3c73391f6e8c4d28f4ed3d1fc7379acdebaa6fa6 (patch)
tree316fe13daf0dde6fa1fff702b136fbf9a817ae46
parent4b86f1dbae158737cd05aa4d5b5634985534cfc3 (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.cpp1
-rw-r--r--drm/mediadrm/plugins/clearkey/default/include/DrmPlugin.h2
-rw-r--r--drm/mediadrm/plugins/clearkey/hidl/DrmPlugin.cpp3
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();
}