Repository navigation
RadioMedium: fix two null dereferences in the communication cache - #1298
Merged
torokati44 merged 2 commits intoOct 13, 2026
Merged
Conversation
There was a problem hiding this comment.
🔍 Devin Review: 1 flag
Not posted on this PR by your GitHub settings — view it in Devin Review. (Configure)
levy
reviewed
Oct 7, 2026
| communicationCache->setCachedListening(radio, transmission, listening); | ||
| }); | ||
| // like addTransmission(), cache no arrival and no listening at a radio without a receiver | ||
| if (radio->getReceiver() != nullptr) { |
Contributor
There was a problem hiding this comment.
A receiver cannot be null.
/**
* Returns the receiver part of this radio. This function never returns nullptr.
*/
virtual const IReceiver *getReceiver() const = 0;
Member
Author
There was a problem hiding this comment.
NoiseSource breaks this contract, and it is already accomodated for: a2a3925f96
Should the "non-radio transmitter/receiver" change (mentioned yesterday) be done first then?
With listeningFilter = true, a radio that changes its listening, for example by switching channel, while one of its own transmissions is still in the communication cache crashes the simulation. RadioMedium::receiveSignal() refreshes the cached listening of the radio for every cached transmission, but addTransmission() caches no arrival at the transmitter itself, so the handler dereferences a null arrival. The handler now skips the radio's own transmissions, as addTransmission(), sendToRadio() and pickUpSignals() already do. The window is as long as a transmission stays cached, which is until the medium purges it after its interference end time. Reproduce: run the new module test RadioMedium_ListeningFilter_1. Host1 pings host2 at 0.5s, and both radios switch channel at 0.502s. Before this commit, the run segfaults in RadioMedium::receiveSignal(). No fingerprint moves: the only configuration with listeningFilter = true, examples/wireless/filtering, never reaches the skipped case, and its four rows pass unchanged. Change: src.physicallayer.RadioMedium | behavior.change.fix | test
Adding a radio without a receiver, such as a NoiseSource, to the radio medium while transmissions are in its communication cache crashes the simulation. RadioMedium::addRadio() creates a listening at the new radio for every cached transmission, and dereferences the radio's null receiver to do so. addRadio() now caches no arrival, interval or listening at such a radio, as addTransmission() already does for a transmission that starts after the radio was added. At network initialization the cache is empty, so the crash needs a radio that is created at runtime, for example by the ScenarioManager create-module command, within a transmission's purge window. Reproduce: run the new module test RadioMedium_AddRadioWithoutReceiver_1. Host1 pings host2 at 0.5s, and a NoiseSource is created at 0.502s. Before this commit, the run segfaults in RadioMedium::addRadio(). No fingerprint moves: the change only affects radios added at runtime without a receiver, which crashed before. The rows of the noise configuration of showcases/wireless/analogmodel pass unchanged. Change: src.physicallayer.RadioMedium | behavior.change.fix | test
torokati44
force-pushed
the
listeningfilter-crash-fix
branch
from
October 13, 2026 10:58
744b545 to
d7ee856
Compare
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.
RadioMediumcan crash with a segfault in two situations, because two code paths read per-radio cache entries that the medium never creates.addTransmission()caches no arrival and no listening at the transmitting radio itself, and none at a radio without a receiver, such asNoiseSource. Two other paths assume those entries exist:listeningFilter = true, when a radio changes its listening (for example, it switches channel), the handler refreshes the radio's listening for every cached transmission, including the radio's own. Its arrival is null.addRadio()creates a listening at the new radio for every cached transmission, through the radio's receiver. For aNoiseSourcethe receiver is null.Both crashes need a transmission still in the cache, that is, before the medium purges it after its interference end time. The first needs a listening change within that window after the radio's own transmission. The second needs a receiver-less radio created at runtime (for example by the
ScenarioManagercreate-modulecommand), because the cache is empty at network initialization. Both reproduce on master. The transmission purge mode proposed in #1281 widens the first window: with it, the crash happens even 0.5 s after the transmission, so this pull request should land first.Each fix skips the missing case, as
addTransmission(),sendToRadio()andpickUpSignals()already do. Neither changes behavior anywhere else: each skipped case crashed before. Neither touches an interface, a parameter or packet content.Commits
physicallayer: fix: skip own transmissions on a listening change: fixes the listening-change handler, with the module testRadioMedium_ListeningFilter_1.physicallayer: fix: add a radio without a receiver during transmissions: fixesaddRadio(), with the module testRadioMedium_AddRadioWithoutReceiver_1.Each commit is a fix with its own regression test. Neither depends on the other.
Evidence
Each test segfaults on its parent commit and passes on its own commit:
At the head, the related module tests pass (87 PASS):
No fingerprint moves. The only configuration with
listeningFilter = true, and the one with aNoiseSource, pass unchanged (7 rows, OK):Both commits build in debug and release mode.
check-commits.shandcheck-classification.shpass on the series.