RDKEMW-12176: btrCore_PopulateListOfPairedCrash l

[PreethiALPI]

No description provided.

[Copilot]

Pull request overview

This PR adds a termination guard to reduce the chance of use-after-free during teardown by preventing btrCore_PopulateListOfPairedDevices() from running once BTRCore_DeInit() begins.

Changes:

• Introduces a global “terminating” flag and checks it in btrCore_PopulateListOfPairedDevices().
• Adds an early NULL-handle check in btrCore_PopulateListOfPairedDevices().
• Resets/sets the termination flag in BTRCore_Init() / BTRCore_DeInit().

Suppressed comments (2)

src/btrCore.c:1501

• Missing semicolon after the BTRCORELOG_WARN call will cause a compile error in this new allocation-failure branch.

    if ((pstBTPairedDeviceInfo = g_malloc0(sizeof(stBTPairedDeviceInfo))) == NULL) {
        BTRCORELOG_WARN("btrCore: g_malloc0 failed\n")
        return enBTRCoreFailure;

src/btrCore.c:3708

• To minimize the race window this change is intended to address, set the terminating flag before emitting logs so concurrent callers can observe termination as early as possible.

    BTRCORELOG_INFO ("hBTRCore   =   %8p\n", hBTRCore);

    /* Set Terminating variable when deinit is in progress. */
    g_atomic_int_set(&gIsBtrCoreTerminating, 1);

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (4)

src/btrCore.c:3564

• gIsBtrCoreTerminating is a single global flag, but it’s cleared at the start of BTRCore_Init. If BTRCore_Init can be called concurrently with an in-flight BTRCore_DeInit on another thread, this can clear the termination guard while teardown is still freeing the old handle, re-opening the UAF window the guard is meant to close. Consider making the terminating state per-handle (member of stBTRCoreHdl) or protecting Init/DeInit with a process-wide mutex/state machine so Init can’t proceed until teardown completes.

    /* Reset the variable indicating btrCore is initialized, not terminating */
    g_atomic_int_set(&gIsBtrCoreTerminating, 0);

src/btrCore.c:3729

• The new terminating-flag checks reduce some crashes, but they don’t fully prevent UAF: a thread can read gIsBtrCoreTerminating == 0, proceed to dereference hBTRCore, and then BTRCore_DeInit can set the flag and free hBTRCore (see later g_free(hBTRCore)), leaving the in-flight caller using freed memory. To make this robust, you likely need a lifetime/ownership mechanism (e.g., a refcount around public entry points, or a RW lock where DeInit takes a write lock and APIs take a read lock) so memory isn’t freed until all users have exited.

    /* Set Terminating variable when deinit is in progress. */
    g_atomic_int_set(&gIsBtrCoreTerminating, 1);

src/btrCore.c:79

• g_atomic_int_get/set are used with &gIsBtrCoreTerminating, but the variable is declared volatile. In common GLib headers g_atomic_int_* takes a gint*, so passing a volatile gint* can trigger -Wdiscarded-qualifiers (and can become a build break under -Werror). Since GLib atomics already provide the needed memory ordering, keep this as a plain gint (or cast explicitly).

This issue also appears in the following locations of the same file:

• line 3563
• line 3727

/* Prevent UAF during teardown */
static volatile gint gIsBtrCoreTerminating = 0;

src/btrCore.c:4881

• New behavior is introduced to short-circuit public APIs during termination. There are existing unit tests for BTRCore APIs (e.g., unitTest/test_btrCore.c), but there doesn’t appear to be coverage asserting that calls made during/after BTRCore_DeInit safely return without dereferencing a potentially freed handle. Adding a unit test that (1) calls BTRCore_DeInit(h), then (2) calls an API like BTRCore_GetListOfPairedDevices(h, ...) and asserts it returns the expected error without invoking BT iface mocks would help prevent regressions of the original crash.

    /* Prevent UAF during teardown is in progress */
    if(g_atomic_int_get(&gIsBtrCoreTerminating)) {
        BTRCORELOG_WARN("btrCore: Ignoring hBTRCore access during termination\n");
        return enBTRCoreFailure;
    }

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.