Fix dangling callback reference and sim ESP-NOW channel reliability#6
Merged
peterus merged 2 commits intonew_channel_managementfrom Mar 13, 2026
Merged
Fix dangling callback reference and sim ESP-NOW channel reliability#6peterus merged 2 commits intonew_channel_managementfrom
peterus merged 2 commits intonew_channel_managementfrom
Conversation
Merged
…an-out, thread-safe channel switch Co-authored-by: peterus <1764325+peterus@users.noreply.github.com>
Contributor
Author
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Copilot
AI
changed the title
[WIP] Add new channel management system for OpenDriveHub
Fix dangling callback reference and sim ESP-NOW channel reliability
Mar 13, 2026
peterus
approved these changes
Mar 13, 2026
There was a problem hiding this comment.
Pull request overview
Dieses PR behebt zwei Regressionen aus dem Channel-Discovery-Feature: einen Use-after-free durch einen langlebigen Discovery-Callback im Receiver sowie unzuverlässiges Paket-Fanout in der nativen ESP-NOW-Simulation durch Wechsel von UDP-Unicast auf Multicast.
Changes:
- Receiver: RAII-Guard, der den
onDiscoveryResponse-Callback beim Verlassen vonrunChannelDiscovery()zuverlässig zurücksetzt (verhindert dangling reference auf stack-lokalenChannelScanner). - Simulation: ESP-NOW-Transport auf UDP-Multicast pro Kanal umgestellt und Channel-Switch so angepasst, dass der Receive-Thread vor Rebind gestoppt/restarted wird.
- Transmitter Shell: cppcheck-Suppression-Tag auf die tatsächlich gemeldete Warnung angepasst.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
firmware/src/transmitter/shell/cmd_io.cpp |
Korrigiert cppcheck-Suppression auf den passenden Warnungsnamen. |
firmware/src/receiver/ReceiverApp.cpp |
Fügt Scope-Guard hinzu, der Discovery-Callback beim Return löscht und so UAF verhindert. |
firmware/sim/src/sim_espnow.cpp |
Wechselt auf Multicast pro Kanal und implementiert Thread-Stop/Restart beim Channel-Wechsel. |
Comment on lines
+256
to
+262
| const bool wasRunning = s_running; | ||
| if (wasRunning) { | ||
| s_running = false; | ||
| if (pthread_join(s_recvThread, nullptr) != 0) { | ||
| perror("[SIM] pthread_join failed during channel switch"); | ||
| } | ||
| } |
Comment on lines
264
to
+273
| s_currentChannel = channel; | ||
| if (s_running) { | ||
|
|
||
| if (wasRunning) { | ||
| rebindSocket(); | ||
| if (s_sock >= 0) { | ||
| s_running = true; | ||
| pthread_create(&s_recvThread, nullptr, recvLoop, nullptr); | ||
| } else { | ||
| fprintf(stderr, "[SIM] rebindSocket failed during channel switch to %u\n", channel); | ||
| } |
Comment on lines
+74
to
+79
| /// Returns the multicast group IPv4 address for the given channel in host | ||
| /// byte order: channel ch → 239.0.0.ch. | ||
| /// Valid input channels are 1, 6, and 11 (see channel::kCandidateChannels). | ||
| static inline uint32_t channelMulticastIp(uint8_t ch) { | ||
| return (239u << 24) | static_cast<uint32_t>(ch); | ||
| } |
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.
Two bugs introduced in the channel discovery PR: a dangling reference UAF in the receiver's discovery callback, and a broken sim ESP-NOW transport that only delivered packets to one process per port.
ReceiverApp: RAII guard for onDiscoveryResponse callback
runChannelDiscovery()registered a callback capturing a reference to a stack-localChannelScanner. The callback outlived the scanner, creating a UAF on the next discovery cycle.Fixed with a local RAII guard that clears the callback at every exit point (C++ destruction order guarantees the guard runs before the scanner):
_radio.onDiscoveryResponse([&scanner](...) { scanner.onDiscoveryResponse(...); }); struct DiscoveryResponseGuard { ReceiverRadioLink &radio; ~DiscoveryResponseGuard() { radio.onDiscoveryResponse(nullptr); } } cbGuard{_radio};sim_espnow: UDP multicast + thread-safe channel switching
239.0.0.<ch>:<port>):SO_REUSEPORTunicast delivers to only one socket under kernel load-balancing, breaking multi-process sim scenarios. Each process now joins the multicast group viaIP_ADD_MEMBERSHIPon loopback;esp_now_send()targets the group address.sim_set_wifi_channel()previously closed/reopened the socket whilerecvLoopwas live. Now stops the thread withpthread_join()(error-checked) before touching the socket, then restarts it.cmd_io: Fix incorrect cppcheck suppression
txCmdModulehad// cppcheck-suppress constParameterCallbackbut the actual warning isconstParameterPointer. Corrected to match the emitted diagnostic.💬 Send tasks to Copilot coding agent from Slack and Teams to turn conversations into code. Copilot posts an update in your thread when it's finished.