RDK-60307 [RRD] RDK Coverity Defect Resolution for Device Management #175

madhubabutt · 2026-02-03T14:50:52Z

No description provided.

github-actions · 2026-02-03T14:52:16Z

Code Coverage Summary

                               Total:|83.6%   5104|97.6%  1301|    -      0

Copilot

Pull request overview

This pull request addresses Coverity defects in the RDK Remote Debugger Device Management code, focusing on fixing memory leaks, buffer overflows, and resource handling issues.

Changes:

Added NULL checks for memory allocations (malloc, strdup, fread) with proper error handling and resource cleanup
Fixed buffer overflow vulnerabilities by using correct size parameters in strncpy/strncat operations
Corrected file pointer closure to only occur when pointers are valid (v_secure_pclose moved inside if blocks)
Fixed memory management issues with rfcbuf allocations and ownership

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
src/rrdRunCmdThread.c	Added mutex protection for cacheDataNode initialization; fixed buffer overflow in strncpy/strncat; corrected v_secure_pclose placement to prevent potential crashes
src/rrdJsonParser.c	Added NULL checks for malloc/strdup/fread operations with proper cleanup; fixed rfcbuf memory leaks in error paths; clarified rfcbuf ownership with comment
src/rrdEventProcess.c	Fixed staticstrlen calculation after quote removal; corrected appendstrlen calculation; improved realloc error handling with proper log level

Copilot · 2026-02-03T14:55:27Z

src/rrdRunCmdThread.c

+    pthread_mutex_lock(&rrdCacheMut);
    cacheDataNode = NULL;
+    pthread_mutex_unlock(&rrdCacheMut);


The mutex lock/unlock around setting cacheDataNode to NULL in initCache() is unnecessary and potentially problematic. This function is called once during initialization before any threads are created (from main), so there's no concurrent access to protect against. Additionally, locking a newly initialized mutex that no other thread can access yet serves no purpose. The assignment to NULL is already atomic and thread-safe for pointer writes.

Suggested change

pthread_mutex_lock(&rrdCacheMut);

cacheDataNode = NULL;

pthread_mutex_unlock(&rrdCacheMut);

cacheDataNode = NULL;

Copilot · 2026-02-03T14:55:28Z

src/rrdEventProcess.c

                        char *appendcommandstr = (char *)realloc(staticprofiledata->command, appendstrlen);
                        if (appendcommandstr == NULL) {
-                            RDK_LOG(RDK_LOG_DEBUG, LOG_REMDEBUG, "[%s:%d]: Memory Allocation Failed... \n", __FUNCTION__, __LINE__);
+                            RDK_LOG(RDK_LOG_ERROR, LOG_REMDEBUG, "[%s:%d]: Memory Allocation Failed... \n", __FUNCTION__, __LINE__);


When realloc fails, there are multiple memory leaks: 1) The original memory pointed to by staticprofiledata->command is not freed (realloc leaves the original pointer valid when it returns NULL), 2) staticprofiledata->rfcvalue is not freed, 3) staticprofiledata itself is not freed, 4) dynamicprofiledata->rfcvalue, dynamicprofiledata->command, and dynamicprofiledata itself are not freed. All of these allocated resources need to be properly freed before returning from the function on this error path.

Suggested change

RDK_LOG(RDK_LOG_ERROR, LOG_REMDEBUG, "[%s:%d]: Memory Allocation Failed... \n", __FUNCTION__, __LINE__);

RDK_LOG(RDK_LOG_ERROR, LOG_REMDEBUG, "[%s:%d]: Memory Allocation Failed... \n", __FUNCTION__, __LINE__);

/* Cleanup allocated resources on realloc failure */

if (staticprofiledata != NULL)

{

if (staticprofiledata->command != NULL)

{

free(staticprofiledata->command);

staticprofiledata->command = NULL;

}

if (staticprofiledata->rfcvalue != NULL)

{

free(staticprofiledata->rfcvalue);

staticprofiledata->rfcvalue = NULL;

}

free(staticprofiledata);

staticprofiledata = NULL;

}

if (dynamicprofiledata != NULL)

{

if (dynamicprofiledata->command != NULL)

{

free(dynamicprofiledata->command);

dynamicprofiledata->command = NULL;

}

if (dynamicprofiledata->rfcvalue != NULL)

{

free(dynamicprofiledata->rfcvalue);

dynamicprofiledata->rfcvalue = NULL;

}

free(dynamicprofiledata);

dynamicprofiledata = NULL;

}

rdkcmf-jenkins · 2026-02-03T15:02:36Z

Coverity Issue - Resource leak

Variable "staticprofiledata" going out of scope leaks the storage it points to.

High Impact, CWE-404
RESOURCE_LEAK

Issue location

This issue was discovered outside the diff for this Pull Request. You can find it at:
src/rrdEventProcess.c:251

rdkcmf-jenkins · 2026-02-06T07:49:02Z

src/rrdEventProcess.c

+                        {
+                            free(dynamicprofiledata->rfcvalue);
+                        }
+                        if (dynamicprofiledata->command != NULL)


Coverity issue no longer present as of: undefined

Show issue

Coverity Issue - Dereference before null check

Null-checking "dynamicprofiledata->command" suggests that it may be null, but it has already been dereferenced on all paths leading to the check.

Medium Impact, CWE-476
REVERSE_INULL

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

