SWRDKV-5447: ocdm-playready defect fix.#14
Open
jianwang-brcm wants to merge 3 commits into
Open
Conversation
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
There was a problem hiding this comment.
Pull request overview
This PR addresses stability issues in the PlayReady CDMi integration by hardening IV handling in decrypt, adding additional bounds checks around secure-token operations, and introducing session tracking to prevent resetting/freeing the shared PlayReady app context while sessions still reference it.
Changes:
- Replaced
assert(ivLength > 0)with a runtime validation thativLengthis exactly 8 (CTR) or 16 (CBC), and clamped a Netflix-path IVmemcpy. - Added additional safety checks in the decrypt path to avoid buffer overruns (SVP token size, decrypted length vs. encrypted length).
- Introduced
m_sessionCountplus additional locking around app-context usage and lifecycle operations.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| MediaSystem.cpp | Adds session counting and additional locking to prevent app-context reset/uninit while sessions exist. |
| MediaSession.cpp | Adds runtime IV length validation, clamps IV copy, adds several mutex guards, and adds extra length checks in decrypt paths. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
153
to
166
| bool isNetflixPlayready = (strstr(keySystem.c_str(), "netflix") != nullptr); | ||
| if (isNetflixPlayready) { | ||
| SafeCriticalSection systemLock(drmAppContextMutex_); | ||
| if(!m_isAppCtxInitialized) | ||
| { | ||
| InitializeAppCtx(); | ||
| } | ||
| } | ||
| *f_ppiMediaKeySession = new CDMi::MediaKeySession(f_pbInitData, f_cbInitData, m_poAppContext.get(), !isNetflixPlayready); | ||
| } else { | ||
| *f_ppiMediaKeySession = new CDMi::MediaKeySession(f_pbInitData, f_cbInitData, f_pbCDMData, f_cbCDMData, m_poAppContext.get(), !isNetflixPlayready); | ||
| } | ||
| return CDMi_SUCCESS; | ||
| ++m_sessionCount; | ||
| return CDMi_SUCCESS; | ||
| } |
Comment on lines
+258
to
264
| SafeCriticalSection systemLock(drmAppContextMutex_); | ||
| bool isNetflixPlayready = (strstr(keySystem.c_str(), "netflix") != nullptr); | ||
| printf("\n [TEL ELXSI] isNetflixPlayready is %d",&isNetflixPlayready); | ||
| *session = new CDMi::MediaKeySession(drmHeader, drmHeaderLength, m_poAppContext.get(), !isNetflixPlayready); | ||
| ++m_sessionCount; | ||
|
|
||
| return CDMi_SUCCESS; |
Comment on lines
269
to
273
| SafeCriticalSection systemLock(drmAppContextMutex_); | ||
| delete f_piMediaKeySession; | ||
| if (m_sessionCount > 0) --m_sessionCount; | ||
| return CDMi_SUCCESS; | ||
| } |
Comment on lines
211
to
213
| delete f_piMediaKeySession; | ||
| if (m_sessionCount > 0) --m_sessionCount; | ||
| } |
Comment on lines
+623
to
+628
| if (m_sessionCount > 0) | ||
| { | ||
| /* Existing MediaKeySession(s) hold a raw alias of m_poAppContext.get(); freeing here is a UAF. */ | ||
| fprintf(stderr, "[%s:%d] UninitializeAppCtx refused: %u session(s) still hold m_poAppContext\n",__FUNCTION__,__LINE__,m_sessionCount); | ||
| return CDMi_S_FALSE; | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What it does: Replaces the
assert(ivLength>0)with a runtime check thatsampleInfo->ivLengthis exactlyEXPECTED_AES_CTR_IVDATA_SIZE(8) orEXPECTED_AES_CBC_IVDATA_SIZE(16) — returningCDMi_S_FALSEotherwise — and additionally clamps the Netflix-pathmemcpytosizeof(iv_vector).