summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorEdwin Wong <edwinwong@google.com>2021-01-22 20:38:33 -0800
committerEdwin Wong <edwinwong@google.com>2021-04-02 13:28:00 -0700
commitabb7ad47b00ae158eded8813801345d91d2b2671 (patch)
tree716ce02e739a132a3825b9231f1b8ed4c1f62316
parent6889557362e8d542c99d4138fa0c0a98e8ffb9a7 (diff)
[RESTRICT AUTOMERGE] Fix clearkey CryptoPlugin use after free vulnerability.
The shared memory buffer used by srcPtr can be freed by another thread because it is not protected by a mutex. Subsequently, a use after free AIGABRT can occur in a race condition. SafetyNet logging is not added to avoid log spamming. The mutex lock is called to setup for decryption, which is called frequently. The crash was reproduced on the device before the fix. Verified the test passes after the fix. Test: sts sts-tradefed run sts-engbuild-no-spl-lock -m StsHostTestCases --test android.security.sts.Bug_176495665#testPocBug_176495665 Test: push to device with target_hwasan-userdebug build adb shell /data/local/tmp/Bug-176495665_sts64 Bug: 176495665 Bug: 176444161 Change-Id: I2094ab904a34104089f29a219596e61a7317e00b
-rw-r--r--drm/mediadrm/plugins/clearkey/hidl/CryptoPlugin.cpp6
-rw-r--r--drm/mediadrm/plugins/clearkey/hidl/include/CryptoPlugin.h7
2 files changed, 11 insertions, 2 deletions
diff --git a/drm/mediadrm/plugins/clearkey/hidl/CryptoPlugin.cpp b/drm/mediadrm/plugins/clearkey/hidl/CryptoPlugin.cpp
index 005e551383..302dd39ddc 100644
--- a/drm/mediadrm/plugins/clearkey/hidl/CryptoPlugin.cpp
+++ b/drm/mediadrm/plugins/clearkey/hidl/CryptoPlugin.cpp
@@ -37,6 +37,8 @@ Return<void> CryptoPlugin::setSharedBufferBase(
sp<IMemory> hidlMemory = mapMemory(base);
ALOGE_IF(hidlMemory == nullptr, "mapMemory returns nullptr");
+ std::lock_guard<std::mutex> shared_buffer_lock(mSharedBufferLock);
+
// allow mapMemory to return nullptr
mSharedBufferMap[bufferId] = hidlMemory;
return Void();
@@ -94,6 +96,7 @@ Return<void> CryptoPlugin::decrypt_1_2(
return Void();
}
+ std::unique_lock<std::mutex> shared_buffer_lock(mSharedBufferLock);
if (mSharedBufferMap.find(source.bufferId) == mSharedBufferMap.end()) {
_hidl_cb(Status_V1_2::ERROR_DRM_CANNOT_HANDLE, 0,
"source decrypt buffer base not set");
@@ -151,6 +154,9 @@ Return<void> CryptoPlugin::decrypt_1_2(
}
destPtr = static_cast<void*>(base + destination.nonsecureMemory.offset);
+ // release mSharedBufferLock
+ shared_buffer_lock.unlock();
+
// Calculate the output buffer size and determine if any subsamples are
// encrypted.
size_t destSize = 0;
diff --git a/drm/mediadrm/plugins/clearkey/hidl/include/CryptoPlugin.h b/drm/mediadrm/plugins/clearkey/hidl/include/CryptoPlugin.h
index 8680f0ca54..23a64fac50 100644
--- a/drm/mediadrm/plugins/clearkey/hidl/include/CryptoPlugin.h
+++ b/drm/mediadrm/plugins/clearkey/hidl/include/CryptoPlugin.h
@@ -20,6 +20,8 @@
#include <android/hardware/drm/1.2/ICryptoPlugin.h>
#include <android/hidl/memory/1.0/IMemory.h>
+#include <mutex>
+
#include "ClearKeyTypes.h"
#include "Session.h"
#include "Utils.h"
@@ -93,7 +95,7 @@ struct CryptoPlugin : public drm::V1_2::ICryptoPlugin {
const SharedBuffer& source,
uint64_t offset,
const DestinationBuffer& destination,
- decrypt_1_2_cb _hidl_cb);
+ decrypt_1_2_cb _hidl_cb) NO_THREAD_SAFETY_ANALYSIS; // use unique_lock
Return<void> setSharedBufferBase(const hidl_memory& base,
uint32_t bufferId);
@@ -105,7 +107,8 @@ struct CryptoPlugin : public drm::V1_2::ICryptoPlugin {
private:
CLEARKEY_DISALLOW_COPY_AND_ASSIGN(CryptoPlugin);
- std::map<uint32_t, sp<IMemory> > mSharedBufferMap;
+ std::mutex mSharedBufferLock;
+ std::map<uint32_t, sp<IMemory>> mSharedBufferMap GUARDED_BY(mSharedBufferLock);
sp<Session> mSession;
Status mInitStatus;
};