Copilot · 2026-02-06T08:23:30Z

src/rrdEventProcess.c

+                            RDK_LOG(RDK_LOG_ERROR, LOG_REMDEBUG, "[%s:%d]: Memory Allocation Failed... \n", __FUNCTION__, __LINE__);
+                            free(pIssueNode); // free issue node struct on realloc failure


Missing cleanup for dynamicprofiledata and staticprofiledata when realloc fails. After logging the memory allocation error at line 198, only pIssueNode is freed. However, both dynamicprofiledata and staticprofiledata contain allocated memory that needs to be freed (their rfcvalue and command fields). This causes memory leaks when realloc fails.

Suggested change

RDK_LOG(RDK_LOG_ERROR, LOG_REMDEBUG, "[%s:%d]: Memory Allocation Failed... \n", __FUNCTION__, __LINE__);

free(pIssueNode); // free issue node struct on realloc failure

RDK_LOG(RDK_LOG_ERROR, LOG_REMDEBUG, "[%s:%d]: Memory Allocation Failed... \n", __FUNCTION__, __LINE__);

/* Cleanup on realloc failure to avoid memory leaks */

if (dynamicprofiledata != NULL)

{

if (dynamicprofiledata->rfcvalue != NULL)

{

free(dynamicprofiledata->rfcvalue);

}

if (dynamicprofiledata->command != NULL)

{

free(dynamicprofiledata->command);

}

free(dynamicprofiledata);

}

if (staticprofiledata != NULL)

{

if (staticprofiledata->rfcvalue != NULL)

{

free(staticprofiledata->rfcvalue);

}

if (staticprofiledata->command != NULL)

{

free(staticprofiledata->command);

}

free(staticprofiledata);

}

free(pIssueNode); // free issue node struct on realloc failure

return;

Copilot · 2026-02-06T08:23:30Z

src/rrdEventProcess.c

                        if (appendcommandstr == NULL) {
-                            RDK_LOG(RDK_LOG_DEBUG, LOG_REMDEBUG, "[%s:%d]: Memory Allocation Failed... \n", __FUNCTION__, __LINE__);
+                            RDK_LOG(RDK_LOG_ERROR, LOG_REMDEBUG, "[%s:%d]: Memory Allocation Failed... \n", __FUNCTION__, __LINE__);
+                            free(pIssueNode); // free issue node struct on realloc failure
+                        }


Critical memory leak when realloc fails. Both staticprofiledata and dynamicprofiledata contain allocated memory (rfcvalue and command fields) that must be freed when realloc fails. The current code only frees pIssueNode. Add cleanup code similar to lines 212-220 here to free both profile data structures and their members. Additionally, staticprofiledata->command should be freed since realloc failure means the old pointer is still valid.

github-actions · 2026-02-06T08:47:50Z

Code Coverage Summary

                               Total:|83.2%   5126|97.6%  1301|    -      0

src/rrdEventProcess.c

github-actions · 2026-02-09T06:17:51Z

Code Coverage Summary

                               Total:|83.0%   5136|97.6%  1301|    -      0

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.

Comments suppressed due to low confidence (1)

src/rrdInterface.c:348

dataMsg is freed only in the empty-message branch. In the non-empty case, dataMsg is passed to pushIssueTypesToMsgQueue and never freed anywhere in the current flow, which leaks per event. Either make pushIssueTypesToMsgQueue deep-copy the string so the caller can always free dataMsg, or ensure the consumer frees the message buffer after processing (but keep ownership consistent to avoid double-free).

    strncpy(dataMsg, rbusValue_GetString(value, NULL), len-1);
    dataMsg[len-1]='\0';

    if (dataMsg[0] == '\0' || len <= 0  )
    {
        RDK_LOG(RDK_LOG_DEBUG,LOG_REMDEBUG,"[%s:%d]: Message Received is empty, Exit Processing!!! \n", __FUNCTION__, __LINE__);
        free(dataMsg);
    }
    else
    {
        pushIssueTypesToMsgQueue(dataMsg, EVENT_MSG);
    }

Copilot · 2026-02-09T06:23:38Z

src/rrdEventProcess.c

                if (dynamicprofiledata == NULL)
                {
                    RDK_LOG(RDK_LOG_INFO, LOG_REMDEBUG, "[%s:%d]: Dynamic Profie Info not found, Download RDM package... \n", __FUNCTION__, __LINE__);
+                    free(pIssueNode); // free issue node struct when dynamic profile not found
                }


In the dynamicprofiledata == NULL path, pIssueNode is freed but pIssueNode->Node / pIssueNode->subNode allocated by getIssueInfo() are not freed, which leaks memory on this early-exit path. Free the node strings (when non-null) before freeing pIssueNode, or centralize cleanup in a helper.

Copilot · 2026-02-09T06:23:39Z

src/rrdEventProcess.c

