Coverity cleanup - LOCK_EVASION and MISSING LOCK #28

rirfha948 · 2025-10-14T05:24:58Z

Before COVERITY CLEANUP
Defect occurrences found : 434 Total
3 ATOMICITY
2 BAD_CHECK_OF_WAIT_COND
11 CHECKED_RETURN
2 CONSTANT_EXPRESSION_RESULT
2 COPY_PASTE_ERROR
87 DEADCODE
24 FORWARD_NULL
2 IDENTICAL_BRANCHES
44 INTEGER_OVERFLOW
14 LOCK_EVASION
30 MISSING_LOCK
1 NEGATIVE_RETURNS
3 NO_EFFECT
5 NULL_RETURNS
3 PW.INCLUDE_RECURSION
1 PW.PARAMETER_HIDDEN
24 RESOURCE_LEAK
7 REVERSE_INULL
3 SIZEOF_MISMATCH
2 SLEEP
8 STRING_OVERFLOW
17 TAINTED_SCALAR
4 UNREACHABLE
80 UNUSED_VALUE
1 USELESS_CALL
39 USE_AFTER_FREE
15 Y2K38_SAFETY

Analysis summary report:

Files analyzed : 1395 Total
C : 1395
Total LoC input to cov-analyze : 449133
Functions analyzed : 7353
Classes/structs analyzed : 909
Paths analyzed : 558951
Time taken by analysis : 00:00:14
Defect occurrences found : 415 Total
3 ATOMICITY
2 BAD_CHECK_OF_WAIT_COND
11 CHECKED_RETURN
2 CONSTANT_EXPRESSION_RESULT
2 COPY_PASTE_ERROR
87 DEADCODE
24 FORWARD_NULL
2 IDENTICAL_BRANCHES
43 INTEGER_OVERFLOW
1 LOCK
1 LOCK_EVASION
24 MISSING_LOCK
1 NEGATIVE_RETURNS
3 NO_EFFECT
5 NULL_RETURNS
3 PW.INCLUDE_RECURSION
1 PW.PARAMETER_HIDDEN
24 RESOURCE_LEAK
7 REVERSE_INULL
3 SIZEOF_MISMATCH
2 SLEEP
8 STRING_OVERFLOW
17 TAINTED_SCALAR
4 UNREACHABLE
80 UNUSED_VALUE
1 USELESS_CALL
39 USE_AFTER_FREE
15 Y2K38_SAFETY

Reason for change: Fixes for CcspCommonLibrary Coverity Fixes Test Procedure: - TBD Risks: None Priority: P3 Signed-off-by: rirfha948 <[email protected]>

…on-library into Coverity-cleanup

CLAassistant · 2025-10-14T05:25:06Z

All committers have signed the CLA.

Copilot

Pull Request Overview

This PR addresses Coverity static analysis findings by fixing lock ordering and missing lock issues. The changes reduce LOCK_EVASION defects from 14 to 1 and MISSING_LOCK defects from 30 to 24.

Key changes:

Fixed lock acquisition timing to prevent race conditions when checking shared data structures
Added missing lock releases on early return paths
Corrected lock ordering to prevent lock evasion issues

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
slap_oeo_management.c	Moved lock acquisition before pool depth check to prevent race condition
ansc_protection.c	Added lock around signal state check to prevent lock evasion
ansc_packet.c	Added missing locks before pool size checks and releases on early returns
ansc_http_parser.c	Added missing return statement in else branch
ansc_beto_management.c	Added lock around socket count check with proper release on early return
ansc_memory.c	Fixed lock ordering for initialization checks
sys_rfo_sorting.c	Added locks around array existence checks with proper releases

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

source/util_api/ansc/AnscPlatform/ansc_packet.c

source/util_api/slap/components/SlapObjEntity/slap_oeo_management.c

source/cosa/package/system/components/SysRepositoryFolder/sys_rfo_sorting.c

source/util_api/ansc/AnscPlatform/ansc_packet.c

source/util_api/ansc/AnscBrokerEngineTcp/ansc_beto_management.c

source/util_api/ansc/AnscPlatform/ansc_http_parser.c

