From 83700a67efa1e733afe3b38ee401ef9cc745abf0 Mon Sep 17 00:00:00 2001 From: skrist381 Date: Tue, 30 Dec 2025 15:16:30 +0530 Subject: [PATCH] RDKB-61951 coverity issues in ccsp-psm Reason for change: Fixing coverity Medium impact issues Test Procedure: Primary Builds passed Risks: Low Priority: P0 Signed-off-by: Sudharshan Reddy --- source/PsmSysRegistry/psm_sysro_base.c | 12 ++++++- source/PsmSysRegistry/psm_sysro_sysramif.c | 8 +++++ source/Ssp/ssp_cfmif.c | 39 ++++++++++++++------ source/Ssp/ssp_dbus.c | 10 +++++- source/Ssp/ssp_main.c | 9 ++++- source/Ssp/ssp_rbus.c | 41 +++++++++++++++++----- 6 files changed, 98 insertions(+), 21 deletions(-) diff --git a/source/PsmSysRegistry/psm_sysro_base.c b/source/PsmSysRegistry/psm_sysro_base.c index 56a7af3..ea7ca33 100644 --- a/source/PsmSysRegistry/psm_sysro_base.c +++ b/source/PsmSysRegistry/psm_sysro_base.c @@ -495,6 +495,17 @@ PsmSysroInitialize pMyObject->FileSyncRefCount = 0; pMyObject->bNoSave = TRUE; /* do NOT allow save until being notified */ pMyObject->bNeedFlush = FALSE; /* indicate whether save is required */ + /* Coverity Fix CID:158659 - Avoid double mutex initialization */ + if (pthread_mutex_trylock(&pMyObject->AccessLock) != 0) // Try locking to check if the mutex is uninitialized + { + // If the mutex is uninitialized, initialize it + pthread_mutex_init(&pMyObject->AccessLock, NULL); // Initialize the mutex + } + else + { + // If the mutex is already initialized, unlock it + pthread_mutex_unlock(&pMyObject->AccessLock); + } /*Coverity Fix CID:135586 */ AnscAcquireLock(&pMyObject->AccessLock); pMyObject->bSaveInProgress = FALSE; /* indicate whether save is in progress */ @@ -532,7 +543,6 @@ PsmSysroInitialize pMyObject->SysRamEnableFileSync = PsmSysroSysRamEnableFileSync; pMyObject->SysRamNotify = PsmSysroSysRamNotify; - AnscInitializeLock(&pMyObject->AccessLock); /* * We shall initialize the object properties to the default values, which may be changed later diff --git a/source/PsmSysRegistry/psm_sysro_sysramif.c b/source/PsmSysRegistry/psm_sysro_sysramif.c index f7efab7..bccf73e 100644 --- a/source/PsmSysRegistry/psm_sysro_sysramif.c +++ b/source/PsmSysRegistry/psm_sysro_sysramif.c @@ -173,10 +173,18 @@ PsmSysroSysRamNotify PPSM_SYS_REGISTRY_OBJECT pMyObject = (PPSM_SYS_REGISTRY_OBJECT )hThisObject; // PANSC_TIMER_DESCRIPTOR_OBJECT pRegTimerObj = (PANSC_TIMER_DESCRIPTOR_OBJECT)pMyObject->hRegTimerObj; //CcspTraceInfo(("\n##PsmSysroSysRamNotify() begins##\n")); + + // CID 340552: Data race condition - Fixed by adding lock around access to bSaveInProgress + AnscAcquireLock(&pMyObject->AccessLock); + if ( pMyObject->bNoSave || pMyObject->bSaveInProgress ) { + // Release the lock before returning + AnscReleaseLock(&pMyObject->AccessLock); return ANSC_STATUS_SUCCESS; } + // Release the lock after the check + AnscReleaseLock(&pMyObject->AccessLock); switch ( ulEvent ) { diff --git a/source/Ssp/ssp_cfmif.c b/source/Ssp/ssp_cfmif.c index 3f6350f..7a49488 100644 --- a/source/Ssp/ssp_cfmif.c +++ b/source/Ssp/ssp_cfmif.c @@ -560,7 +560,16 @@ int merge_missing_Partner_params() { CcspTraceInfo(("data is not NULL %d\n",len)); memset( data, 0, ( sizeof(char) * (len + 1) )); - fread( data, 1, len, fp ); + // Fix for CID 190387: Check the number of bytes read by fread + size_t bytesRead = fread(data, 1, len, fp); + if ((size_t)bytesRead != (size_t)len) + { + // If fread doesn't read the expected number of bytes, handle the error + CcspTraceInfo(("Error reading file %s: expected %d bytes, but read %zu bytes\n", BOOTSTRAP_INFO_FILE, len, bytesRead)); + free(data); + fclose(fp); + return -1; + } } else { @@ -584,6 +593,10 @@ int merge_missing_Partner_params() //CcspTraceInfo(("IsEntryMissed PartnerID %s \n", PartnerID)); cJSON* wc_url=NULL; unsigned int m; + + // CID 559731: Data race condition (MISSING_LOCK) + // Acquire the lock before accessing parm_present_table + pthread_mutex_lock(&rec_hash_lock); for(m=0; m< PSM_PARAM_TOTAL; m++ ) { if( parm_present_table[m].value == false ) @@ -606,6 +619,8 @@ int merge_missing_Partner_params() } } } + // CID 559731:Release the lock after all accesses to parm_present_table are complete + pthread_mutex_unlock(&rec_hash_lock); free(data); data=NULL; cJSON_Delete(buf); @@ -724,7 +739,9 @@ static int import_custom_params(int overwrite) return -1; for (i = 0; i < cus_cnt; i++) { - if (!cus_params[i].name || !strlen(cus_params[i].name)) { + //CID 67111: Ensuring name is not NULL and not an empty string + //CID 65264: Simplified the check for NULL and empty string + if (cus_params[i].name == NULL || cus_params[i].name[0] == '\0') { CcspTraceError(("%s: invalid custom param\n", __FUNCTION__)); continue; } @@ -1953,8 +1970,10 @@ static ANSC_STATUS NodeGetRecord (PANSC_XML_DOM_NODE_OBJECT node, PsmRecord_t *r /* value */ size = sizeof(rec->value) - 1; if (node->GetDataString(node, NULL, rec->value, &size) != ANSC_STATUS_SUCCESS) + {//Fix for CID:57012 Structurally dead code CcspTraceInfo(("NodeGetRecord -> node->GetDataString(node, NULL, rec->value, &size) != ANSC_STATUS_SUCCESS\n")); return ANSC_STATUS_FAILURE; + } rec->value[size] = '\0'; return ANSC_STATUS_SUCCESS; @@ -1965,8 +1984,10 @@ static ANSC_STATUS AddRecToXml (const PsmRecord_t *rec, PANSC_XML_DOM_NODE_OBJEC PANSC_XML_DOM_NODE_OBJECT node; if ((node = xml->AddChildByName(xml, ELEM_RECORD)) == NULL) + {//CID 68537: Structurally dead code CcspTraceInfo(("AddRecToXml-> node = xml->AddChildByName(xml, ELEM_RECORD) \n")); return ANSC_STATUS_FAILURE; + } if (RecordSetNode(rec, node) != ANSC_STATUS_SUCCESS) { @@ -1989,7 +2010,9 @@ static ANSC_STATUS XmlToBuffer (PANSC_XML_DOM_NODE_OBJECT xml, char **buf, ULONG || xml->Encode(xml, (PVOID)newBuf, &newSize) != ANSC_STATUS_SUCCESS) { if (newBuf) + {// CID :57685 Nesting level does not match indentation AnscFreeMemory(newBuf); + } CcspTraceInfo(("XmlToBuffer ends->newBuf")); return ANSC_STATUS_FAILURE; } @@ -2006,12 +2029,13 @@ static ANSC_STATUS XmlToFile (PANSC_XML_DOM_NODE_OBJECT xml, const char *file) ANSC_HANDLE pFile; // CcspTraceInfo(("XmlToFile begins\n")); if (XmlToBuffer(xml, &buf, &size) != ANSC_STATUS_SUCCESS) + { //Fix for CID:64065 Structurally dead code CcspTraceInfo(("XmlToFile -> XmlToBuffer(xml, &buf, &size) != ANSC_STATUS_SUCCESS\n")); /*Coverity Fix CID:67400 RESOURCE_LEAK */ if( buf != NULL) AnscFreeMemory(buf); return ANSC_STATUS_FAILURE; - + } if ((pFile = AnscOpenFile((char *)file, ANSC_FILE_TYPE_RDWR, ANSC_FILE_TYPE_RDWR)) == NULL) @@ -2077,7 +2101,6 @@ static PANSC_XML_DOM_NODE_OBJECT ReadCfgXmlWithCustom (const char *path, int ove PsmHalParam_t *cusParams = NULL; int cusCnt, i; char *buf = NULL; - int success = 0; int missing; ULONG size; errno_t rc = -1; @@ -2132,14 +2155,10 @@ static PANSC_XML_DOM_NODE_OBJECT ReadCfgXmlWithCustom (const char *path, int ove } } - success = 1; done: - if (!success && root) - { - root->Remove(root); - root = NULL; - } + //CID: 5337 logically dead code + //Removed the logically dead code because the condition if (!success && root) will never be true when root is NULL if (buf) AnscFreeMemory(buf); if (cusParams) diff --git a/source/Ssp/ssp_dbus.c b/source/Ssp/ssp_dbus.c index 674a79f..1d95717 100644 --- a/source/Ssp/ssp_dbus.c +++ b/source/Ssp/ssp_dbus.c @@ -1497,7 +1497,15 @@ int PsmDbusInit() } } - CCSP_Message_Bus_Init(CName, CCSP_MSG_BUS_CFG, &bus_handle,(CCSP_MESSAGE_BUS_MALLOC) Ansc_AllocateMemory_Callback, Ansc_FreeMemory_Callback); + int bus_init_result = CCSP_Message_Bus_Init(CName, CCSP_MSG_BUS_CFG, &bus_handle, (CCSP_MESSAGE_BUS_MALLOC)Ansc_AllocateMemory_Callback, Ansc_FreeMemory_Callback); + + // CID :71410 Unchecked return value + //Check the return value of CCSP_Message_Bus_Init + if (bus_init_result != CCSP_SUCCESS) { + CcspTraceWarning(("RDKB_SYSTEM_BOOT_UP_LOG : CCSP_Message_Bus_Init failed with error code %d\n", bus_init_result)); + return -1; // Or handle the failure accordingly + } + g_psmHealth = CCSP_COMMON_COMPONENT_HEALTH_Yellow; /* Wait for CR ready */ waitConditionReady(bus_handle, CrName, CCSP_DBUS_PATH_CR, CName); diff --git a/source/Ssp/ssp_main.c b/source/Ssp/ssp_main.c index bf2a8c2..5267bc5 100644 --- a/source/Ssp/ssp_main.c +++ b/source/Ssp/ssp_main.c @@ -382,7 +382,14 @@ int main(int argc, char* argv[]) if(ret != 0){ return 1; } - creat("/tmp/psm_initialized", S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH); + // Coverity Fix: Handle return value from creat to avoid unhandled error (CID 257719) + int fd_creat = creat("/tmp/psm_initialized", S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH); + if (fd_creat == -1) { + perror("Error creating file /tmp/psm_initialized"); + return 1; // Handle the error + } + close(fd_creat); // Close the file descriptor after use + if(!blocklist_ret){ update_process_caps(&appcaps); read_capability(&appcaps); diff --git a/source/Ssp/ssp_rbus.c b/source/Ssp/ssp_rbus.c index a81cfc6..6ae2de0 100644 --- a/source/Ssp/ssp_rbus.c +++ b/source/Ssp/ssp_rbus.c @@ -155,6 +155,7 @@ static int setParameterValues_rbus(rbusObject_t inParams, rbusObject_t outParams rbusValueType_t type; int param_size = 0; char *parameterName, *parameterValue = NULL; + if ( pPsmSysRegistry == NULL ) { CcspTraceError(("%s - pPsmSysRegistry is NULL\n",__func__)); @@ -176,6 +177,7 @@ static int setParameterValues_rbus(rbusObject_t inParams, rbusObject_t outParams parameterValue = rbusValue_ToString(value, NULL, 0); parameterValStruct_t *val = NULL; val = AnscAllocateMemory(sizeof(parameterValStruct_t)); + if(val == NULL) { CcspTraceInfo(("Memory Allocation failed - %s : %d\n", __FUNCTION__, __LINE__)); @@ -194,12 +196,22 @@ static int setParameterValues_rbus(rbusObject_t inParams, rbusObject_t outParams CcspTraceInfo(("Memory Allocation failed - %s : %d\n", __FUNCTION__, __LINE__)); ret = RBUS_ERROR_BUS_ERROR; } - strcpy_s(val->parameterName, paramNameLen, parameterName); - /* Copy the Value */ + // Fix for Coverity CID : 420196 : Check the return value of strcpy_s + if (strcpy_s(val->parameterName, paramNameLen, parameterName) != 0) { + CcspTraceInfo(("strcpy_s failed - %s : %d\n", __FUNCTION__, __LINE__)); + ret = RBUS_ERROR_BUS_ERROR; + } + + /* Copy the Value */ val->parameterValue = AnscAllocateMemory(paramValLen); memset(val->parameterValue, 0, paramValLen); - strcpy_s(val->parameterValue, paramValLen, parameterValue); + + // Fix for Coverity CID :420196 : Check the return value of strcpy_s + if (strcpy_s(val->parameterValue, paramValLen, parameterValue) != 0) { + CcspTraceInfo(("strcpy_s failed - %s : %d\n", __FUNCTION__, __LINE__)); + ret = RBUS_ERROR_BUS_ERROR; + } /* Find the matching ccsp_type */ rbus_type_to_ccsp_type(type, &val->type); @@ -209,13 +221,14 @@ static int setParameterValues_rbus(rbusObject_t inParams, rbusObject_t outParams returnStatus = setParameterValues(0, 0, val, 1, 0, NULL, NULL); CcspTraceDebug(("%s Add entry: param : %s , val : %s %s , return %d \n", __func__,((returnStatus != CCSP_SUCCESS)? "failed" : "success"), val->parameterName, val->parameterValue, returnStatus)); } + if (val->parameterName) { - AnscFreeMemory(val->parameterName); + AnscFreeMemory(val->parameterName); } if (val->parameterValue) { - AnscFreeMemory(val->parameterValue); + AnscFreeMemory(val->parameterValue); } AnscFreeMemory(val); @@ -375,6 +388,7 @@ static int delParameterValues_rbus(rbusObject_t inParams, rbusObject_t outParams rbusProperty_t prop; int i = 0; int param_size = 0; + if ( pPsmSysRegistry == NULL ) { CcspTraceError(("%s - pPsmSysRegistry is NULL\n",__func__)); @@ -384,21 +398,30 @@ static int delParameterValues_rbus(rbusObject_t inParams, rbusObject_t outParams prop = rbusObject_GetProperties(inParams); while(prop) { - param_size++; + param_size++; prop = rbusProperty_GetNext(prop); } prop = rbusObject_GetProperties(inParams); + /* Copy Parameter Name */ for (i = 0; i < param_size; i++) { parameterAttributeStruct_t *parameterAttribute = 0; parameterAttribute = AnscAllocateMemory(1*sizeof(parameterAttributeStruct_t)); memset(parameterAttribute, 0, 1*sizeof(parameterAttributeStruct_t)); + unsigned int ParameterNameLen = strlen(rbusProperty_GetName(prop))+1; parameterAttribute->parameterName = AnscAllocateMemory(ParameterNameLen); - strcpy_s(parameterAttribute->parameterName, ParameterNameLen, rbusProperty_GetName(prop)); - /*Set Parameter Attributes*/ + // Fix for Coverity CID: 420197: Check the return value of strcpy_s + if (strcpy_s(parameterAttribute->parameterName, ParameterNameLen, rbusProperty_GetName(prop)) != 0) { + CcspTraceInfo(("strcpy_s failed - %s : %d\n", __FUNCTION__, __LINE__)); + rc = RBUS_ERROR_BUS_ERROR; + AnscFreeMemory(parameterAttribute); + break; // Exit the loop in case of error + } + + /* Set Parameter Attributes */ parameterAttribute->notificationChanged = 0; parameterAttribute->notification = 0; parameterAttribute->access = 0; @@ -416,9 +439,11 @@ static int delParameterValues_rbus(rbusObject_t inParams, rbusObject_t outParams CcspTraceError(("%s Failed to delete entry: %s \n", __func__,parameterAttribute->parameterName)); setOutparams(outParams,parameterAttribute->parameterName,false); } + AnscFreeMemory(parameterAttribute); prop = rbusProperty_GetNext(prop); } + return rc; }