+                        size_t appendstrlen = (staticstrlen + dynamicstrlen + 1);
                        char *appendcommandstr = (char *)realloc(staticprofiledata->command, appendstrlen);
                        if (appendcommandstr == NULL) {
-                            RDK_LOG(RDK_LOG_DEBUG, LOG_REMDEBUG, "[%s:%d]: Memory Allocation Failed... \n", __FUNCTION__, __LINE__);
+                            RDK_LOG(RDK_LOG_ERROR, LOG_REMDEBUG, "[%s:%d]: Memory Allocation Failed... \n", __FUNCTION__, __LINE__);
+                            // Free staticprofiledata on realloc failure
+                            if (staticprofiledata->rfcvalue != NULL)
+                            {
+                                free(staticprofiledata->rfcvalue);
+                            }


The realloc failure branch frees dynamicprofiledata/pIssueNode, but execution continues after the if (appendcommandstr == NULL) block and later unconditionally frees dynamicprofiledata again ("Free dynamicprofiledata after use"), which can lead to a double-free. Add an early return/goto-cleanup after handling realloc failure, and ensure dynamicprofiledata is freed exactly once.

Copilot · 2026-02-09T06:23:39Z

src/rrdEventProcess.c

                    staticprofiledata = processIssueTypeInStaticProfileappend(rbuf, pIssueNode);
                    if (staticprofiledata == NULL)
                    {
                        RDK_LOG(RDK_LOG_INFO, LOG_REMDEBUG, "[%s:%d]: Static Command Info not found for IssueType!!! \n", __FUNCTION__, __LINE__);
+                        // Free dynamicprofiledata since we can't proceed
+                        if (dynamicprofiledata->rfcvalue != NULL)
+                        {
+                            free(dynamicprofiledata->rfcvalue);
+                        }
+                        if (dynamicprofiledata->command != NULL)
+                        {
+                            free(dynamicprofiledata->command);
+                        }
+                        free(dynamicprofiledata);
+                        free(pIssueNode); // free issue node struct
                    }


In the staticprofiledata == NULL path, pIssueNode is freed but its Node/subNode members allocated by getIssueInfo() are not freed, causing a leak. Free pIssueNode->Node and pIssueNode->subNode (if set) before freeing pIssueNode here.

Copilot · 2026-02-09T06:23:39Z

src/rrdInterface.c

        {
            RDK_LOG(RDK_LOG_DEBUG, LOG_REMDEBUG, "[%s:%d]:Received command apppend request for the issue \n", __FUNCTION__, __LINE__);
            sbuf->appendMode = true;
        }	
        RRDMsgDeliver(msqid, sbuf);
        RDK_LOG(RDK_LOG_INFO, LOG_REMDEBUG, "[%s:%d]: SUCCESS: Message sending Done, ID=%d MSG=%s Size=%d Type=%u AppendMode=%d! \n", __FUNCTION__, __LINE__, msqid, sbuf->mdata, strlen(sbuf->mdata), sbuf->mtype, sbuf->appendMode);
+        /* coverity[leaked_storage] */
    }


pushIssueTypesToMsgQueue allocates sbuf and enqueues it, but there is no corresponding free of sbuf/sbuf->mdata in the consumer path (and this function does not free it either). Adding /* coverity[leaked_storage] */ here suppresses a real unbounded leak in a long-running daemon. Prefer defining clear ownership and freeing the message buffer after processing (e.g., call RRD_data_buff_deAlloc in the receiver thread once processing is complete, or make the queue payload a deep copy and free the original here).

Copilot · 2026-02-09T06:23:39Z

src/rrdInterface.c

    	RDK_LOG(RDK_LOG_DEBUG, LOG_REMDEBUG, "[%s:%d]: IssueType: %s... jsonPath: %s... \n", __FUNCTION__, __LINE__, (char *)sendbuf->mdata, sendbuf->jsonPath);
    	RDK_LOG(RDK_LOG_DEBUG, LOG_REMDEBUG, "[%s:%d]: Copying Message Received to the queue.. \n", __FUNCTION__, __LINE__);
    	RRDMsgDeliver(msqid, sendbuf);
 	RDK_LOG(RDK_LOG_INFO, LOG_REMDEBUG, "[%s:%d]: SUCCESS: Message sending Done, ID=%d MSG=%s Size=%d Type=%u AppendMode=%d! \n", __FUNCTION__, __LINE__, msqid, sendbuf->mdata, strlen(sendbuf->mdata), sendbuf->mtype, sendbuf->appendMode);
+	/* coverity[leaked_storage] */
 	remove_item(cache);
    }


In _rdmDownloadEventHandler, sendbuf is heap-allocated and passed to RRDMsgDeliver, but there is no matching deallocation shown for sendbuf after the message is processed. The added /* coverity[leaked_storage] */ suppression doesn’t address the underlying leak. Please establish where ownership is released (e.g., free in the receiver after handling the message, or change the queue to transmit a copy rather than a pointer).

Copilot · 2026-02-09T06:23:40Z

src/unittest/rrdUnitTestRunner.cpp

    char buf[] = "rfcvalue123";
    issueData* result = getIssueCommandInfo(&node, root, buf);
-    ASSERT_NE(result, nullptr);
+    ASSERT_EQ(result, nullptr);



ReturnsValidStruct now asserts result == nullptr, but the test JSON includes "kill" and the Sanity.Check.Commands list contains "kill", so getIssueCommandInfo correctly returns NULL (harmful command). Either rename the test to reflect the harmful-command case, or update the JSON to use a non-harmful command and assert result != nullptr. Also ensure root is deleted (and result freed when non-null) to avoid leaking test allocations.

rdkcmf-jenkins · 2026-02-09T06:27:08Z

src/rrdEventProcess.c

-                        RDK_LOG(RDK_LOG_DEBUG,LOG_REMDEBUG,"[%s:%d]: Executing Commands in Runtime Service... \n",__FUNCTION__,__LINE__);
-                        checkIssueNodeInfo(pIssueNode, NULL, rbuf, false, staticprofiledata); 
+                        // Free dynamicprofiledata after use
+                        if (dynamicprofiledata->rfcvalue != NULL)


Coverity issue no longer present as of: undefined

Show issue

Coverity Issue - Read from pointer after free

Dereferencing freed pointer "dynamicprofiledata".

High Impact, CWE-416
USE_AFTER_FREE

rdkcmf-jenkins · 2026-02-09T06:27:08Z

src/rrdEventProcess.c

+                            {
+                                free(staticprofiledata->rfcvalue);
+                            }
+                            if (staticprofiledata->command != NULL)


Coverity issue no longer present as of: undefined

Show issue

Coverity Issue - Dereference before null check

Null-checking "staticprofiledata->command" suggests that it may be null, but it has already been dereferenced on all paths leading to the check.

Medium Impact, CWE-476
REVERSE_INULL

rdkcmf-jenkins · 2026-02-09T06:27:08Z

src/rrdEventProcess.c

+                            {
+                                free(dynamicprofiledata->rfcvalue);
+                            }
+                            if (dynamicprofiledata->command != NULL)


Coverity issue no longer present as of: undefined

Show issue

Coverity Issue - Dereference before null check

Null-checking "dynamicprofiledata->command" suggests that it may be null, but it has already been dereferenced on all paths leading to the check.

Medium Impact, CWE-476
REVERSE_INULL

github-actions · 2026-02-09T06:29:55Z

Code Coverage Summary

                               Total:|83.1%   5132|97.6%  1301|    -      0

github-actions · 2026-02-09T09:11:47Z

Code Coverage Summary

                               Total:|82.8%   5148|97.6%  1301|    -      0

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

Copilot · 2026-02-09T09:16:05Z

src/rrdEventProcess.c

                    staticprofiledata = processIssueTypeInStaticProfileappend(rbuf, pIssueNode);
                    if (staticprofiledata == NULL)
                    {
                        RDK_LOG(RDK_LOG_INFO, LOG_REMDEBUG, "[%s:%d]: Static Command Info not found for IssueType!!! \n", __FUNCTION__, __LINE__);
+                        // Free dynamicprofiledata since we can't proceed
+                        if (dynamicprofiledata != NULL)
+                        {
+                            if (dynamicprofiledata->rfcvalue != NULL)
+                            {
+                                free(dynamicprofiledata->rfcvalue);
+                            }
+                            if (dynamicprofiledata->command != NULL)
+                            {
+                                free(dynamicprofiledata->command);
+                            }
+                            free(dynamicprofiledata);
+                        }
+                        free(pIssueNode); // free issue node struct
                    }


When staticprofiledata == NULL, the code frees dynamicprofiledata and pIssueNode (struct) but does not free pIssueNode->Node/subNode (which are still allocated in this path) and does not clean up the incoming rbuf message buffer. This leaves leaks on the failure path; add a cleanup that frees the issue node members and the message buffer when you can’t proceed.

Copilot · 2026-02-09T09:16:06Z

src/rrdJsonParser.c

 	    }
        }
-        free(rfcbuf); // free rfc value
+        // Note: rfcbuf is owned by caller and will be freed there


processAllDebugCommand no longer frees rfcbuf (to avoid a double-free from checkIssueNodeInfo), but the function’s doc comment still doesn’t state the ownership contract, and existing unit tests pass a strdup’d buffer without freeing it. Please update the function/header documentation to explicitly say the caller owns rfcbuf (and ensure tests/callers free it accordingly).

Suggested change

// Note: rfcbuf is owned by caller and will be freed there

// Note: rfcbuf is owned by the caller; this function must not free it.

// The caller is responsible for freeing rfcbuf after this function returns.

Copilot · 2026-02-09T09:16:06Z

src/rrdEventProcess.c

+                        // Free staticprofiledata after use (only if not already freed)
+                        if (staticprofiledata != NULL)
+                        {
+                            if (staticprofiledata->rfcvalue != NULL)
+                            {
+                                free(staticprofiledata->rfcvalue);
+                            }
+                            if (staticprofiledata->command != NULL)
+                            {
+                                free(staticprofiledata->command);
+                            }
+                            free(staticprofiledata);
                        }


In the append-mode path, checkIssueNodeInfo(..., appendprofiledata) calls executeCommands(appendprofiledata), and executeCommands frees the passed issueData in non-test builds. Freeing staticprofiledata here will double-free/use-after-free in production. Avoid freeing staticprofiledata after checkIssueNodeInfo (or change ownership so executeCommands doesn’t free its input) and ensure the pointer is not reused.

Suggested change

// Free staticprofiledata after use (only if not already freed)

if (staticprofiledata != NULL)

{

if (staticprofiledata->rfcvalue != NULL)

{

free(staticprofiledata->rfcvalue);

}

if (staticprofiledata->command != NULL)

{

free(staticprofiledata->command);

}

free(staticprofiledata);

}

/*

* Do not free staticprofiledata here.

* Ownership is transferred to checkIssueNodeInfo/executeCommands,

* which frees issueData in non-test builds. Freeing it again here

* would cause a double free/use-after-free.

*/