Co-authored-by: Copilot <[email protected]>

Reason for change: Fixes for CcspCommonLibrary Coverity Fixes Test Procedure: - TBD Risks: None Priority: P3 Signed-off-by: rirfha948 <[email protected]>

Copilot

Pull Request Overview

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

Comments suppressed due to low confidence (1)

source/util_api/ansc/AnscPlatform/ansc_packet.c:344

The lock is released at line 337, but then immediately re-acquired at line 344 to perform the list push operation. Between these two lock operations, the shared resource is accessed without protection, creating a potential race condition where another thread could modify the pool between the release and re-acquire. The lock should be held continuously from the size check through the list push operation.

                AnscReleaseSpinLock(&g_qPdoPoolSpinLock);
#ifndef _ANSC_TRACE_PACKET_
                AnscPdoClean((ANSC_HANDLE)pPdo);
#else
                AnscPdoCleanTrace((ANSC_HANDLE)pPdo, pFileName, iLineNo);
#endif

                AnscAcquireSpinLock(&g_qPdoPoolSpinLock);

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-05T10:47:16Z

source/util_api/ansc/AnscPlatform/ansc_packet.c

+            AnscReleaseSpinLock(&g_qBdoPoolSpinLock);
            AnscBdoClean((ANSC_HANDLE)pBdo);

            AnscAcquireSpinLock(&g_qBdoPoolSpinLock);


The lock is released at line 1760, but then immediately re-acquired at line 1763 to perform the list push operation. This creates a potential race condition where another thread could modify the pool between the release and re-acquire. The lock should be held continuously from the size check through the list push operation.

Suggested change

AnscReleaseSpinLock(&g_qBdoPoolSpinLock);

AnscBdoClean((ANSC_HANDLE)pBdo);

AnscAcquireSpinLock(&g_qBdoPoolSpinLock);

AnscBdoClean((ANSC_HANDLE)pBdo);

Copilot · 2025-11-05T10:47:16Z

source/util_api/ansc/AnscPlatform/ansc_packet.c

+            AnscReleaseSpinLock(&g_qBdoPoolSpinLock);
            AnscBdoClean((ANSC_HANDLE)pSonBdo);

            AnscAcquireSpinLock(&g_qBdoPoolSpinLock);


The lock is released at line 2188, but then immediately re-acquired at line 2191 to perform the list push operation. This creates a potential race condition where another thread could modify the pool between the release and re-acquire. The lock should be held continuously from the size check through the list push operation.

Suggested change

AnscReleaseSpinLock(&g_qBdoPoolSpinLock);

AnscBdoClean((ANSC_HANDLE)pSonBdo);

AnscAcquireSpinLock(&g_qBdoPoolSpinLock);

AnscBdoClean((ANSC_HANDLE)pSonBdo);

/* Hold the lock from size check through push operation */

/* The lock is already held from line 2176 */

Copilot · 2025-11-05T10:47:16Z

source/util_api/ansc/AnscBrokerEngineTcp/ansc_beto_management.c

+    AnscAcquireLock   (&pMyObject->SocketTableLock);
    if ( pMyObject->CurSocketCount >= pMyObject->MaxSocketCount )
    {
+        AnscReleaseLock   (&pMyObject->SocketTableLock);
        return  (ANSC_HANDLE)NULL;
    }
+    AnscReleaseLock   (&pMyObject->SocketTableLock);


The lock is released at line 305 after checking CurSocketCount, but before the socket is actually created and added. This creates a race condition where multiple threads could pass the count check simultaneously and exceed MaxSocketCount. The lock should be held until after the socket is successfully added to ensure the count remains accurate.

Copilot · 2025-11-05T10:47:17Z

source/debug_api/ansc_memory.c

+    AnscInitializeSpinLock(&g_tCountSizeSpinLock);
+    AnscAcquireSpinLock(&g_tCountSizeSpinLock);


AnscInitializeSpinLock is called unconditionally on every allocation, which will reinitialize the lock even when it's already initialized. This could corrupt the lock state if another thread is concurrently using it. The initialization should only occur once, protected by a different synchronization mechanism (e.g., pthread_once or a separate initialization flag with atomic operations).

