From de78ca82d8ce231b9c9bf63ec20d6e9a89aa4788 Mon Sep 17 00:00:00 2001
From: Edwin Wong <edwinwong@google.com>
Date: Wed, 7 Apr 2021 22:49:41 +0000
Subject: [PATCH] Revert "[RESTRICT AUTOMERGE] Fix WVCryptoPlugin use after
 free vulnerability."

This reverts commit 93dd56bae2d433f0f330cef49613219ea27a8bd2.

Reason for revert: <missing thread_annotation header>


[RESTRICT AUTOMERGE] Fix WVCryptoPlugin 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: Ieadd74a3cae024184b1a73ba8f3a640af74f1cf1
---
 libwvdrmengine/mediacrypto/Android.mk                    | 2 --
 libwvdrmengine/mediacrypto/include_hidl/WVCryptoPlugin.h | 8 ++------
 libwvdrmengine/mediacrypto/src_hidl/WVCryptoPlugin.cpp   | 7 +------
 3 files changed, 3 insertions(+), 14 deletions(-)

diff --git a/libwvdrmengine/mediacrypto/Android.mk b/libwvdrmengine/mediacrypto/Android.mk
index 80c3470f..5855cbf1 100644
--- a/libwvdrmengine/mediacrypto/Android.mk
+++ b/libwvdrmengine/mediacrypto/Android.mk
@@ -69,8 +69,6 @@ LOCAL_SHARED_LIBRARIES := \
   libhidlmemory \
   liblog
 
-LOCAL_CFLAGS := -Wthread-safety
-
 LOCAL_MODULE := libwvdrmcryptoplugin_hidl
 LOCAL_PROPRIETARY_MODULE := true
 
diff --git a/libwvdrmengine/mediacrypto/include_hidl/WVCryptoPlugin.h b/libwvdrmengine/mediacrypto/include_hidl/WVCryptoPlugin.h
index ba9bb880..ea6769cd 100644
--- a/libwvdrmengine/mediacrypto/include_hidl/WVCryptoPlugin.h
+++ b/libwvdrmengine/mediacrypto/include_hidl/WVCryptoPlugin.h
@@ -10,8 +10,6 @@
 #include <android/hardware/drm/1.0/ICryptoPlugin.h>
 #include <android/hidl/memory/1.0/IMemory.h>
 
-#include <mutex>
-
 #include "wv_content_decryption_module.h"
 #include "WVTypes.h"
 
@@ -62,13 +60,13 @@ struct WVCryptoPlugin : public ICryptoPlugin {
       const SharedBuffer& source,
       uint64_t offset,
       const DestinationBuffer& destination,
-      decrypt_cb _hidl_cb) override NO_THREAD_SAFETY_ANALYSIS; // use unique_lock
+      decrypt_cb _hidl_cb) override;
 
  private:
   WVDRM_DISALLOW_COPY_AND_ASSIGN_AND_NEW(WVCryptoPlugin);
 
   wvcdm::CdmSessionId mSessionId;
-  std::map<uint32_t, sp<IMemory> > mSharedBufferMap GUARDED_BY(mSharedBufferLock);
+  std::map<uint32_t, sp<IMemory> > mSharedBufferMap;
 
   sp<wvcdm::WvContentDecryptionModule> const mCDM;
 
@@ -78,8 +76,6 @@ struct WVCryptoPlugin : public ICryptoPlugin {
   static wvcdm::CdmResponseType countEncryptedBlocksInPatternedRange(
       size_t range, const Pattern& pattern, uint64_t* result);
   static void incrementIV(uint64_t increaseBy, std::vector<uint8_t>* ivPtr);
-
-  std::mutex mSharedBufferLock;
 };
 
 }  // namespace widevine
diff --git a/libwvdrmengine/mediacrypto/src_hidl/WVCryptoPlugin.cpp b/libwvdrmengine/mediacrypto/src_hidl/WVCryptoPlugin.cpp
index 44273a62..61846c01 100644
--- a/libwvdrmengine/mediacrypto/src_hidl/WVCryptoPlugin.cpp
+++ b/libwvdrmengine/mediacrypto/src_hidl/WVCryptoPlugin.cpp
@@ -100,8 +100,6 @@ Return<void> WVCryptoPlugin::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();
@@ -118,7 +116,7 @@ Return<void> WVCryptoPlugin::decrypt(
     uint64_t offset,
     const DestinationBuffer& destination,
     decrypt_cb _hidl_cb) {
-    std::unique_lock<std::mutex> lock(mSharedBufferLock);
+
     if (mSharedBufferMap.find(source.bufferId) == mSharedBufferMap.end()) {
       _hidl_cb(Status::ERROR_DRM_CANNOT_HANDLE, 0,
                "source decrypt buffer base not set");
@@ -186,9 +184,6 @@ Return<void> WVCryptoPlugin::decrypt(
     destPtr = static_cast<void *>(handle);
   }
 
-  // release mSharedBufferLock
-  lock.unlock();
-
   // Calculate the output buffer size and determine if any subsamples are
   // encrypted.
   size_t destSize = 0;