staticprofiledata = NULL;

Copilot · 2026-02-09T09:16:06Z

src/rrdEventProcess.c

                if (dynamicprofiledata == NULL)
                {
                    RDK_LOG(RDK_LOG_INFO, LOG_REMDEBUG, "[%s:%d]: Dynamic Profie Info not found, Download RDM package... \n", __FUNCTION__, __LINE__);
+                    free(pIssueNode); // free issue node struct when dynamic profile not found
                }


When dynamicprofiledata == NULL, the code frees only pIssueNode (struct) and returns to the caller without freeing the node members (pIssueNode->Node / pIssueNode->subNode) or the incoming rbuf data. processIssueTypeInDynamicProfileappend can return NULL without calling RRDRdmManagerDownloadRequest (e.g., allocation failure or issue not found), so the members will leak in those cases. Ensure consistent ownership/cleanup here (free members + struct, and free the message buffer if it won’t be processed further).

rdkcmf-jenkins · 2026-02-09T09:24:29Z

src/rrdEventProcess.c

+                                {
+                                    free(staticprofiledata->rfcvalue);
+                                }
+                                if (staticprofiledata->command != NULL)


Coverity issue no longer present as of: undefined

Show issue

Coverity Issue - Dereference before null check

Null-checking "staticprofiledata->command" suggests that it may be null, but it has already been dereferenced on all paths leading to the check.

Medium Impact, CWE-476
REVERSE_INULL

rdkcmf-jenkins · 2026-02-09T09:24:29Z

src/rrdEventProcess.c

+                                {
+                                    free(dynamicprofiledata->rfcvalue);
+                                }
+                                if (dynamicprofiledata->command != NULL)


Coverity issue no longer present as of: undefined

Show issue

Coverity Issue - Dereference before null check

Null-checking "dynamicprofiledata->command" suggests that it may be null, but it has already been dereferenced on all paths leading to the check.

Medium Impact, CWE-476
REVERSE_INULL

github-actions · 2026-02-09T10:46:52Z

Code Coverage Summary

                               Total:|82.7%   5157|97.6%  1301|    -      0

nhanasi · 2026-02-09T19:41:12Z

src/rrdEventProcess.c

                if (dynamicprofiledata == NULL)
                {
                    RDK_LOG(RDK_LOG_INFO, LOG_REMDEBUG, "[%s:%d]: Dynamic Profie Info not found, Download RDM package... \n", __FUNCTION__, __LINE__);
+                    free(pIssueNode); // free issue node struct when dynamic profile not found


can u check dynamic test on RDKB and RDKE?

nhanasi · 2026-02-09T20:52:28Z

src/rrdEventProcess.c

    { // Static Profile JSON Parsing or Read Fail
        RDK_LOG(RDK_LOG_ERROR, LOG_REMDEBUG, "[%s:%d]: Static Profile Parse/Read failed... %s\n", __FUNCTION__, __LINE__, RRD_JSON_FILE);
        processIssueTypeInInstalledPackage(rbuf, pIssueNode);
+        RDK_LOG(RDK_LOG_DEBUG, LOG_REMDEBUG, "[%s:%d]: ...Exiting...\n", __FUNCTION__, __LINE__);


Not needed as we go for dynamic download

github-actions · 2026-02-10T05:02:04Z

Code Coverage Summary

                               Total:|82.8%   5150|97.6%  1301|    -      0

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Copilot · 2026-02-10T05:07:59Z

src/rrdEventProcess.c

+            }
        }
    }



processIssueTypeEvent() allocates/receives data_buf* rbuf via the message queue, but the function never frees rbuf (and doesn’t free rbuf->mdata/rbuf->jsonPath either). Since RRDMsgDeliver() passes pointers (not copies), this is a real heap leak per message and likely the root cause behind the Coverity leaked_storage suppressions. Please add a single well-defined deallocation point (e.g., call RRD_data_buff_deAlloc(rbuf) once the message has been fully processed, possibly in RRDEventThreadFunc after each switch case).

Suggested change

if (rbuf != NULL)

{

RRD_data_buff_deAlloc(rbuf);

}

Copilot · 2026-02-10T05:07:59Z

src/rrdEventProcess.c

+                                // NOTE: staticprofiledata is freed by executeCommands() via checkIssueNodeInfo()
+                                // Do NOT free staticprofiledata here to avoid double-free


