commit addcb3aa51f67c8c4f2959ee6f022fdb9473859c
author Chong Zhang <chz@google.com> Wed Mar 21 15:52:21 2018 -0700
committer Chong Zhang <chz@google.com> Sat Mar 24 01:46:21 2018 +0000
tree 4d966d9bb70f9b39f58b625421c1635bcd2c689d
parent 8ca409dfa17b5961be671134a12362c862168a51

cas: fix UAF in descrambler -- DO NOT MERGE

Change the plugin holder in both CasImpl and DescramblerImpl
to shared_ptr, and use atomic store/load for read/write.

bug: 73172817
Test:CTS MediaCasTest, VTS VtsHalCasV1_0Target, poc in bug
Change-Id: I6b040680c28c38cef2fef3042f570dc64e86bb77

cas/1.0/default/DescramblerImpl.cpp

diff --git a/cas/1.0/default/DescramblerImpl.cpp b/cas/1.0/default/DescramblerImpl.cpp
index 36699ba..1f89933 100644
--- a/cas/1.0/default/DescramblerImpl.cpp
+++ b/cas/1.0/default/DescramblerImpl.cpp
@@ -50,12 +50,12 @@
 
 DescramblerImpl::DescramblerImpl(
         const sp<SharedLibrary>& library, DescramblerPlugin *plugin) :
-        mLibrary(library), mPlugin(plugin) {
-    ALOGV("CTOR: mPlugin=%p", mPlugin);
+        mLibrary(library), mPluginHolder(plugin) {
+    ALOGV("CTOR: plugin=%p", mPluginHolder.get());
 }
 
 DescramblerImpl::~DescramblerImpl() {
-    ALOGV("DTOR: mPlugin=%p", mPlugin);
+    ALOGV("DTOR: plugin=%p", mPluginHolder.get());
     release();
 }
 
@@ -63,12 +63,22 @@
     ALOGV("%s: sessionId=%s", __FUNCTION__,
             sessionIdToString(sessionId).string());
 
-    return toStatus(mPlugin->setMediaCasSession(sessionId));
+    std::shared_ptr<DescramblerPlugin> holder = std::atomic_load(&mPluginHolder);
+    if (holder.get() == nullptr) {
+        return toStatus(INVALID_OPERATION);
+    }
+
+    return toStatus(holder->setMediaCasSession(sessionId));
 }
 
 Return<bool> DescramblerImpl::requiresSecureDecoderComponent(
         const hidl_string& mime) {
-    return mPlugin->requiresSecureDecoderComponent(String8(mime.c_str()));
+    std::shared_ptr<DescramblerPlugin> holder = std::atomic_load(&mPluginHolder);
+    if (holder.get() == nullptr) {
+        return false;
+    }
+
+    return holder->requiresSecureDecoderComponent(String8(mime.c_str()));
 }
 
 static inline bool validateRangeForSize(
@@ -86,12 +96,23 @@
         descramble_cb _hidl_cb) {
     ALOGV("%s", __FUNCTION__);
 
+    // Get a local copy of the shared_ptr for the plugin. Note that before
+    // calling the HIDL callback, this shared_ptr must be manually reset,
+    // since the client side could proceed as soon as the callback is called
+    // without waiting for this method to go out of scope.
+    std::shared_ptr<DescramblerPlugin> holder = std::atomic_load(&mPluginHolder);
+    if (holder.get() == nullptr) {
+        _hidl_cb(toStatus(INVALID_OPERATION), 0, NULL);
+        return Void();
+    }
+
     sp<IMemory> srcMem = mapMemory(srcBuffer.heapBase);
 
     // Validate if the offset and size in the SharedBuffer is consistent with the
     // mapped ashmem, since the offset and size is controlled by client.
     if (srcMem == NULL) {
         ALOGE("Failed to map src buffer.");
+        holder.reset();
         _hidl_cb(toStatus(BAD_VALUE), 0, NULL);
         return Void();
     }
@@ -100,6 +121,7 @@
         ALOGE("Invalid src buffer range: offset %llu, size %llu, srcMem size %llu",
                 srcBuffer.offset, srcBuffer.size, (uint64_t)srcMem->getSize());
         android_errorWriteLog(0x534e4554, "67962232");
+        holder.reset();
         _hidl_cb(toStatus(BAD_VALUE), 0, NULL);
         return Void();
     }
@@ -117,6 +139,7 @@
                 "srcOffset %llu, totalBytesInSubSamples %llu, srcBuffer size %llu",
                 srcOffset, totalBytesInSubSamples, srcBuffer.size);
         android_errorWriteLog(0x534e4554, "67962232");
+        holder.reset();
         _hidl_cb(toStatus(BAD_VALUE), 0, NULL);
         return Void();
     }
@@ -135,6 +158,7 @@
                     "dstOffset %llu, totalBytesInSubSamples %llu, srcBuffer size %llu",
                     dstOffset, totalBytesInSubSamples, srcBuffer.size);
             android_errorWriteLog(0x534e4554, "67962232");
+            holder.reset();
             _hidl_cb(toStatus(BAD_VALUE), 0, NULL);
             return Void();
         }
@@ -146,7 +170,7 @@
     // Casting hidl SubSample to DescramblerPlugin::SubSample, but need
     // to ensure structs are actually idential
 
-    int32_t result = mPlugin->descramble(
+    int32_t result = holder->descramble(
             dstBuffer.type != BufferType::SHARED_MEMORY,
             (DescramblerPlugin::ScramblingControl)scramblingControl,
             subSamples.size(),
@@ -157,17 +181,17 @@
             dstOffset,
             NULL);
 
+    holder.reset();
     _hidl_cb(toStatus(result >= 0 ? OK : result), result, NULL);
     return Void();
 }
 
 Return<Status> DescramblerImpl::release() {
-    ALOGV("%s: mPlugin=%p", __FUNCTION__, mPlugin);
+    ALOGV("%s: plugin=%p", __FUNCTION__, mPluginHolder.get());
 
-    if (mPlugin != NULL) {
-        delete mPlugin;
-        mPlugin = NULL;
-    }
+    std::shared_ptr<DescramblerPlugin> holder(nullptr);
+    std::atomic_store(&mPluginHolder, holder);
+
     return Status::OK;
 }