RDKEMW-15033: Move tenablehdcp to rdke GitHub#191
Open
santoshcomcast wants to merge 3 commits into
Open
Conversation
Reason for change: Move tenablehdcp to rdke GitHub Test Procedure: refer RDKEMW-15033 Risks: High Signed-off-by:gsanto722 <grandhi_santoshkumar@comcast.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the dsmgr HDCP enable flow in dsMgr.c by changing how the MFR serialized-data request buffer is managed and by adding extra validation around the returned key length before HDCP is enabled. In the broader codebase, this sits on the HDMI/HDCP startup path that pulls keys from MFR and passes them into _dsEnableHDCP.
Changes:
- Replace the stack-allocated
IARM_Bus_MFRLib_GetSerializedData_Param_twith a heap allocation and explicit cleanup. - Add a new
bufLenvalidation branch before copying the HDCP key buffer. - Keep the existing HDCP enable flow but gate it behind the updated MFR-read handling.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+271
to
+278
| /* Validate bufLen before using it - guard against corrupt/unexpected MFR response */ | ||
| if (param->bufLen == 0 || param->bufLen > MAX_SERIALIZED_BUF) { | ||
| INT_ERROR("Invalid bufLen from MFR: %d, expected 0 < bufLen <= %d\n", param->bufLen, MAX_SERIALIZED_BUF); | ||
| sleep(2); | ||
| continue; | ||
| } | ||
| hdcpParam.keySize = param->bufLen; | ||
| if (hdcpParam.keySize < 0 || hdcpParam.keySize > HDCP_KEY_MAX_SIZE) { | ||
| if (hdcpParam.keySize > HDCP_KEY_MAX_SIZE) { |
Comment on lines
+231
to
+235
| IARM_Bus_MFRLib_GetSerializedData_Param_t *param = | ||
| (IARM_Bus_MFRLib_GetSerializedData_Param_t *)malloc(sizeof(*param)); | ||
| if (!param) { | ||
| INT_ERROR("Failed to allocate IARM_Bus_MFRLib_GetSerializedData_Param_t\n"); | ||
| return NULL; |
Comment on lines
+231
to
+235
| IARM_Bus_MFRLib_GetSerializedData_Param_t *param = | ||
| (IARM_Bus_MFRLib_GetSerializedData_Param_t *)malloc(sizeof(*param)); | ||
| if (!param) { | ||
| INT_ERROR("Failed to allocate IARM_Bus_MFRLib_GetSerializedData_Param_t\n"); | ||
| return NULL; |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 7 comments.
Comments suppressed due to low confidence (1)
dsmgr/dsMgr.c:240
- If memset_s(&hdcpParam, ...) fails, the code logs an error but continues and later passes hdcpParam to memcpy_s/_dsEnableHDCP. That leaves hdcpParam potentially uninitialized. On memset_s failure, bail out (free param and return) or otherwise ensure hdcpParam is in a known-safe state before use.
rc = memset_s(&hdcpParam, sizeof(hdcpParam), 0, sizeof(hdcpParam));
if (rc != EOK) {
INT_ERROR("Failed to reset HDCP Param: error code:%d\n", rc);
}
| { | ||
| if (data.buf == NULL || data.bufLen == 0) { | ||
| LOG("[mfrMgr] getSerializedData_ ERROR: mfrGetSerializedData returned empty data (buf=%p bufLen=%d)\n", | ||
| data.buf, data.bufLen); |
Comment on lines
+127
to
+138
| LOG("[mfrMgr] getSerializedData_ mfrGetSerializedData returned err=%d data.bufLen=%d data.buf=%p\n", | ||
| err, data.bufLen, data.buf); | ||
|
|
||
| if(mfrERR_NONE == err) | ||
| { | ||
| if (data.buf == NULL || data.bufLen == 0) { | ||
| LOG("[mfrMgr] getSerializedData_ ERROR: mfrGetSerializedData returned empty data (buf=%p bufLen=%d)\n", | ||
| data.buf, data.bufLen); | ||
| return IARM_RESULT_IPCCORE_FAIL; | ||
| } | ||
| if (data.bufLen > (int)sizeof(param->buffer)) { | ||
| LOG("[mfrMgr] getSerializedData_ ERROR: data.bufLen=%d exceeds param->buffer size=%zu\n", |
| data.buf, data.bufLen); | ||
| return IARM_RESULT_IPCCORE_FAIL; | ||
| } | ||
| if (data.bufLen > (int)sizeof(param->buffer)) { |
Comment on lines
+271
to
+276
| /* Validate bufLen before using it - guard against corrupt/unexpected MFR response */ | ||
| if (param->bufLen == 0 || param->bufLen > MAX_SERIALIZED_BUF) { | ||
| INT_ERROR("Invalid bufLen from MFR: %d, expected 0 < bufLen <= %d\n", param->bufLen, MAX_SERIALIZED_BUF); | ||
| sleep(2); | ||
| continue; | ||
| } |
Comment on lines
+27
to
+28
| ExecStartPre=/bin/mkdir -p /tmp/persistent | ||
| ExecStartPre=/bin/sh -c 'echo "/tmp/persistent/%%e.%%s.core" > /proc/sys/kernel/core_pattern' |
Comment on lines
+494
to
+495
| INT_INFO("calling _enableHDCPAsync()\n"); | ||
| _enableHDCPAsync(); |
Comment on lines
+108
to
+111
| if (!param) { | ||
| LOG("[mfrMgr] getSerializedData_ ERROR: NULL param\n"); | ||
| return IARM_RESULT_IPCCORE_FAIL; | ||
| } |
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.
Reason for change: Move tenablehdcp to rdke GitHub Test Procedure: refer RDKEMW-15033
Risks: High
Signed-off-by:gsanto722 grandhi_santoshkumar@comcast.com