In the append-mode path, checkIssueNodeInfo(pIssueNode, ...) frees pIssueNode->Node/subNode but pIssueNode itself is never freed in this function, so each message leaks the issueNodeData allocation. Also, the comment that staticprofiledata “is freed by executeCommands()” is only true in non-GTEST builds (the frees are under #if !defined(GTEST_ENABLE) in executeCommands()), which makes the ownership expectation ambiguous and can hide leaks in test builds. Please make ownership explicit and free pIssueNode after checkIssueNodeInfo() returns.

Suggested change

// NOTE: staticprofiledata is freed by executeCommands() via checkIssueNodeInfo()

// Do NOT free staticprofiledata here to avoid double-free

/* In append mode, checkIssueNodeInfo() frees pIssueNode->Node/subNode,

* but not the issueNodeData container itself. Free it here to avoid

* leaking the allocation.

*/

if (pIssueNode != NULL)

{

free(pIssueNode);

pIssueNode = NULL;

}

/* Ownership of staticprofiledata:

* - In non-GTEST builds, staticprofiledata is freed by executeCommands()

* (invoked via checkIssueNodeInfo()).

* - In GTEST builds, executeCommands() does not free it, so clean it up here.

*/

#ifdef GTEST_ENABLE

if (staticprofiledata != NULL)

{

if (staticprofiledata->rfcvalue != NULL)

{

free(staticprofiledata->rfcvalue);

}

if (staticprofiledata->command != NULL)

{

free(staticprofiledata->command);

}

free(staticprofiledata);

staticprofiledata = NULL;

}

#endif

Copilot · 2026-02-10T05:08:00Z

src/rrdJsonParser.c

+    {
+        RDK_LOG(RDK_LOG_ERROR,LOG_REMDEBUG,"[%s:%d]: Memory allocation failed for rfcbuf\n",__FUNCTION__,__LINE__);
+        free(buff->mdata); // free rfc data
+        free(buff->jsonPath); // free rrd path info


The early-return on rfcbuf == NULL frees buff->mdata / buff->jsonPath but does not free issuestructNode->Node / issuestructNode->subNode (and potentially the issuestructNode container, depending on ownership in the caller). Other exit paths free these members, so this new branch introduces an inconsistent cleanup path and leaks on allocation failure. Please align cleanup here with the rest of checkIssueNodeInfo()’s ownership expectations.

Suggested change

free(buff->jsonPath); // free rrd path info

free(buff->jsonPath); // free rrd path info

if (issuestructNode != NULL)

{

free(issuestructNode->Node);

issuestructNode->Node = NULL;

free(issuestructNode->subNode);

issuestructNode->subNode = NULL;

}

github-actions · 2026-02-10T07:04:46Z

Code Coverage Summary

                               Total:|82.8%   5150|97.6%  1301|    -      0

github-actions · 2026-02-10T07:10:28Z

Code Coverage Summary

                               Total:|82.9%   5150|97.6%  1301|    -      0

github-actions · 2026-02-10T07:14:18Z

Code Coverage Summary

                               Total:|82.8%   5150|97.6%  1301|    -      0

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.

Copilot · 2026-02-10T07:18:19Z

src/rrdJsonParser.c

    if(issuestdata->command == NULL)
    {
        RDK_LOG(RDK_LOG_ERROR,LOG_REMDEBUG,"[%s:%d]: No Commands found, exiting.. \n",__FUNCTION__,__LINE__);
        free(issuestdata);
+        return NULL;


getIssueCommandInfo() now returns NULL when no commands are found. There are existing callers that assume the return is non-NULL and dereference it for logging (e.g., processIssueTypeInDynamicProfileappend/processIssueTypeInStaticProfileappend in rrdEventProcess.c), which can now crash. Update callers to handle NULL (or redesign this API to return a status + out-params).

Copilot · 2026-02-10T07:18:20Z

src/rrdInterface.c

    {
        pushIssueTypesToMsgQueue(dataMsg, EVENT_MSG);
+        /* coverity[leaked_storage] */
    }


The added Coverity suppression here is masking a real leak/ownership issue: dataMsg is heap-allocated and passed into the queue, but the receiver path does not consistently deallocate the queued data_buf and its mdata. Please fix the ownership end-to-end (free after consumption) instead of suppressing.

Copilot · 2026-02-10T07:18:20Z

src/rrdInterface.c

        }	
        RRDMsgDeliver(msqid, sbuf);
        RDK_LOG(RDK_LOG_INFO, LOG_REMDEBUG, "[%s:%d]: SUCCESS: Message sending Done, ID=%d MSG=%s Size=%d Type=%u AppendMode=%d! \n", __FUNCTION__, __LINE__, msqid, sbuf->mdata, strlen(sbuf->mdata), sbuf->mtype, sbuf->appendMode);
+        /* coverity[leaked_storage] */
    }


pushIssueTypesToMsgQueue() allocates sbuf and enqueues it, but the consumer side currently doesn’t free the data_buf after processing. The Coverity suppression here hides a growing leak in a long-running daemon; ensure the message buffer is freed after it is consumed (and clarify/transfer ownership of mdata/jsonPath safely).

Copilot · 2026-02-10T07:18:20Z

src/rrdIarmEvents.c

                    RRDMsgDeliver(msqid, sendbuf);
                    RDK_LOG(RDK_LOG_INFO, LOG_REMDEBUG, "[%s:%d]: SUCCESS: Message sending Done, ID=%d MSG=%s Size=%d Type=%u AppendMode=%d! \n", __FUNCTION__, __LINE__, msqid, sendbuf->mdata, strlen(sendbuf->mdata), sendbuf->mtype, sendbuf->appendMode);
+                    /* coverity[leaked_storage] */
                }


This Coverity suppression appears to be masking that sendbuf (and its mdata/jsonPath) is heap-allocated and queued via RRDMsgDeliver() without a clear/freeing path afterward. Please fix the ownership end-to-end (free the message buffer after it is consumed) rather than suppressing, otherwise the daemon leaks on every completed install event.

Copilot · 2026-02-10T07:18:20Z

src/unittest/rrdUnitTestRunner.cpp

    char buf[] = "rfcvalue123";
    issueData* result = getIssueCommandInfo(&node, root, buf);
-    ASSERT_NE(result, nullptr);
+    ASSERT_EQ(result, nullptr);



