diff --git a/middleware/drm/DrmSession.cpp b/middleware/drm/DrmSession.cpp index f3e48c987b..35d10c9525 100644 --- a/middleware/drm/DrmSession.cpp +++ b/middleware/drm/DrmSession.cpp @@ -24,12 +24,17 @@ #include "DrmSession.h" #include "PlayerLogManager.h" +#include /** * @brief Constructor for DrmSession. */ DrmSession::DrmSession(const string &keySystem) : m_keySystem(keySystem),m_OutputProtectionEnabled(false) , mContentSecurityManagerSession() + , mLifecycleMutex() + , mLifecycleCV() + , mActiveOperations(0) + , mMarkedForDestruction(false) { } @@ -40,6 +45,55 @@ DrmSession::~DrmSession() { } +/** + * @brief DELIA-70726 fix: Acquire lifecycle guard before use in decrypt(). + */ +bool DrmSession::AcquireForUse() +{ + std::lock_guard lock(mLifecycleMutex); + if (mMarkedForDestruction) + { + return false; + } + mActiveOperations++; + return true; +} + +/** + * @brief DELIA-70726 fix: Release lifecycle guard after use in decrypt(). + */ +void DrmSession::ReleaseAfterUse() +{ + std::lock_guard lock(mLifecycleMutex); + if (mActiveOperations > 0) + { + mActiveOperations--; + } + if (mActiveOperations == 0) + { + mLifecycleCV.notify_all(); + } +} + +/** + * @brief DELIA-70726 fix: Block deletion of this session until any decrypt() + * call already in progress (having acquired the guard) has finished. + */ +void DrmSession::PrepareForDestruction(uint32_t timeoutMs) +{ + std::unique_lock lock(mLifecycleMutex); + mMarkedForDestruction = true; + if (mActiveOperations > 0) + { + MW_LOG_WARN("DrmSession::PrepareForDestruction : waiting for %d in-flight decrypt operation(s) to complete before delete", mActiveOperations); + mLifecycleCV.wait_for(lock, std::chrono::milliseconds(timeoutMs), [this]() { return mActiveOperations == 0; }); + if (mActiveOperations > 0) + { + MW_LOG_ERR("DrmSession::PrepareForDestruction : timed out waiting for in-flight decrypt operation(s); proceeding with destruction"); + } + } +} + /** * @brief Get the DRM System, ie, UUID for PlayReady WideVine etc.. */ diff --git a/middleware/drm/DrmSession.h b/middleware/drm/DrmSession.h index 1766304c8d..0072fdcf7a 100755 --- a/middleware/drm/DrmSession.h +++ b/middleware/drm/DrmSession.h @@ -29,6 +29,9 @@ #include #include #include +#include +#include +#include #include "DrmUtils.h" #include "ContentSecurityManagerSession.h" @@ -66,7 +69,53 @@ class DrmSession std::string m_keySystem; bool m_OutputProtectionEnabled; ContentSecurityManagerSession mContentSecurityManagerSession; + + /* DELIA-70726 fix: + * Lifecycle guard used to prevent the DrmSession object from being + * deleted (e.g. by DrmSessionManager during DRM session slot + * reuse/eviction on back-to-back channel changes) while a GStreamer + * pipeline thread (multiqueue/decryptor) is concurrently inside + * decrypt()/verifyOutputProtection(). Without this guard, deletion of + * a session that is still referenced by an old, not-yet-fully-torn-down + * pipeline results in a use-after-free SIGSEGV inside + * OCDMSessionAdapter::verifyOutputProtection(). + */ + std::mutex mLifecycleMutex; + std::condition_variable mLifecycleCV; + int mActiveOperations; + bool mMarkedForDestruction; + public: + /** + * @fn AcquireForUse + * @brief Must be called by any external caller (e.g. the GStreamer + * decryptor element) before invoking decrypt() on a DrmSession + * obtained via a raw/cached pointer. Returns false if the + * session is already being torn down, in which case decrypt() + * MUST NOT be called on this object. + * @retval true if it is safe to call decrypt(), false otherwise. + */ + bool AcquireForUse(); + + /** + * @fn ReleaseAfterUse + * @brief Must be called exactly once for every successful AcquireForUse(), + * after the decrypt() call completes. + */ + void ReleaseAfterUse(); + + /** + * @fn PrepareForDestruction + * @brief Must be called by the owner (DrmSessionManager) before deleting + * this DrmSession. Marks the session so that any new + * AcquireForUse() calls fail fast, and blocks (bounded) until all + * in-flight decrypt() operations that already acquired the guard + * have completed, making it safe to free the object. + * @param timeoutMs maximum time to wait for in-flight operations to drain. + */ + void PrepareForDestruction(uint32_t timeoutMs = 3000); + + /** * @brief Create drm session with given init data * @param f_pbInitData : pointer to initdata diff --git a/middleware/drm/DrmSessionManager.cpp b/middleware/drm/DrmSessionManager.cpp index 078ffe0afe..305626aa51 100755 --- a/middleware/drm/DrmSessionManager.cpp +++ b/middleware/drm/DrmSessionManager.cpp @@ -109,6 +109,10 @@ void DrmSessionManager::clearSessionData() { if (drmSessionContexts != NULL && drmSessionContexts[i].drmSession != NULL) { + /* DELIA-70726 fix: block until any in-flight decrypt() on this session + * (called from a GStreamer pipeline thread via a cached raw pointer) + * has completed, before freeing the object. */ + drmSessionContexts[i].drmSession->PrepareForDestruction(); MW_SAFE_DELETE(drmSessionContexts[i].drmSession); drmSessionContexts[i] = DrmSessionContext(); } @@ -201,6 +205,8 @@ void DrmSessionManager::clearDrmSession(bool forceClearSession) if (drmSessionContexts[i].drmSession != NULL) { MW_LOG_WARN("DrmSessionManager:: Clearing failed Session Data Slot : %d", i); + /* DELIA-70726 fix: see clearSessionData() for rationale. */ + drmSessionContexts[i].drmSession->PrepareForDestruction(); MW_SAFE_DELETE(drmSessionContexts[i].drmSession); } } @@ -874,6 +880,13 @@ KeyState DrmSessionManager::getDrmSession(int &err, std::shared_ptr d } } MW_LOG_WARN("deleting existing DRM session for %s ", drmSessionContexts[sessionSlot].drmSession->getKeySystem().c_str()); + /* DELIA-70726 fix: this slot may still be referenced by a GStreamer + * decryptor element of a previous, not-yet-fully-torn-down pipeline + * (rapid/back-to-back channel change). Block here until any decrypt() + * call already in flight against this session finishes, so the delete + * below cannot race with OCDMSessionAdapter::verifyOutputProtection()/ + * decrypt() running on the old pipeline's multiqueue thread. */ + drmSessionContexts[sessionSlot].drmSession->PrepareForDestruction(); MW_SAFE_DELETE(drmSessionContexts[sessionSlot].drmSession); } this->ProfileUpdateCb(); diff --git a/middleware/gst-plugins/drm/gst/gstcdmidecryptor.cpp b/middleware/gst-plugins/drm/gst/gstcdmidecryptor.cpp index 10add97a62..245ee8bfd0 100755 --- a/middleware/gst-plugins/drm/gst/gstcdmidecryptor.cpp +++ b/middleware/gst-plugins/drm/gst/gstcdmidecryptor.cpp @@ -459,6 +459,7 @@ gst_cdmidecryptor_transform_caps(GstBaseTransform * trans, GST_LOG_OBJECT(trans, "returning %" GST_PTR_FORMAT, transformedCaps); if (direction == GST_PAD_SINK && !gst_caps_is_empty(transformedCaps)) { + GstCaps* sinkCapsCopy = NULL; g_mutex_lock(&cdmidecryptor->mutex); // clean up previous caps if (cdmidecryptor->sinkCaps) @@ -467,9 +468,12 @@ gst_cdmidecryptor_transform_caps(GstBaseTransform * trans, cdmidecryptor->sinkCaps = NULL; } cdmidecryptor->sinkCaps = gst_caps_copy(transformedCaps); - g_cond_signal(&cdmidecryptor->sinkCapsCond); - g_mutex_unlock(&cdmidecryptor->mutex); - GST_DEBUG_OBJECT(trans, "Set sinkCaps to %" GST_PTR_FORMAT, cdmidecryptor->sinkCaps); + sinkCapsCopy = gst_caps_ref(cdmidecryptor->sinkCaps); // take an extra ref to keep it alive + g_cond_signal(&cdmidecryptor->sinkCapsCond); + g_mutex_unlock(&cdmidecryptor->mutex); + GST_DEBUG_OBJECT(trans, "Set sinkCaps to %" GST_PTR_FORMAT, sinkCapsCopy); + gst_caps_unref(sinkCapsCopy); // release the extra ref + } return transformedCaps; } @@ -534,12 +538,19 @@ static GstFlowReturn gst_cdmidecryptor_transform_ip( { // call decrypt even for clear samples in order to copy it to a secure buffer. If secure buffers are not supported // decrypt() call will return without doing anything - if (cdmidecryptor->drmSession != NULL && cdmidecryptor->sinkCaps != NULL) - errorCode = cdmidecryptor->drmSession->decrypt(keyIDBuffer, ivBuffer, buffer, subSampleCount, subsamplesBuffer, cdmidecryptor->sinkCaps); + /* DELIA-70726 fix: guard against the DrmSession being concurrently torn down + * (e.g. DrmSessionManager reusing/evicting the slot during a back-to-back + * channel change) while this pipeline still has buffers in flight. */ + if (cdmidecryptor->drmSession != NULL && cdmidecryptor->sinkCaps != NULL + && cdmidecryptor->drmSession->AcquireForUse()) + { + errorCode = cdmidecryptor->drmSession->decrypt(keyIDBuffer, ivBuffer, buffer, subSampleCount, subsamplesBuffer, cdmidecryptor->sinkCaps); + cdmidecryptor->drmSession->ReleaseAfterUse(); + } else - { /* If drmSession creation failed, then the call will be aborted here */ + { /* If drmSession creation failed, or is being destroyed, the call will be aborted here */ result = GST_FLOW_NOT_SUPPORTED; - GST_ERROR_OBJECT(cdmidecryptor, "drmSession or sinkCaps is **** NULL ****, returning GST_FLOW_NOT_SUPPORTED"); + GST_ERROR_OBJECT(cdmidecryptor, "drmSession or sinkCaps is **** NULL **** (or session is being destroyed), returning GST_FLOW_NOT_SUPPORTED"); } } goto free_resources; @@ -656,7 +667,20 @@ static GstFlowReturn gst_cdmidecryptor_transform_ip( result = GST_FLOW_NOT_SUPPORTED; goto free_resources; } + /* DELIA-70726 fix: guard against the DrmSession being concurrently torn down + * (e.g. DrmSessionManager reusing/evicting the slot during a back-to-back + * channel change) while this pipeline still has buffers in flight. Without + * this guard, the multiqueue/decryptor thread can call decrypt() on a + * DrmSession that is being (or has already been) freed, causing a + * use-after-free SIGSEGV inside OCDMSessionAdapter::verifyOutputProtection(). */ + if (!cdmidecryptor->drmSession->AcquireForUse()) + { + GST_ERROR_OBJECT(cdmidecryptor, "drmSession is being destroyed, aborting decrypt"); + result = GST_FLOW_NOT_SUPPORTED; + goto free_resources; + } errorCode = cdmidecryptor->drmSession->decrypt(keyIDBuffer, ivBuffer, buffer, subSampleCount, subsamplesBuffer, cdmidecryptor->sinkCaps); + cdmidecryptor->drmSession->ReleaseAfterUse(); cdmidecryptor->streamEncrypted = true; if (errorCode != 0 || cdmidecryptor->hdcpOpProtectionFailCount)