Skip to content

Issue 79: Detect use-after-free in sa_invoke. Synchronize processorHandleList traversal in SecProcessor_Release#82

Merged
riwoh merged 2 commits intordkcentral:mainfrom
riwoh:issue-79-use-after-free-sa-invoke-v2
Apr 14, 2026
Merged

Issue 79: Detect use-after-free in sa_invoke. Synchronize processorHandleList traversal in SecProcessor_Release#82
riwoh merged 2 commits intordkcentral:mainfrom
riwoh:issue-79-use-after-free-sa-invoke-v2

Conversation

@riwoh
Copy link
Copy Markdown
Contributor

@riwoh riwoh commented Apr 13, 2026

Issue 79: Detect use-after-free in sa_invoke. Synchronize processorHandleList traversal in SecProcessor_Release

riwoh added 2 commits April 13, 2026 16:23
Validate that processorHandle is still in the global processorHandleList
before accessing its fields. This prevents undefined behavior when
sa_invoke is called with a dangling pointer to a freed handle.

Signed-off-by: jhewit200 <joseph_hewitt@cable.comcast.com>
…elease

SecProcessor_Release traversed processorHandleList without holding the global
mutex, then only locked it for the unlink. This created a data race with
concurrent sa_invoke, SecProcessor_GetInstance_Directories, or other
SecProcessor_Release calls that also access the list.

Move the mutex lock to cover both the search and unlink operations, matching
the synchronization in sa_invoke.
Copilot AI review requested due to automatic review settings April 13, 2026 23:30
Copy link
Copy Markdown

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 addresses Issue 79 by adding runtime detection for use-after-free processor handles in sa_invoke, synchronizing access to the global processor handle list during SecProcessor_Release, and adding a regression unit test to exercise the use-after-free scenario.

Changes:

  • Add a new unit test testProcessorUseAfterFree and register it in the processor test suite.
  • Register newly created Sec_ProcessorHandle instances in the global handle list earlier during SecProcessor_GetInstance_Directories initialization.
  • Synchronize processorHandleList traversal in SecProcessor_Release and add list-membership validation to sa_invoke before locking the processor handle mutex.

Reviewed changes

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

File Description
test/main/cpp/sec_api_utest_main.cpp Registers the new processor use-after-free regression test in the test runner.
test/main/cpp/processor.h Exposes the new testProcessorUseAfterFree test declaration.
test/main/cpp/processor.cpp Implements the new regression test that calls an API with a freed processor handle.
src/sec_adapter_processor.c Adds handle-list synchronization, earlier handle registration, and use-after-free detection in sa_invoke.
Comments suppressed due to low confidence (1)

src/sec_adapter_processor.c:385

  • SecProcessor_Release unlinks processorHandle from processorHandleList before releasing SVP/opaque buffers. release_svp_buffer calls sa_invoke(..., SA_SVP_BUFFER_RELEASE, ...), and sa_invoke now rejects any handle not present in the global list, so SVP buffer releases will never reach sa_svp_buffer_release during processor teardown (risking leaks / incomplete cleanup). Consider keeping the handle in processorHandleList until after all cleanup that uses sa_invoke is complete (opaque buffers, etc.), or provide a teardown-safe path for SVP buffer release that bypasses the list-membership check.
    pthread_mutex_lock(&mutex);
    Sec_ProcessorHandle* tempHandle = processorHandleList;
    Sec_ProcessorHandle* parentHandle = NULL;
    while (tempHandle != NULL && tempHandle != processorHandle) {
        parentHandle = tempHandle;
        tempHandle = tempHandle->nextHandle;
    }

    if (tempHandle != processorHandle) {
        pthread_mutex_unlock(&mutex);
        SEC_LOG_ERROR("Attempting to free a handle that has already been freed");
        return SEC_RESULT_INVALID_HANDLE;
    }

    if (parentHandle == NULL)
        processorHandleList = tempHandle->nextHandle;
    else
        parentHandle->nextHandle = tempHandle->nextHandle;

    pthread_mutex_unlock(&mutex);

    while (processorHandle->opaque_buffer_handle != NULL)
        release_svp_buffer(processorHandle, processorHandle->opaque_buffer_handle->opaqueBufferHandle);


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

@riwoh riwoh requested review from JRHewitt-CC and mhabrat April 14, 2026 02:34
@riwoh riwoh merged commit 135f85a into rdkcentral:main Apr 14, 2026
8 checks passed
@riwoh riwoh deleted the issue-79-use-after-free-sa-invoke-v2 branch April 14, 2026 16:08
@github-actions github-actions Bot locked and limited conversation to collaborators Apr 14, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants