Fix uninitialized-pointer crash and double-free in ModuleFrn init failure paths - #808
Open
MarkRose wants to merge 1 commit into
Open
Fix uninitialized-pointer crash and double-free in ModuleFrn init failure paths#808MarkRose wants to merge 1 commit into
MarkRose wants to merge 1 commit into
Conversation
…lure paths - audio_fifo was never set in the ModuleFrn constructor, only assigned later in initialize(). If Module::initialize() fails early (e.g. a version mismatch), ModuleFrn::initialize() returns before any of the audio pipeline members (audio_fifo, audio_splitter, audio_valve) are assigned, leaving them as garbage/null. The destructor then unconditionally calls audio_fifo->unregisterSource(), audio_splitter->removeSink(), and audio_valve->unregisterSink() on those pointers, crashing the module. Fixed by initializing audio_fifo to null in the constructor and guarding all three dereferences in moduleCleanup() with null checks. - When qso->initOk() returns false (any required FRN config variable is missing), initialize() deletes qso but leaves the member pointing at the freed object. The module is then destroyed, and moduleCleanup() uses the dangling qso pointer (audio_splitter->removeSink(qso)) and deletes it a second time. Fixed by nulling qso immediately after the delete on this failure path. Co-Authored-By: Claude Opus 4.8 <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
audio_fifo was never set in the ModuleFrn constructor, only assigned
later in initialize(). If Module::initialize() fails early (e.g. a
version mismatch), ModuleFrn::initialize() returns before any of the
audio pipeline members (audio_fifo, audio_splitter, audio_valve) are
assigned, leaving them as garbage/null. The destructor then
unconditionally calls audio_fifo->unregisterSource(),
audio_splitter->removeSink(), and audio_valve->unregisterSink() on
those pointers, crashing the module. Fixed by initializing audio_fifo
to null in the constructor and guarding all three dereferences in
moduleCleanup() with null checks.
When qso->initOk() returns false (any required FRN config variable is
missing), initialize() deletes qso but leaves the member pointing at
the freed object. The module is then destroyed, and moduleCleanup()
uses the dangling qso pointer (audio_splitter->removeSink(qso)) and
deletes it a second time. Fixed by nulling qso immediately after the
delete on this failure path.
Co-Authored-By: Claude Opus 4.8 [email protected]
This PR also adds a unit test (
ModuleFrnTest.cpp). It is auto-discovered and executed by the CTest suite proposed in #762 once that is merged; without that suite present the test file is inert and does not affect the build.