From 39cf4af279a9f2c67fd07c606e7d6ca730627aa4 Mon Sep 17 00:00:00 2001 From: nightjoker7 Date: Tue, 21 Apr 2026 12:30:46 -0500 Subject: [PATCH 1/2] MeshService: null-check sender node before dereferencing has_user on hot RX path In handleFromRadio(), the "heard a new node, send our NodeInfo" branch dereferences nodeDB->getMeshNode(mp->from)->has_user without first confirming the lookup succeeded. The preceding updateFrom(*mp) call does NOT guarantee mp->from is now in the DB. It early-returns in three cases: when mp.from equals our own nodenum (local loopback / MQTT echo), when getOrCreateMeshNode fails because the DB is full and no non-favorite non-ignored non-verified entry is available for eviction, and when mp.from == 0 on an encrypted-tag packet. In any of those paths, getMeshNode returns NULL on the next line and ->has_user dereferences it, producing a null-pointer crash. The trailing !nodeDB->isFull() check does not guard this because short-circuit evaluation reaches the dereference first. Add a nullptr check to the short-circuit chain, matching the pattern already used correctly in Router.cpp, RoutingModule.cpp, and ReliableRouter.cpp. If the sender is not in the DB we simply skip the NodeInfo-send branch, which is the correct behavior. --- src/mesh/MeshService.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/mesh/MeshService.cpp b/src/mesh/MeshService.cpp index 952a6d2be37..570d0b97a38 100644 --- a/src/mesh/MeshService.cpp +++ b/src/mesh/MeshService.cpp @@ -94,8 +94,8 @@ int MeshService::handleFromRadio(const meshtastic_MeshPacket *mp) mp->decoded.portnum == meshtastic_PortNum_TELEMETRY_APP && mp->decoded.request_id > 0) { LOG_DEBUG("Received telemetry response. Skip sending our NodeInfo"); // ignore our request for its NodeInfo - } else if (mp->which_payload_variant == meshtastic_MeshPacket_decoded_tag && !nodeDB->getMeshNode(mp->from)->has_user && - nodeInfoModule && !isPreferredRebroadcaster && !nodeDB->isFull()) { + } else if (mp->which_payload_variant == meshtastic_MeshPacket_decoded_tag && nodeDB->getMeshNode(mp->from) != nullptr && + !nodeDB->getMeshNode(mp->from)->has_user && nodeInfoModule && !isPreferredRebroadcaster && !nodeDB->isFull()) { if (airTime->isTxAllowedChannelUtil(true)) { const int8_t hopsUsed = getHopsAway(*mp, config.lora.hop_limit); if (hopsUsed > (int32_t)(config.lora.hop_limit + 2)) { From a6a93d281ebad044828eb8a22222c60e68f01f6a Mon Sep 17 00:00:00 2001 From: nightjoker7 Date: Thu, 23 Apr 2026 15:56:36 -0500 Subject: [PATCH 2/2] Address Copilot review: cache getMeshNode lookup on hot RX path Review feedback from @thebentern (via Copilot) on PR #10229: the null-check and has_user read called getMeshNode(mp->from) twice in the same condition, doubling the linear-scan work on every RX packet. Cache the result in a local fromNode pointer, then do the null-check and has_user read against the cached pointer. Same behaviour; one scan instead of two. Also eliminates the TOCTOU window where a concurrent task could evict the node between calls. --- src/mesh/MeshService.cpp | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/src/mesh/MeshService.cpp b/src/mesh/MeshService.cpp index 570d0b97a38..8579b6422a5 100644 --- a/src/mesh/MeshService.cpp +++ b/src/mesh/MeshService.cpp @@ -94,18 +94,23 @@ int MeshService::handleFromRadio(const meshtastic_MeshPacket *mp) mp->decoded.portnum == meshtastic_PortNum_TELEMETRY_APP && mp->decoded.request_id > 0) { LOG_DEBUG("Received telemetry response. Skip sending our NodeInfo"); // ignore our request for its NodeInfo - } else if (mp->which_payload_variant == meshtastic_MeshPacket_decoded_tag && nodeDB->getMeshNode(mp->from) != nullptr && - !nodeDB->getMeshNode(mp->from)->has_user && nodeInfoModule && !isPreferredRebroadcaster && !nodeDB->isFull()) { - if (airTime->isTxAllowedChannelUtil(true)) { - const int8_t hopsUsed = getHopsAway(*mp, config.lora.hop_limit); - if (hopsUsed > (int32_t)(config.lora.hop_limit + 2)) { - LOG_DEBUG("Skip send NodeInfo: %d hops away is too far away", hopsUsed); + } else if (mp->which_payload_variant == meshtastic_MeshPacket_decoded_tag && nodeInfoModule && !isPreferredRebroadcaster && + !nodeDB->isFull()) { + // Cache the NodeDB lookup so the hot RX path does one linear scan instead of two, and + // so another task can't evict the node between the null-check and the has_user read. + meshtastic_NodeInfoLite *fromNode = nodeDB->getMeshNode(mp->from); + if (fromNode != nullptr && !fromNode->has_user) { + if (airTime->isTxAllowedChannelUtil(true)) { + const int8_t hopsUsed = getHopsAway(*mp, config.lora.hop_limit); + if (hopsUsed > (int32_t)(config.lora.hop_limit + 2)) { + LOG_DEBUG("Skip send NodeInfo: %d hops away is too far away", hopsUsed); + } else { + LOG_INFO("Heard new node on ch. %d, send NodeInfo and ask for response", mp->channel); + nodeInfoModule->sendOurNodeInfo(mp->from, true, mp->channel); + } } else { - LOG_INFO("Heard new node on ch. %d, send NodeInfo and ask for response", mp->channel); - nodeInfoModule->sendOurNodeInfo(mp->from, true, mp->channel); + LOG_DEBUG("Skip sending NodeInfo > 25%% ch. util"); } - } else { - LOG_DEBUG("Skip sending NodeInfo > 25%% ch. util"); } }