From cbbe131ec21c827a3b334ab689854cdeda2b2b43 Mon Sep 17 00:00:00 2001 From: Ixitxachitl Date: Wed, 22 Jul 2026 15:20:18 +0000 Subject: [PATCH 1/3] Fix: queue reply packets again, but carry RxSource through so origin tracking isn't lost. --- src/mesh/MeshModule.cpp | 6 +++++- src/mesh/Router.cpp | 36 +++++++++++++++++++----------------- src/mesh/Router.h | 25 +++++++++++++++++-------- 3 files changed, 41 insertions(+), 26 deletions(-) diff --git a/src/mesh/MeshModule.cpp b/src/mesh/MeshModule.cpp index 5dc7fba4a37..0ec5b97d9b7 100644 --- a/src/mesh/MeshModule.cpp +++ b/src/mesh/MeshModule.cpp @@ -195,7 +195,11 @@ void MeshModule::callModules(meshtastic_MeshPacket &mp, RxSource src) if (isDecoded && mp.decoded.want_response && toUs) { if (currentReply) { printPacket("Send response", currentReply); - service->sendToMesh(currentReply); + // A reply to a phone request loops back to us as its own destination (RX_SRC_LOCAL), + // which the loopback guard above hides from every module - including RoutingModule, + // whose handleReceivedProtobuf() is what forwards packets to the phone. Without + // ccToPhone the reply would be silently dropped instead of reaching the requester. + service->sendToMesh(currentReply, RX_SRC_LOCAL, isToUs(currentReply)); currentReply = NULL; } else if (mp.from != ourNodeNum && !ignoreRequest) { // Note: if the message started with the local node or a module asked to ignore the request, we don't want to send a diff --git a/src/mesh/Router.cpp b/src/mesh/Router.cpp index d10e36fc2e9..8d3d6eb5459 100644 --- a/src/mesh/Router.cpp +++ b/src/mesh/Router.cpp @@ -216,10 +216,10 @@ bool Router::shouldDecrementHopLimit(const meshtastic_MeshPacket *p) */ int32_t Router::runOnce() { - meshtastic_MeshPacket *mp; - while ((mp = fromRadioQueue.dequeuePtr(0)) != NULL) { - // printPacket("handle fromRadioQ", mp); - perhapsHandleReceived(mp); + QueuedFromRadio qp; + while (fromRadioQueue.dequeue(&qp, 0)) { + // printPacket("handle fromRadioQ", qp.packet); + perhapsHandleReceived(qp.packet, qp.src); } // LOG_DEBUG("Sleep forever!"); @@ -230,15 +230,14 @@ int32_t Router::runOnce() * RadioInterface calls this to queue up packets that have been received from the radio. The router is now responsible for * freeing the packet */ -void Router::enqueueReceivedMessage(meshtastic_MeshPacket *p) +void Router::enqueueReceivedMessage(meshtastic_MeshPacket *p, RxSource src) { // Try enqueue until successful - while (!fromRadioQueue.enqueue(p, 0)) { - meshtastic_MeshPacket *old_p; - old_p = fromRadioQueue.dequeuePtr(0); // Dequeue and discard the oldest packet - if (old_p) { - printPacket("fromRadioQ full, drop oldest!", old_p); - packetPool.release(old_p); + while (!fromRadioQueue.enqueue(QueuedFromRadio{p, src}, 0)) { + QueuedFromRadio old_qp; + if (fromRadioQueue.dequeue(&old_qp, 0)) { // Dequeue and discard the oldest packet + printPacket("fromRadioQ full, drop oldest!", old_qp.packet); + packetPool.release(old_qp.packet); } } // Nasty hack because our threading is primitive. interfaces shouldn't need to know about routers FIXME @@ -327,10 +326,13 @@ ErrorCode Router::sendLocal(meshtastic_MeshPacket *p, RxSource src) // No need to deliver externally if the destination is the local node if (isToUs(p)) { printPacket("Enqueued local", p); - // Preserve the trusted origin explicitly. Queueing used to erase src and make a local - // phone/module packet indistinguishable from remote already-decoded ingress. - handleReceived(p, src); - return ERRNO_SHOULD_RELEASE; + // Queue rather than call handleReceived() synchronously: a reply generated from inside + // MeshModule::callModules() (e.g. an admin/module-config response) lands here via + // sendToMesh(), and calling handleReceived() in-line would re-enter callModules() from + // within itself. The queue carries src through so the packet is still replayed with its + // true origin instead of defaulting to RX_SRC_RADIO. + enqueueReceivedMessage(p, src); + return ERRNO_OK; } else if (!iface) { // We must be sending to remote nodes also, fail if no interface found abortSendAndNak(meshtastic_Routing_Error_NO_INTERFACE, p); @@ -1356,7 +1358,7 @@ void Router::handleReceived(meshtastic_MeshPacket *p, RxSource src) packetPool.release(p_encrypted); // Release the encrypted packet (release() handles nullptr) } -void Router::perhapsHandleReceived(meshtastic_MeshPacket *p) +void Router::perhapsHandleReceived(meshtastic_MeshPacket *p, RxSource src) { #if ARCH_PORTDUINO // Even ignored packets get logged in the trace @@ -1425,6 +1427,6 @@ void Router::perhapsHandleReceived(meshtastic_MeshPacket *p) // Note: we avoid calling shouldFilterReceived if we are supposed to ignore certain nodes - because some overrides might // cache/learn of the existence of nodes (i.e. FloodRouter) that they should not - handleReceived(p); + handleReceived(p, src); packetPool.release(p); } diff --git a/src/mesh/Router.h b/src/mesh/Router.h index d1c36878506..c3c4ace3017 100644 --- a/src/mesh/Router.h +++ b/src/mesh/Router.h @@ -5,8 +5,8 @@ #include "MeshTypes.h" #include "Observer.h" #include "PacketHistory.h" -#include "PointerQueue.h" #include "RadioInterface.h" +#include "TypedQueue.h" #include "concurrency/OSThread.h" #include @@ -16,9 +16,17 @@ class Router : protected concurrency::OSThread, protected PacketHistory { private: - /// Packets which have just arrived from the radio, ready to be processed by this service and possibly - /// forwarded to the phone. - PointerQueue fromRadioQueue; + /** A queued fromRadioQueue entry - pairs the packet with the RxSource it arrived/originated with, so a + * locally-addressed packet queued via sendLocal() is replayed with its true origin instead of defaulting + * to RX_SRC_RADIO. */ + struct QueuedFromRadio { + meshtastic_MeshPacket *packet; + RxSource src; + }; + + /// Packets which have just arrived from the radio (or were generated locally and addressed to us), ready to + /// be processed by this service and possibly forwarded to the phone. + TypedQueue fromRadioQueue; protected: std::unique_ptr iface = nullptr; @@ -82,9 +90,11 @@ class Router : protected concurrency::OSThread, protected PacketHistory /** * RadioInterface calls this to queue up packets that have been received from the radio. The router is now responsible for - * freeing the packet + * freeing the packet. Also used by sendLocal() to defer processing of a locally-addressed packet instead of + * re-entering the packet-handling pipeline synchronously; src defaults to RX_SRC_RADIO for the radio/MQTT/UDP + * ingress callers. */ - virtual void enqueueReceivedMessage(meshtastic_MeshPacket *p); + virtual void enqueueReceivedMessage(meshtastic_MeshPacket *p, RxSource src = RX_SRC_RADIO); /** * Send a packet on a suitable interface. This routine will @@ -145,10 +155,9 @@ class Router : protected concurrency::OSThread, protected PacketHistory * Handle any packet that is received by an interface on this node. * Note: some packets may merely being passed through this node and will be forwarded elsewhere. * - * Note: this packet will never be called for messages sent/generated by this node. * Note: this method will free the provided packet. */ - void perhapsHandleReceived(meshtastic_MeshPacket *p); + void perhapsHandleReceived(meshtastic_MeshPacket *p, RxSource src = RX_SRC_RADIO); /** * Called from perhapsHandleReceived() - allows subclass message delivery behavior. From a2c03e56264ddda644e8ec47cf14769b1da18e85 Mon Sep 17 00:00:00 2001 From: Ixitxachitl Date: Thu, 23 Jul 2026 09:15:00 +0000 Subject: [PATCH 2/3] Skip radio-ingress dedup/ignore-list filters for locally-generated reply packets --- src/mesh/Router.cpp | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/mesh/Router.cpp b/src/mesh/Router.cpp index 8d3d6eb5459..6cb323a45e6 100644 --- a/src/mesh/Router.cpp +++ b/src/mesh/Router.cpp @@ -1367,6 +1367,17 @@ void Router::perhapsHandleReceived(meshtastic_MeshPacket *p, RxSource src) LOG_TRACE("%s", MeshPacketSerializer::JsonSerializeEncrypted(p).c_str()); } #endif + // A locally-generated packet (e.g. a module's reply to a phone request, queued here only to avoid + // re-entering callModules() synchronously) was never actually received over the mesh. The ignore-list, + // PacketHistory/dedup, MQTT and pre-hop filters below exist to police untrusted radio ingress, and + // handleReceived() already special-cases RX_SRC_LOCAL (e.g. it only applies the routing-auth cache for + // RX_SRC_RADIO) - so skip straight there instead of risking the reply getting deduped or ignore-listed. + if (src == RX_SRC_LOCAL) { + handleReceived(p, src); + packetPool.release(p); + return; + } + // assert(radioConfig.has_preferences); if (is_in_repeated(config.lora.ignore_incoming, p->from)) { clearRoutingAuthCache(); From 782b5da987d9df66c1e7815d6feaa5f39aa83471 Mon Sep 17 00:00:00 2001 From: Ixitxachitl Date: Fri, 24 Jul 2026 03:19:07 +0000 Subject: [PATCH 3/3] Exempt RX_SRC_USER (not just RX_SRC_LOCAL) from radio-ingress filters in perhapsHandleReceived, and fix tests broken by the queue-defer change --- src/mesh/Router.cpp | 13 +++++++------ test/test_mqtt/MQTT.cpp | 3 ++- test/test_packet_signing/test_main.cpp | 11 +++++++++-- 3 files changed, 18 insertions(+), 9 deletions(-) diff --git a/src/mesh/Router.cpp b/src/mesh/Router.cpp index 6cb323a45e6..2348baf85ee 100644 --- a/src/mesh/Router.cpp +++ b/src/mesh/Router.cpp @@ -1367,12 +1367,13 @@ void Router::perhapsHandleReceived(meshtastic_MeshPacket *p, RxSource src) LOG_TRACE("%s", MeshPacketSerializer::JsonSerializeEncrypted(p).c_str()); } #endif - // A locally-generated packet (e.g. a module's reply to a phone request, queued here only to avoid - // re-entering callModules() synchronously) was never actually received over the mesh. The ignore-list, - // PacketHistory/dedup, MQTT and pre-hop filters below exist to police untrusted radio ingress, and - // handleReceived() already special-cases RX_SRC_LOCAL (e.g. it only applies the routing-auth cache for - // RX_SRC_RADIO) - so skip straight there instead of risking the reply getting deduped or ignore-listed. - if (src == RX_SRC_LOCAL) { + // A non-radio packet (a module's reply to a phone request, or a phone/serial-originated packet + // addressed to us, queued here only to avoid re-entering callModules() synchronously) was never + // actually received over the mesh. The ignore-list, PacketHistory/dedup, MQTT and pre-hop filters + // below exist to police untrusted radio ingress, and handleReceived() already special-cases non- + // RX_SRC_RADIO sources (e.g. it only applies the routing-auth cache for RX_SRC_RADIO) - so skip + // straight there instead of risking a trusted local/user packet getting deduped or ignore-listed. + if (src != RX_SRC_RADIO) { handleReceived(p, src); packetPool.release(p); return; diff --git a/test/test_mqtt/MQTT.cpp b/test/test_mqtt/MQTT.cpp index ab9a64e44a8..d1111f075a3 100644 --- a/test/test_mqtt/MQTT.cpp +++ b/test/test_mqtt/MQTT.cpp @@ -41,8 +41,9 @@ class MockRouter : public Router delete cryptLock; cryptLock = NULL; } - void enqueueReceivedMessage(meshtastic_MeshPacket *p) override + void enqueueReceivedMessage(meshtastic_MeshPacket *p, RxSource src = RX_SRC_RADIO) override { + (void)src; packets_.emplace_back(*p); packetPool.release(p); } diff --git a/test/test_packet_signing/test_main.cpp b/test/test_packet_signing/test_main.cpp index f5eaa44bbdf..ecdeb7b5cd8 100644 --- a/test/test_packet_signing/test_main.cpp +++ b/test/test_packet_signing/test_main.cpp @@ -389,6 +389,10 @@ void setUp(void) channels.initDefaults(); channels.onConfigChanged(); + // sendLocal() defers isToUs() packets to fromRadioQueue rather than handling them in-line; drain + // any left behind by a test that failed an assertion before draining its own (e.g. an aborted + // TEST_ASSERT partway through), so it can't leak into the next test's counters. + pipelineRouter->runOnce(); pipelineRouter->clearPending(); pipelineRouter->rxDupe = 0; pipelineRouter->txRelayCanceled = 0; @@ -1216,9 +1220,12 @@ void test_C8_trusted_local_decoded_delivery_is_not_filtered(void) meshtastic_MeshPacket *local = packetPool.allocCopy(makeDecoded(0, LOCAL_NODE, meshtastic_PortNum_POSITION_APP, SMALL_PAYLOAD)); TEST_ASSERT_NOT_NULL(local); - TEST_ASSERT_EQUAL(ERRNO_SHOULD_RELEASE, pipelineRouter->sendLocal(local, RX_SRC_USER)); + // sendLocal() defers isToUs() packets to fromRadioQueue instead of calling handleReceived() + // in-line (avoids re-entering callModules() from within itself); drain it like the real Router + // thread would on its next tick. The queue - not this call - now owns releasing the packet. + TEST_ASSERT_EQUAL(ERRNO_OK, pipelineRouter->sendLocal(local, RX_SRC_USER)); + pipelineRouter->runOnce(); TEST_ASSERT_EQUAL_MESSAGE(1, pipelineModule->calls, "trusted phone-origin packet must reach local modules"); - packetPool.release(local); } void test_C9_known_channel_malformed_plaintext_is_not_relayed_as_opaque(void)