Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions src/svxlink/modules/frn/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,9 @@ install(FILES ${MODNAME}.tcl DESTINATION ${SVX_SHARE_INSTALL_DIR}/events.d)
install_if_not_exists(Module${MODNAME}.conf
${SVX_SYSCONF_INSTALL_DIR}/svxlink.d
)

# Unit tests: every *Test.cpp here is auto-registered once the CTest
# suite (SvxLinkAddTests) is present; harmless without it.
if(COMMAND svxlink_add_tests)
svxlink_add_tests()
endif()
17 changes: 14 additions & 3 deletions src/svxlink/modules/frn/ModuleFrn.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ ModuleFrn::ModuleFrn(void *dl_handle, Logic *logic, const string& cfg_name)
, audio_valve(0)
, audio_splitter(0)
, audio_selector(0)
, audio_fifo(0)
{
cout << "\tModule Frn v" MODULE_FRN_VERSION " starting...\n";

Expand Down Expand Up @@ -229,6 +230,7 @@ bool ModuleFrn::initialize(void)
if (!qso->initOk())
{
delete qso;
qso = 0;
cerr << "*** ERROR: Creation of Qso object failed\n";
return false;
}
Expand All @@ -241,10 +243,19 @@ bool ModuleFrn::initialize(void)
void ModuleFrn::moduleCleanup()
{
AudioSource::clearHandler();
audio_fifo->unregisterSource();
if (audio_fifo != 0)
{
audio_fifo->unregisterSource();
}

audio_splitter->removeSink(qso);
audio_valve->unregisterSink();
if (audio_splitter != 0)
{
audio_splitter->removeSink(qso);
}
if (audio_valve != 0)
{
audio_valve->unregisterSink();
}
AudioSink::clearHandler();

delete qso;
Expand Down
95 changes: 95 additions & 0 deletions src/svxlink/modules/frn/ModuleFrnTest.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
/**
@file ModuleFrnTest.cpp
@brief Unit test for ModuleFrn init-failure pointer lifecycle
@author Mark Rose
@date 2026-07-11

ModuleFrn::initialize() only assigns audio_fifo, audio_splitter and
audio_valve after the base Module::initialize() call succeeds. If
initialization fails (or is never called) before those members are
assigned, the ModuleFrn destructor still unconditionally calls
moduleCleanup(), which used to dereference audio_fifo unconditionally.
Because audio_fifo was not part of the constructor initializer list, it
held whatever garbage bits were already present in the object's storage,
so moduleCleanup() dereferenced an uninitialized/garbage pointer and
crashed.

This test places a ModuleFrn instance into memory that has been poisoned
with a non-canonical bit pattern *before* construction -- exactly the
kind of "garbage" an uninitialized pointer member would contain -- then
destroys the object without ever calling initialize(). With the fix,
the constructor explicitly nulls audio_fifo and moduleCleanup() guards
every audio pipeline pointer with a null check, so destruction completes
cleanly. Without the fix, moduleCleanup() dereferences the poisoned
audio_fifo pointer and the process crashes (a non-canonical x86_64
address reliably faults on first access), which CTest observes as a
failing (non-zero/signal-terminated) test.

\verbatim
SvxLink - A Multi Purpose Voice Services System for Ham Radio Use
Copyright (C) 2026 Tobias Blomberg / SM0SVX

This program is free software; you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
the Free Software Foundation; either version 2 of the License, or
(at your option) any later version.
\endverbatim
*/

#include <cstring>
#include <iostream>
#include <string>

#include "ModuleFrn.h"

using namespace std;

namespace {

int failures = 0;

void check(bool cond, const string& msg)
{
cout << (cond ? " ok " : " FAIL ") << msg << endl;
if (!cond)
{
++failures;
}
}

} /* anonymous namespace */


int main(void)
{
// Poison the storage for a ModuleFrn instance with a non-canonical
// x86_64 bit pattern before construction. This stands in for the
// uninitialized garbage that a pointer member would contain if it is
// never assigned by the constructor -- exactly the pre-fix bug for
// audio_fifo. Any real dereference of this pattern as a pointer
// reliably faults.
alignas(ModuleFrn) unsigned char storage[sizeof(ModuleFrn)];
memset(storage, 0xAB, sizeof(storage));

// Construct in place. dl_handle and logic are both unused by the
// constructor and by moduleCleanup(), so passing null is safe as
// long as initialize() (which does dereference logic()) is never
// called -- exactly the failure mode this test targets: initialize()
// fails (or, as here, is never reached) before the audio pipeline
// members are assigned.
ModuleFrn *mod = new (storage) ModuleFrn(0, 0, "frn");

// Destroy without ever calling initialize(). This exercises
// moduleCleanup() with audio_fifo (and friends) left at whatever the
// constructor set them to. Pre-fix, audio_fifo is left as the
// poisoned garbage above and moduleCleanup() crashes dereferencing
// it. Post-fix, audio_fifo is explicitly nulled and every dereference
// in moduleCleanup() is null-guarded, so this returns cleanly.
mod->~ModuleFrn();

check(true, "destructor completed without dereferencing an "
"uninitialized audio_fifo pointer");

cout << (failures == 0 ? "ALL TESTS PASSED" : "TESTS FAILED") << endl;
return failures != 0;
} /* main */
19 changes: 19 additions & 0 deletions src/svxlink/modules/frn/ModuleFrnTest.deps.cmake
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# Extra sources/libs needed to link ModuleFrnTest.
#
# ModuleFrn.cpp requires QsoFrn.cpp/Utils.cpp (its module siblings) and
# Module.cpp (the svxlink core base class it derives from). Module.cpp
# in turn makes a few direct (non-virtual) calls into Logic:: and touches
# the LinkManager singleton pointer; ModuleFrnTestLinkStubs.cpp provides
# trivial link-only stand-ins for those so the real Logic/LinkManager
# subsystem (and the Async event loop / application wiring it needs)
# does not have to be pulled in for this standalone test.
set(ModuleFrnTest_EXTRA_SRCS
ModuleFrn.cpp
QsoFrn.cpp
Utils.cpp
ModuleFrnTestLinkStubs.cpp
${CMAKE_CURRENT_SOURCE_DIR}/../../svxlink/Module.cpp
)
set(ModuleFrnTest_EXTRA_LIBS
asynccpp asynccore asyncaudio svxmisc ${GSM_LIBRARY}
)
59 changes: 59 additions & 0 deletions src/svxlink/modules/frn/ModuleFrnTestLinkStubs.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
/**
@file ModuleFrnTestLinkStubs.cpp
@brief Link-only stand-ins for symbols ModuleFrnTest never executes
@author Mark Rose
@date 2026-07-11

ModuleFrnTest.cpp constructs and destroys a ModuleFrn without ever
calling initialize(), so it never exercises any Logic:: or LinkManager
functionality. However, Module.cpp (compiled in as part of the module
under test, since ModuleFrn derives from Module) contains a handful of
member functions -- reachable only from code paths this test never
runs -- that make *direct* (non-virtual) calls into Logic:: methods and
touch the LinkManager singleton pointer. Because those functions are
part of the same translation unit that is linked into this test binary,
the linker requires those symbols to be resolvable even though they are
never invoked at runtime by this test.

Pulling in the real Logic.cpp/LinkManager.cpp to satisfy this would drag
in the full logic-core/event-handling/link-management subsystem, which
needs a live Async event loop and application wiring -- exactly what this
small unit test intentionally avoids. These stubs instead provide trivial
definitions for just the non-virtual symbols Module.cpp references, so
the parts of ModuleFrn actually under test (the constructor/destructor
pointer lifecycle) can be linked and run standalone. None of these bodies
are ever executed by ModuleFrnTest.

\verbatim
SvxLink - A Multi Purpose Voice Services System for Ham Radio Use
Copyright (C) 2026 Tobias Blomberg / SM0SVX

This program is free software; you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
the Free Software Foundation; either version 2 of the License, or
(at your option) any later version.
\endverbatim
*/

#include "Logic.h"
#include "LinkManager.h"

using namespace std;


// Non-virtual Logic:: member functions that Module.cpp calls directly
// (not through a vtable). Never invoked by this test -- ModuleFrn is
// destroyed before initialize() is ever called -- but the symbols must
// exist for the link to succeed.
void Logic::setEventVariable(const string&, const string&) {}
void Logic::sendDtmf(const string&) {}
Module *Logic::findModule(int) { return 0; }
bool Logic::isWritingMessage(void) { return false; }

// Storage for the LinkManager singleton pointer. Module::activate()/
// deactivate() check LinkManager::hasInstance() (inline, reads this
// pointer) before touching the singleton; leaving it null means that
// guard is always false and the singleton is never dereferenced.
LinkManager *LinkManager::_instance = 0;

void LinkManager::setLogicMute(const LogicBase*, bool) {}
Loading