Copilot · 2025-11-05T10:47:17Z

source/debug_api/ansc_memory.c

+    AnscInitializeSpinLock(&g_tCountSizeSpinLock);
+
+    AnscAcquireSpinLock(&g_tCountSizeSpinLock);


AnscInitializeSpinLock is called unconditionally on every allocation, which will reinitialize the lock even when it's already initialized. This could corrupt the lock state if another thread is concurrently using it. The initialization should only occur once, protected by a different synchronization mechanism (e.g., pthread_once or a separate initialization flag with atomic operations).

Copilot · 2025-11-05T10:47:17Z

source/debug_api/ansc_memory.c

+    AnscInitializeSpinLock(&g_tRecordDetailSpinLock);
+    AnscInitializeMemoryAllocTable();
+
+    AnscAcquireSpinLock(&g_tRecordDetailSpinLock);


AnscInitializeSpinLock is called unconditionally on every allocation, which will reinitialize the lock even when it's already initialized. This could corrupt the lock state if another thread is concurrently using it. The initialization should only occur once, protected by a different synchronization mechanism (e.g., pthread_once or a separate initialization flag with atomic operations).

rirfha948 added 5 commits October 10, 2025 07:21

RDKB-00: Coverity Cleanup for CcspCommonLibrary

4e586cb

Reason for change: Fixes for CcspCommonLibrary Coverity Fixes Test Procedure: - TBD Risks: None Priority: P3 Signed-off-by: rirfha948 <[email protected]>

RDKB-00: Coverity Cleanup for CcspCommonLibrary

fccb89f

Reason for change: Fixes for CcspCommonLibrary Coverity Fixes Test Procedure: - TBD Risks: None Priority: P3 Signed-off-by: rirfha948 <[email protected]>

Merge branch 'Coverity-cleanup' of https://github.com/rdkcentral/comm…

bea2818

…on-library into Coverity-cleanup

LOCK-EVASION COVERITY FXIES

8ff48f9

LOCK_EVASION and MISSING_LOCK - COVERITY FIXES

9915543

rirfha948 requested a review from snayak002c October 14, 2025 05:24

rirfha948 requested review from a team as code owners October 14, 2025 05:24

snayak002c requested a review from Copilot October 27, 2025 18:20

Copilot AI reviewed Oct 27, 2025

View reviewed changes

rirfha948 and others added 8 commits October 28, 2025 09:45

White space addressal

ca93f24

Co-authored-by: Copilot <[email protected]>

White space addressal

e8eea5d

Co-authored-by: Copilot <[email protected]>

White space addressal

8f923c8

Co-authored-by: Copilot <[email protected]>

White space addressal

a70b906

Co-authored-by: Copilot <[email protected]>

White space addressal

a3e8c34

Co-authored-by: Copilot <[email protected]>

White space addressal

6945ed5

Co-authored-by: Copilot <[email protected]>

Apply suggestion from @Copilot

88c975e

Co-authored-by: Copilot <[email protected]>

RDKB-00: Coverity Cleanup for CcspCommonLibrary

8af2489

Reason for change: Fixes for CcspCommonLibrary Coverity Fixes Test Procedure: - TBD Risks: None Priority: P3 Signed-off-by: rirfha948 <[email protected]>

SanthoshGujulvajagadeesh requested a review from Copilot November 5, 2025 10:43

Copilot AI reviewed Nov 5, 2025

View reviewed changes

		AnscInitializeSpinLock(&g_tCountSizeSpinLock);
		AnscAcquireSpinLock(&g_tCountSizeSpinLock);

Coverity cleanup - LOCK_EVASION and MISSING LOCK #28

Are you sure you want to change the base?

Coverity cleanup - LOCK_EVASION and MISSING LOCK #28

Uh oh!

Conversation

rirfha948 commented Oct 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Analysis summary report:

Uh oh!

CLAassistant commented Oct 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Copilot AI Nov 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 5, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

rirfha948 commented Oct 14, 2025 •

edited

Loading

CLAassistant commented Oct 14, 2025 •

edited

Loading