This test now expects getIssueCommandInfo() to return NULL, but the name ReturnsValidStruct is misleading and root isn’t cleaned up on this path. Consider renaming the test to reflect the harmful-command case and add cJSON_Delete(root) before returning.

github-actions · 2026-02-10T07:20:57Z

Code Coverage Summary

                               Total:|82.8%   5150|97.6%  1301|    -      0

github-actions · 2026-02-10T07:23:47Z

Code Coverage Summary

                               Total:|82.8%   5150|97.6%  1301|    -      0

github-actions · 2026-02-10T07:25:31Z

Code Coverage Summary

                               Total:|82.8%   5150|97.6%  1301|    -      0

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

src/rrdEventProcess.c:123

processIssueTypeEvent() allocates/receives a heap data_buf* rbuf from the message queue but never deallocates it (nor rbuf->mdata) before returning. This makes the /* coverity[leaked_storage] */ suppressions elsewhere more likely to be masking a real leak. Consider establishing a clear ownership rule (e.g., consumer always calls RRD_data_buff_deAlloc(rbuf) after processing) and avoid freeing mdata/jsonPath in deep callees if the caller is responsible.

                free(cmdMap);
		cmdMap = NULL;
	    }
        }
    }

Copilot · 2026-02-10T07:32:37Z

src/rrdInterface.c

    else
    {
        pushIssueTypesToMsgQueue(dataMsg, EVENT_MSG);
+        /* coverity[leaked_storage] */
    }


In _remoteDebuggerEventHandler, dataMsg is heap-allocated and passed to pushIssueTypesToMsgQueue() with a /* coverity[leaked_storage] */ suppression. This is only safe if the message-queue consumer reliably frees the data_buf (and its mdata) after processing; otherwise this masks a real leak. Please ensure there is a single clear owner for dataMsg/data_buf and that the consumer deallocates it (or copy the string and free here).

Copilot · 2026-02-10T07:32:38Z

src/rrdJsonParser.c

    {
        RDK_LOG(RDK_LOG_ERROR,LOG_REMDEBUG,"[%s:%d]: No Commands found, exiting.. \n",__FUNCTION__,__LINE__);
        free(issuestdata);
+        return NULL;
    }


getIssueCommandInfo() now returns NULL when no command is found or when the sanity check fails. There are callers in rrdEventProcess.c (e.g., the *Profileappend() helpers) that dereference the returned pointer unconditionally for logging; with these NULL returns they will now crash. Please update those call sites to handle a NULL return before dereferencing/logging.

github-actions · 2026-02-10T07:44:54Z

Code Coverage Summary

                               Total:|82.8%   5150|97.6%  1301|    -      0

madhubabutt requested a review from a team as a code owner February 3, 2026 14:50

Copilot AI review requested due to automatic review settings February 3, 2026 14:50

Copilot started reviewing on behalf of madhubabutt February 3, 2026 14:51 View session

Copilot AI reviewed Feb 3, 2026

View reviewed changes

madhubabutt force-pushed the feature/RDK-60307 branch from c35d07f to d8c0cd1 Compare February 6, 2026 07:37

rdkcmf-jenkins reviewed Feb 6, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings February 6, 2026 08:17

madhubabutt force-pushed the feature/RDK-60307 branch from d8c0cd1 to c90d902 Compare February 6, 2026 08:17

Copilot started reviewing on behalf of madhubabutt February 6, 2026 08:17 View session

Copilot AI reviewed Feb 6, 2026

View reviewed changes

madhubabutt force-pushed the feature/RDK-60307 branch from c90d902 to 536c48f Compare February 6, 2026 08:46

madhubabutt force-pushed the feature/RDK-60307 branch from 536c48f to c2e1e4e Compare February 9, 2026 06:16

Copilot AI review requested due to automatic review settings February 9, 2026 06:16

Copilot started reviewing on behalf of madhubabutt February 9, 2026 06:17 View session

github-advanced-security bot found potential problems Feb 9, 2026

View reviewed changes

src/rrdEventProcess.c Fixed Show fixed Hide fixed

src/rrdEventProcess.c Fixed Show fixed Hide fixed

src/rrdEventProcess.c Fixed Show fixed Hide fixed

Copilot AI reviewed Feb 9, 2026

View reviewed changes

rdkcmf-jenkins reviewed Feb 9, 2026

View reviewed changes

madhubabutt force-pushed the feature/RDK-60307 branch from c2e1e4e to 9689c88 Compare February 9, 2026 06:28

Copilot AI review requested due to automatic review settings February 9, 2026 09:10

madhubabutt force-pushed the feature/RDK-60307 branch from 9689c88 to b80690a Compare February 9, 2026 09:10

Copilot started reviewing on behalf of madhubabutt February 9, 2026 09:10 View session

Copilot AI reviewed Feb 9, 2026

View reviewed changes

rdkcmf-jenkins reviewed Feb 9, 2026

View reviewed changes

madhubabutt force-pushed the feature/RDK-60307 branch from b80690a to 36d1c83 Compare February 9, 2026 10:45

nhanasi reviewed Feb 9, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings February 10, 2026 05:00

madhubabutt force-pushed the feature/RDK-60307 branch from 36d1c83 to 86cf29d Compare February 10, 2026 05:00

