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
(cherry picked from commit addcb3aa51f67c8c4f2959ee6f022fdb9473859c)
diff --git a/cas/1.0/default/CasImpl.cpp b/cas/1.0/default/CasImpl.cpp
index 9d1f4a3..178020e 100644
--- a/cas/1.0/default/CasImpl.cpp
+++ b/cas/1.0/default/CasImpl.cpp
@@ -31,19 +31,8 @@
namespace V1_0 {
namespace implementation {
-struct CasImpl::PluginHolder : public RefBase {
-public:
- explicit PluginHolder(CasPlugin *plugin) : mPlugin(plugin) {}
- ~PluginHolder() { if (mPlugin != NULL) delete mPlugin; }
- CasPlugin* get() { return mPlugin; }
-
-private:
- CasPlugin *mPlugin;
- DISALLOW_EVIL_CONSTRUCTORS(PluginHolder);
-};
-
CasImpl::CasImpl(const sp<ICasListener> &listener)
- : mPluginHolder(NULL), mListener(listener) {
+ : mListener(listener) {
ALOGV("CTOR");
}
@@ -69,7 +58,8 @@
void CasImpl::init(const sp<SharedLibrary>& library, CasPlugin *plugin) {
mLibrary = library;
- mPluginHolder = new PluginHolder(plugin);
+ std::shared_ptr<CasPlugin> holder(plugin);
+ std::atomic_store(&mPluginHolder, holder);
}
void CasImpl::onEvent(
@@ -88,21 +78,22 @@
Return<Status> CasImpl::setPrivateData(const HidlCasData& pvtData) {
ALOGV("%s", __FUNCTION__);
- sp<PluginHolder> holder = mPluginHolder;
- if (holder == NULL) {
+ std::shared_ptr<CasPlugin> holder = std::atomic_load(&mPluginHolder);
+ if (holder.get() == nullptr) {
return toStatus(INVALID_OPERATION);
}
- return toStatus(holder->get()->setPrivateData(pvtData));
+ return toStatus(holder->setPrivateData(pvtData));
}
Return<void> CasImpl::openSession(openSession_cb _hidl_cb) {
ALOGV("%s", __FUNCTION__);
CasSessionId sessionId;
- sp<PluginHolder> holder = mPluginHolder;
+ std::shared_ptr<CasPlugin> holder = std::atomic_load(&mPluginHolder);
status_t err = INVALID_OPERATION;
- if (holder != NULL) {
- err = holder->get()->openSession(&sessionId);
+ if (holder.get() != nullptr) {
+ err = holder->openSession(&sessionId);
+ holder.reset();
}
_hidl_cb(toStatus(err), sessionId);
@@ -114,87 +105,87 @@
const HidlCasSessionId &sessionId, const HidlCasData& pvtData) {
ALOGV("%s: sessionId=%s", __FUNCTION__,
sessionIdToString(sessionId).string());
- sp<PluginHolder> holder = mPluginHolder;
- if (holder == NULL) {
+ std::shared_ptr<CasPlugin> holder = std::atomic_load(&mPluginHolder);
+ if (holder.get() == nullptr) {
return toStatus(INVALID_OPERATION);
}
- return toStatus(
- holder->get()->setSessionPrivateData(
- sessionId, pvtData));
+ return toStatus(holder->setSessionPrivateData(sessionId, pvtData));
}
Return<Status> CasImpl::closeSession(const HidlCasSessionId &sessionId) {
ALOGV("%s: sessionId=%s", __FUNCTION__,
sessionIdToString(sessionId).string());
- sp<PluginHolder> holder = mPluginHolder;
- if (holder == NULL) {
+ std::shared_ptr<CasPlugin> holder = std::atomic_load(&mPluginHolder);
+ if (holder.get() == nullptr) {
return toStatus(INVALID_OPERATION);
}
- return toStatus(holder->get()->closeSession(sessionId));
+ return toStatus(holder->closeSession(sessionId));
}
Return<Status> CasImpl::processEcm(
const HidlCasSessionId &sessionId, const HidlCasData& ecm) {
ALOGV("%s: sessionId=%s", __FUNCTION__,
sessionIdToString(sessionId).string());
- sp<PluginHolder> holder = mPluginHolder;
- if (holder == NULL) {
+ std::shared_ptr<CasPlugin> holder = std::atomic_load(&mPluginHolder);
+ if (holder.get() == nullptr) {
return toStatus(INVALID_OPERATION);
}
- return toStatus(holder->get()->processEcm(sessionId, ecm));
+ return toStatus(holder->processEcm(sessionId, ecm));
}
Return<Status> CasImpl::processEmm(const HidlCasData& emm) {
ALOGV("%s", __FUNCTION__);
- sp<PluginHolder> holder = mPluginHolder;
- if (holder == NULL) {
+ std::shared_ptr<CasPlugin> holder = std::atomic_load(&mPluginHolder);
+ if (holder.get() == nullptr) {
return toStatus(INVALID_OPERATION);
}
- return toStatus(holder->get()->processEmm(emm));
+ return toStatus(holder->processEmm(emm));
}
Return<Status> CasImpl::sendEvent(
int32_t event, int32_t arg,
const HidlCasData& eventData) {
ALOGV("%s", __FUNCTION__);
- sp<PluginHolder> holder = mPluginHolder;
- if (holder == NULL) {
+ std::shared_ptr<CasPlugin> holder = std::atomic_load(&mPluginHolder);
+ if (holder.get() == nullptr) {
return toStatus(INVALID_OPERATION);
}
- status_t err = holder->get()->sendEvent(event, arg, eventData);
+ status_t err = holder->sendEvent(event, arg, eventData);
return toStatus(err);
}
Return<Status> CasImpl::provision(const hidl_string& provisionString) {
ALOGV("%s: provisionString=%s", __FUNCTION__, provisionString.c_str());
- sp<PluginHolder> holder = mPluginHolder;
- if (holder == NULL) {
+ std::shared_ptr<CasPlugin> holder = std::atomic_load(&mPluginHolder);
+ if (holder.get() == nullptr) {
return toStatus(INVALID_OPERATION);
}
- return toStatus(holder->get()->provision(String8(provisionString.c_str())));
+ return toStatus(holder->provision(String8(provisionString.c_str())));
}
Return<Status> CasImpl::refreshEntitlements(
int32_t refreshType,
const HidlCasData& refreshData) {
ALOGV("%s", __FUNCTION__);
- sp<PluginHolder> holder = mPluginHolder;
- if (holder == NULL) {
+ std::shared_ptr<CasPlugin> holder = std::atomic_load(&mPluginHolder);
+ if (holder.get() == nullptr) {
return toStatus(INVALID_OPERATION);
}
- status_t err = holder->get()->refreshEntitlements(refreshType, refreshData);
+ status_t err = holder->refreshEntitlements(refreshType, refreshData);
return toStatus(err);
}
Return<Status> CasImpl::release() {
- ALOGV("%s: plugin=%p", __FUNCTION__,
- mPluginHolder != NULL ? mPluginHolder->get() : NULL);
- mPluginHolder.clear();
+ ALOGV("%s: plugin=%p", __FUNCTION__, mPluginHolder.get());
+
+ std::shared_ptr<CasPlugin> holder(nullptr);
+ std::atomic_store(&mPluginHolder, holder);
+
return Status::OK;
}
diff --git a/cas/1.0/default/CasImpl.h b/cas/1.0/default/CasImpl.h
index 841d64e..d792838 100644
--- a/cas/1.0/default/CasImpl.h
+++ b/cas/1.0/default/CasImpl.h
@@ -88,7 +88,7 @@
private:
struct PluginHolder;
sp<SharedLibrary> mLibrary;
- sp<PluginHolder> mPluginHolder;
+ std::shared_ptr<CasPlugin> mPluginHolder;
sp<ICasListener> mListener;
DISALLOW_EVIL_CONSTRUCTORS(CasImpl);
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;
}
diff --git a/cas/1.0/default/DescramblerImpl.h b/cas/1.0/default/DescramblerImpl.h
index d3b146e..305f115 100644
--- a/cas/1.0/default/DescramblerImpl.h
+++ b/cas/1.0/default/DescramblerImpl.h
@@ -55,7 +55,7 @@
private:
sp<SharedLibrary> mLibrary;
- DescramblerPlugin *mPlugin;
+ std::shared_ptr<DescramblerPlugin> mPluginHolder;
DISALLOW_EVIL_CONSTRUCTORS(DescramblerImpl);
};
