diff options
author | Edwin Wong <edwinwong@google.com> | 2021-04-06 21:49:14 +0000 |
---|---|---|
committer | Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com> | 2021-04-06 21:49:14 +0000 |
commit | 6effd16a8b75cac0bd42e0639ebccc3f8b5d214a (patch) | |
tree | 1fcd877fcbcdbf827c4335b1ec8860d479053e81 /drm | |
parent | 217962707889edcd9cc40080a3b3ab1cb3e95c0a (diff) | |
parent | a4e76aab230a565dd0cef11e2e6e2d782b685327 (diff) |
[RESTRICT AUTOMERGE] Fix CryptoPlugin use after free vulnerability. am: a4e76aab23
Original change: https://googleplex-android-review.googlesource.com/c/platform/hardware/interfaces/+/13812193
Change-Id: I59565fec934a5be32b5c5f32b6586965a7d9a932
Diffstat (limited to 'drm')
-rw-r--r-- | drm/1.0/default/Android.bp | 3 | ||||
-rw-r--r-- | drm/1.0/default/CryptoPlugin.cpp | 8 | ||||
-rw-r--r-- | drm/1.0/default/CryptoPlugin.h | 21 |
3 files changed, 22 insertions, 10 deletions
diff --git a/drm/1.0/default/Android.bp b/drm/1.0/default/Android.bp index ed6bcdeee6..1122e46a05 100644 --- a/drm/1.0/default/Android.bp +++ b/drm/1.0/default/Android.bp @@ -9,6 +9,7 @@ cc_library_static { "-Werror", "-Wextra", "-Wall", + "-Wthread-safety", ], shared_libs: [ "liblog", @@ -19,5 +20,5 @@ cc_library_static { export_header_lib_headers: [ "libutils_headers", ], - export_include_dirs : ["include"] + export_include_dirs: ["include"], } diff --git a/drm/1.0/default/CryptoPlugin.cpp b/drm/1.0/default/CryptoPlugin.cpp index 17a2f241b4..8dea7e9324 100644 --- a/drm/1.0/default/CryptoPlugin.cpp +++ b/drm/1.0/default/CryptoPlugin.cpp @@ -53,6 +53,8 @@ namespace implementation { uint32_t bufferId) { sp<IMemory> hidlMemory = mapMemory(base); + std::lock_guard<std::mutex> shared_buffer_lock(mSharedBufferLock); + // allow mapMemory to return nullptr mSharedBufferMap[bufferId] = hidlMemory; return Void(); @@ -65,7 +67,7 @@ namespace implementation { const SharedBuffer& source, uint64_t offset, const DestinationBuffer& destination, decrypt_cb _hidl_cb) { - + std::unique_lock<std::mutex> shared_buffer_lock(mSharedBufferLock); if (mSharedBufferMap.find(source.bufferId) == mSharedBufferMap.end()) { _hidl_cb(Status::ERROR_DRM_CANNOT_HANDLE, 0, "source decrypt buffer base not set"); return Void(); @@ -173,6 +175,10 @@ namespace implementation { _hidl_cb(Status::BAD_VALUE, 0, "invalid destination type"); return Void(); } + + // release mSharedBufferLock + shared_buffer_lock.unlock(); + ssize_t result = mLegacyPlugin->decrypt(secure, keyId.data(), iv.data(), legacyMode, legacyPattern, srcPtr, legacySubSamples.get(), subSamples.size(), destPtr, &detailMessage); diff --git a/drm/1.0/default/CryptoPlugin.h b/drm/1.0/default/CryptoPlugin.h index 11cc2aae47..0d091fae65 100644 --- a/drm/1.0/default/CryptoPlugin.h +++ b/drm/1.0/default/CryptoPlugin.h @@ -17,11 +17,14 @@ #ifndef ANDROID_HARDWARE_DRM_V1_0__CRYPTOPLUGIN_H #define ANDROID_HARDWARE_DRM_V1_0__CRYPTOPLUGIN_H -#include <android/hidl/memory/1.0/IMemory.h> +#include <android-base/thread_annotations.h> #include <android/hardware/drm/1.0/ICryptoPlugin.h> +#include <android/hidl/memory/1.0/IMemory.h> #include <hidl/Status.h> #include <media/hardware/CryptoAPI.h> +#include <mutex> + namespace android { namespace hardware { namespace drm { @@ -60,19 +63,21 @@ struct CryptoPlugin : public ICryptoPlugin { Return<void> setSharedBufferBase(const ::android::hardware::hidl_memory& base, uint32_t bufferId) override; - Return<void> decrypt(bool secure, const hidl_array<uint8_t, 16>& keyId, - const hidl_array<uint8_t, 16>& iv, Mode mode, const Pattern& pattern, - const hidl_vec<SubSample>& subSamples, const SharedBuffer& source, - uint64_t offset, const DestinationBuffer& destination, - decrypt_cb _hidl_cb) override; + Return<void> decrypt( + bool secure, const hidl_array<uint8_t, 16>& keyId, const hidl_array<uint8_t, 16>& iv, + Mode mode, const Pattern& pattern, const hidl_vec<SubSample>& subSamples, + const SharedBuffer& source, uint64_t offset, const DestinationBuffer& destination, + decrypt_cb _hidl_cb) override NO_THREAD_SAFETY_ANALYSIS; // use unique_lock -private: + private: android::CryptoPlugin *mLegacyPlugin; - std::map<uint32_t, sp<IMemory> > mSharedBufferMap; + std::map<uint32_t, sp<IMemory>> mSharedBufferMap GUARDED_BY(mSharedBufferLock); CryptoPlugin() = delete; CryptoPlugin(const CryptoPlugin &) = delete; void operator=(const CryptoPlugin &) = delete; + + std::mutex mSharedBufferLock; }; } // namespace implementation |