Copilot started reviewing on behalf of madhubabutt February 10, 2026 05:01 View session

Copilot AI reviewed Feb 10, 2026

View reviewed changes

madhubabutt force-pushed the feature/RDK-60307 branch from 86cf29d to c310ede Compare February 10, 2026 07:03

Copilot AI review requested due to automatic review settings February 10, 2026 07:09

madhubabutt force-pushed the feature/RDK-60307 branch from c310ede to 9be5e0c Compare February 10, 2026 07:09

Copilot started reviewing on behalf of madhubabutt February 10, 2026 07:09 View session

madhubabutt force-pushed the feature/RDK-60307 branch from 9be5e0c to ac5f76c Compare February 10, 2026 07:13

Copilot AI reviewed Feb 10, 2026

View reviewed changes

madhubabutt force-pushed the feature/RDK-60307 branch from ac5f76c to 715bc8a Compare February 10, 2026 07:19

Copilot AI review requested due to automatic review settings February 10, 2026 07:22

madhubabutt force-pushed the feature/RDK-60307 branch from 715bc8a to a502adb Compare February 10, 2026 07:22

Copilot started reviewing on behalf of madhubabutt February 10, 2026 07:23 View session

madhubabutt force-pushed the feature/RDK-60307 branch from a502adb to 46d180a Compare February 10, 2026 07:24

Copilot AI reviewed Feb 10, 2026

View reviewed changes

RDK-60307 [RRD] RDK Coverity Defect Resolution for Device Management

2e4ce7c

madhubabutt force-pushed the feature/RDK-60307 branch from 46d180a to 2e4ce7c Compare February 10, 2026 07:43

		RDK_LOG(RDK_LOG_ERROR, LOG_REMDEBUG, "[%s:%d]: Memory Allocation Failed... \n", __FUNCTION__, __LINE__);
		free(pIssueNode); // free issue node struct on realloc failure

	// Note: rfcbuf is owned by caller and will be freed there
	// Note: rfcbuf is owned by the caller; this function must not free it.
	// The caller is responsible for freeing rfcbuf after this function returns.

+    if (rbuf != NULL)
+    {
+        RRD_data_buff_deAlloc(rbuf);
+    }

		// NOTE: staticprofiledata is freed by executeCommands() via checkIssueNodeInfo()
		// Do NOT free staticprofiledata here to avoid double-free

-                                // NOTE: staticprofiledata is freed by executeCommands() via checkIssueNodeInfo()
-                                // Do NOT free staticprofiledata here to avoid double-free
+                                /* In append mode, checkIssueNodeInfo() frees pIssueNode->Node/subNode,
+                                 * but not the issueNodeData container itself. Free it here to avoid
+                                 * leaking the allocation.
+                                 */
+                                if (pIssueNode != NULL)
+                                {
+                                    free(pIssueNode);
+                                    pIssueNode = NULL;
+                                }
+                                /* Ownership of staticprofiledata:
+                                 * - In non-GTEST builds, staticprofiledata is freed by executeCommands()
+                                 *   (invoked via checkIssueNodeInfo()).
+                                 * - In GTEST builds, executeCommands() does not free it, so clean it up here.
+                                 */
+#ifdef GTEST_ENABLE
+                                if (staticprofiledata != NULL)
+                                {
+                                    if (staticprofiledata->rfcvalue != NULL)
+                                    {
+                                        free(staticprofiledata->rfcvalue);
+                                    }
+                                    if (staticprofiledata->command != NULL)
+                                    {
+                                        free(staticprofiledata->command);
+                                    }
+                                    free(staticprofiledata);
+                                    staticprofiledata = NULL;
+                                }
+#endif

-        free(buff->jsonPath); // free rrd path info
+        free(buff->jsonPath); // free rrd path info
+        if (issuestructNode != NULL)
+        {
+            free(issuestructNode->Node);
+            issuestructNode->Node = NULL;
+            free(issuestructNode->subNode);
+            issuestructNode->subNode = NULL;
+        }

RDK-60307 [RRD] RDK Coverity Defect Resolution for Device Management #175

Are you sure you want to change the base?

RDK-60307 [RRD] RDK Coverity Defect Resolution for Device Management #175

Conversation

madhubabutt commented Feb 3, 2026

Uh oh!

github-actions bot commented Feb 3, 2026

Code Coverage Summary

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

Uh oh!

rdkcmf-jenkins commented Feb 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Coverity Issue - Resource leak

Issue location

Uh oh!

rdkcmf-jenkins Feb 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Coverity Issue - Dereference before null check

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Feb 6, 2026

Code Coverage Summary

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Feb 9, 2026

Code Coverage Summary

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

rdkcmf-jenkins Feb 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Coverity Issue - Read from pointer after free

Uh oh!

rdkcmf-jenkins Feb 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Coverity Issue - Dereference before null check

rdkcmf-jenkins commented Feb 3, 2026 •

edited

Loading

rdkcmf-jenkins Feb 6, 2026 •

edited

Loading

rdkcmf-jenkins Feb 9, 2026 •

edited

Loading

rdkcmf-jenkins Feb 9, 2026 •

edited

Loading

rdkcmf-jenkins Feb 9, 2026 •

edited

Loading

rdkcmf-jenkins Feb 9, 2026 •

edited

Loading

rdkcmf-jenkins Feb 9, 2026 •

edited

Loading