summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorEdwin Wong <edwinwong@google.com>2021-02-04 00:03:08 -0800
committerEdwin Wong <edwinwong@google.com>2021-03-08 17:57:34 +0000
commite07417a9b7829cfb32505947f700fd8dad9e12e6 (patch)
treea9266ade5438f4da5156e60b23498946d41d51b2
parent23a3c612893597b2c1d6507ef9c94ae25f8cdcbe (diff)
[RESTRICT AUTOMERGE] 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-176486806_sts64 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-176444154_sts64 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 a77759eaef..97a62be7f1 100644
--- a/drm/mediadrm/plugins/clearkey/hidl/DrmPlugin.cpp
+++ b/drm/mediadrm/plugins/clearkey/hidl/DrmPlugin.cpp
@@ -576,7 +576,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>());
@@ -586,12 +585,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();
}