Skip to content

RDKEMW-14533 : DO NOT MERGE - mgrChk#89

Open
PreethiLakshmi91 wants to merge 1 commit intordkcentral:developfrom
PreethiLakshmi91:RDKEMW-14533_Cov_f
Open

RDKEMW-14533 : DO NOT MERGE - mgrChk#89
PreethiLakshmi91 wants to merge 1 commit intordkcentral:developfrom
PreethiLakshmi91:RDKEMW-14533_Cov_f

Conversation

@PreethiLakshmi91
Copy link
Copy Markdown

Reason for change: Inclusion of coverity for BT
Test Procedure: NA
Risks: Low
Priority: P2

Copilot AI review requested due to automatic review settings April 20, 2026 11:35
@PreethiLakshmi91 PreethiLakshmi91 requested a review from a team as a code owner April 20, 2026 11:35
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR appears to adjust initialization of the local BTRMGR_DiscoveredDevicesList_t discoveredDevices structure in the battery discovery timer handler (likely motivated by Coverity findings).

Changes:

  • Modifies the memset call that clears discoveredDevices before calling BTRMGR_GetDiscoveredDevices.

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

Comment thread src/ifce/btrMgr.c
gTimeoutSeconds = gTimeoutSeconds + BTRMGR_BATTERY_DISCOVERY_TIME_INTERVAL;

memset(&discoveredDevices, 0, sizeof(discoveredDevices));
memset(&discoveredDevices, 0, 1000*sizeof(discoveredDevices));
Copy link

Copilot AI Apr 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new memset size 1000*sizeof(discoveredDevices) will write far past the discoveredDevices stack variable (it’s a single BTRMGR_DiscoveredDevicesList_t), causing stack corruption/UB. This should be limited to the actual object size (and ideally use the project’s safer MEMSET_S pattern used elsewhere for this type).

Suggested change
memset(&discoveredDevices, 0, 1000*sizeof(discoveredDevices));
MEMSET_S(&discoveredDevices, sizeof(discoveredDevices), 0, sizeof(discoveredDevices));

Copilot uses AI. Check for mistakes.
Reason for change: Inclusion of coverity for BT
Test Procedure: NA
Risks: Low
Priority: P2

Signed-off-by: ppalan289 <preethi_palanisamy@comcast.com>
Comment thread src/ifce/btrMgr.c
/* Initialze all the database */
MEMSET_S(&gDefaultAdapterContext, sizeof(gDefaultAdapterContext), 0, sizeof(gDefaultAdapterContext));
MEMSET_S(&gListOfAdapters, sizeof(gListOfAdapters), 0, sizeof(gListOfAdapters));
MEMSET_S(&gListOfAdapters, sizeof(gListOfAdapters), 0, 100*sizeof(gListOfAdapters));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Coverity Issue - Wrong sizeof argument

Passing argument "&gListOfAdapters" of type "stBTRCoreListAdapters *" and argument "51300UL" ("100UL * 513UL") to function "memset" is suspicious because "sizeof (stBTRCoreListAdapters) /513/" is expected.

Medium Impact, CWE-131
SIZEOF_MISMATCH

Comment thread src/ifce/btrMgr.c
/* Initialze all the database */
MEMSET_S(&gDefaultAdapterContext, sizeof(gDefaultAdapterContext), 0, sizeof(gDefaultAdapterContext));
MEMSET_S(&gListOfAdapters, sizeof(gListOfAdapters), 0, sizeof(gListOfAdapters));
MEMSET_S(&gListOfAdapters, sizeof(gListOfAdapters), 0, 100*sizeof(gListOfAdapters));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Coverity Issue - Out-of-bounds access

Overrunning struct type stBTRCoreListAdapters of 513 bytes by passing it to a function which accesses it at byte offset 51299 using argument "51300UL".

High Impact, CWE-119
OVERRUN

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants