fix: defensive fixes for packet/routing related memory leakages

src/mesh/MemoryPool.h

@@ -54,7 +54,7 @@ template <class T> class Allocator
     }
 
     /// Variations of the above methods that return std::unique_ptr instead of raw pointers.
-    using UniqueAllocation = std::unique_ptr<T, const std::function<void(T *)> &>;
+    using UniqueAllocation = std::unique_ptr<T, std::function<void(T *)>>;
     /// Return a queable object which has been prefilled with zeros.
     /// std::unique_ptr wrapped variant of allocZeroed().
     UniqueAllocation allocUniqueZeroed() { return UniqueAllocation(allocZeroed(), deleter); }
@@ -103,7 +103,7 @@ template <class T> class MemoryDynamic : public Allocator<T>
         if (p == nullptr)
             return;
 
-        LOG_HEAP("Freeing 0x%x", p);
+        LOG_HEAP("Freeing %p", static_cast<void *>(p));
 
         this->auditAdd(-(int32_t)sizeof(T));
         free(p);
@@ -148,16 +148,30 @@ template <class T, int MaxSize> class MemoryPool : public Allocator<T>
             return;
         }
 
-        // Find the index of this pointer in our pool
-        int index = p - pool;
-        if (index >= 0 && index < MaxSize) {
-            assert(used[index]); // Should be marked as used
-            used[index] = false;
-            this->auditAdd(-(int32_t)sizeof(T));
-            LOG_HEAP("Released static pool item %d at 0x%x", index, p);
-        } else {
-            LOG_WARN("Pointer 0x%x not from our pool", p);
+        uintptr_t pAddr = reinterpret_cast<uintptr_t>(p);
+        uintptr_t poolStart = reinterpret_cast<uintptr_t>(pool);
+        uintptr_t poolEnd = poolStart + sizeof(pool);
+
+        if (pAddr < poolStart || pAddr >= poolEnd) {
+            LOG_WARN("Pointer %p not from our pool", static_cast<void *>(p));
+            return;
+        }
+
+        uintptr_t offset = pAddr - poolStart;
+        if (offset % sizeof(T) != 0) {
+            LOG_WARN("Pointer %p is misaligned inside static pool", static_cast<void *>(p));
+            return;
         }
+
+        size_t index = offset / sizeof(T);
+        if (!used[index]) {
+            LOG_WARN("Double free detected for pool item %d at %p", static_cast<int>(index), static_cast<void *>(p));
+            return;
+        }
+
+        used[index] = false;
+        this->auditAdd(-(int32_t)sizeof(T));
+        LOG_HEAP("Released static pool item %d at %p", static_cast<int>(index), static_cast<void *>(p));
     }
 
   protected:
@@ -169,7 +183,7 @@ template <class T, int MaxSize> class MemoryPool : public Allocator<T>
             if (!used[i]) {
                 used[i] = true;
                 this->auditAdd((int32_t)sizeof(T));
-                LOG_HEAP("Allocated static pool item %d at 0x%x", i, &pool[i]);
+                LOG_HEAP("Allocated static pool item %d at %p", i, static_cast<void *>(&pool[i]));
                 return &pool[i];
             }
         }

src/mesh/MeshService.cpp

@@ -178,14 +178,21 @@ void MeshService::reloadOwner(bool shouldSave)
 NodeNum MeshService::getNodenumFromRequestId(uint32_t request_id)
 {
     NodeNum nodenum = 0;
-    for (int i = 0; i < toPhoneQueue.numUsed(); i++) {
+    int count = toPhoneQueue.numUsed();
+    for (int i = 0; i < count; i++) {
         meshtastic_MeshPacket *p = toPhoneQueue.dequeuePtr(0);
+        if (!p)
+            continue;
         if (p->id == request_id) {
             nodenum = p->to;
             // make sure to continue this to make one full loop
         }
         // put it right back on the queue
-        toPhoneQueue.enqueue(p, 0);
+        if (!toPhoneQueue.enqueue(p, 0)) {
+            LOG_ERROR("Failed to re-enqueue packet in getNodenumFromRequestId; releasing");
+            packetPool.release(p);
+            fromNum++;
+        }
     }
     return nodenum;
 }
@@ -365,7 +372,7 @@ ErrorCode MeshService::sendQueueStatusToPhone(const meshtastic_QueueStatus &qs,
         releaseQueueStatusToPool(copied);
     fromNum++;
 
-    return res ? ERRNO_OK : ERRNO_UNKNOWN;
+    return ERRNO_OK;
 }
 
 void MeshService::sendToMesh(meshtastic_MeshPacket *p, RxSource src, bool ccToPhone)
@@ -410,7 +417,10 @@ bool MeshService::trySendPosition(NodeNum dest, bool wantReplies)
 {
     meshtastic_NodeInfoLite *node = nodeDB->getMeshNode(nodeDB->getNodeNum());
 
-    assert(node);
+    if (!node) {
+        LOG_WARN("trySendPosition: local node info is null");
+        return false;
+    }
 
     if (nodeDB->hasValidPosition(node)) {
 #if HAS_GPS && !MESHTASTIC_EXCLUDE_GPS
@@ -570,7 +580,10 @@ void MeshService::sendClientNotification(meshtastic_ClientNotification *n)
 meshtastic_NodeInfoLite *MeshService::refreshLocalMeshNode()
 {
     meshtastic_NodeInfoLite *node = nodeDB->getMeshNode(nodeDB->getNodeNum());
-    assert(node);
+    if (!node) {
+        LOG_WARN("refreshLocalMeshNode: local node info is null");
+        return nullptr;
+    }
 
     // Update our local node info with our time (even if we don't decide to update anyone else)
     node->last_heard =
@@ -595,6 +608,8 @@ int MeshService::onGPSChanged(const meshtastic::GPSStatus *newStatus)
 {
     // Update our local node info with our position (even if we don't decide to update anyone else)
     const meshtastic_NodeInfoLite *node = refreshLocalMeshNode();
+    if (!node)
+        return 0;
     meshtastic_Position pos = meshtastic_Position_init_default;
 
     if (newStatus->getHasLock()) {

src/mesh/RadioInterface.cpp

@@ -1486,9 +1486,15 @@ void RadioInterface::limitPower(int8_t loraMaxPower)
 
 void RadioInterface::deliverToReceiver(meshtastic_MeshPacket *p)
 {
+    if (!p)
+        return;
+
     if (router) {
         p->transport_mechanism = meshtastic_MeshPacket_TransportMechanism_TRANSPORT_LORA;
         router->enqueueReceivedMessage(p);
+    } else {
+        LOG_WARN("deliverToReceiver: router is null, releasing packet");
+        packetPool.release(p);
     }
 }
 
@@ -1497,10 +1503,29 @@ void RadioInterface::deliverToReceiver(meshtastic_MeshPacket *p)
  */
 size_t RadioInterface::beginSending(meshtastic_MeshPacket *p)
 {
-    assert(!sendingPacket);
+    if (!p) {
+        LOG_ERROR("beginSending called with null packet");
+        return 0;
+    }
 
-    // LOG_DEBUG("Send queued packet on mesh (txGood=%d,rxGood=%d,rxBad=%d)", rf95.txGood(), rf95.rxGood(), rf95.rxBad());
-    assert(p->which_payload_variant == meshtastic_MeshPacket_encrypted_tag); // It should have already been encoded by now
+    if (sendingPacket) {
+        LOG_WARN("beginSending called while transmission active; dropping new packet");
+        packetPool.release(p);
+        return 0;
+    }
+
+    if (p->which_payload_variant != meshtastic_MeshPacket_encrypted_tag) {
+        LOG_ERROR("beginSending called with unencrypted packet variant");
+        packetPool.release(p);
+        return 0;
+    }
+
+    if (static_cast<size_t>(p->encrypted.size) > sizeof(radioBuffer.payload)) {
+        LOG_ERROR("Packet payload size %u exceeds radioBuffer capacity %u", static_cast<unsigned>(p->encrypted.size),
+                  static_cast<unsigned>(sizeof(radioBuffer.payload)));
+        packetPool.release(p);
+        return 0;
+    }
 
     radioBuffer.header.from = p->from;
     radioBuffer.header.to = p->to;
@@ -1516,9 +1541,12 @@ size_t RadioInterface::beginSending(meshtastic_MeshPacket *p)
         p->hop_limit | (p->want_ack ? PACKET_FLAGS_WANT_ACK_MASK : 0) | (p->via_mqtt ? PACKET_FLAGS_VIA_MQTT_MASK : 0);
     radioBuffer.header.flags |= (p->hop_start << PACKET_FLAGS_HOP_START_SHIFT) & PACKET_FLAGS_HOP_START_MASK;
 
-    // if the sender nodenum is zero, that means uninitialized
-    assert(radioBuffer.header.from);
-    assert(p->encrypted.size <= sizeof(radioBuffer.payload));
+    if (!radioBuffer.header.from) {
+        LOG_ERROR("Sender node num is zero");
+        packetPool.release(p);
+        return 0;
+    }
+
     memcpy(radioBuffer.payload, p->encrypted.bytes, p->encrypted.size);
 
     sendingPacket = p;

src/mesh/RadioLibInterface.cpp

@@ -597,12 +597,14 @@ void RadioLibInterface::completeSending()
         printPacket("Completed sending", p);
 #if !MESHTASTIC_EXCLUDE_BEACON
         MeshBeaconModule::clearTargetRadioSettings(p);
-        MeshBeaconModule::reconfigureForBeaconTX(this, nullptr);
 #endif
 
         // We are done sending that packet, release it
         packetPool.release(p);
     }
+#if !MESHTASTIC_EXCLUDE_BEACON
+    MeshBeaconModule::reconfigureForBeaconTX(this, nullptr);
+#endif
 }
 
 void RadioLibInterface::handleReceiveInterrupt()
@@ -782,7 +784,18 @@ bool RadioLibInterface::startSend(meshtastic_MeshPacket *txp)
     } else {
         configHardwareForSend(); // must be after setStandby
 
+#if !MESHTASTIC_EXCLUDE_BEACON
+        MeshBeaconModule::clearTargetRadioSettings(txp);
+#endif
         size_t numbytes = beginSending(txp);
+        if (numbytes == 0) {
+            if (!sendingPacket) {
+                completeSending();
+                powerMon->clearState(meshtastic_PowerMon_State_Lora_TXOn);
+                startReceive();
+            }
+            return false;
+        }
 
         int res = iface->startTransmit((uint8_t *)&radioBuffer, numbytes);
         if (res != RADIOLIB_ERR_NONE) {

src/mesh/Router.cpp

@@ -386,6 +386,10 @@ meshtastic_MeshPacket *Router::allocForSending()
 void Router::sendAckNak(meshtastic_Routing_Error err, NodeNum to, PacketId idFrom, ChannelIndex chIndex, uint8_t hopLimit,
                         bool ackWantsAck)
 {
+    if (!routingModule) {
+        LOG_WARN("sendAckNak: routingModule is null");
+        return;
+    }
     routingModule->sendAckNak(err, to, idFrom, chIndex, hopLimit, ackWantsAck);
 }
 
@@ -550,6 +554,7 @@ ErrorCode Router::send(meshtastic_MeshPacket *p)
 
     if (!(p->which_payload_variant == meshtastic_MeshPacket_encrypted_tag ||
           p->which_payload_variant == meshtastic_MeshPacket_decoded_tag)) {
+        LOG_ERROR("Invalid payload variant in Router::send");
         // Error returns from here own the packet, as the position-precision path below does.
         packetPool.release(p);
         return meshtastic_Routing_Error_BAD_REQUEST;
@@ -579,6 +584,9 @@ ErrorCode Router::send(meshtastic_MeshPacket *p)
         DEBUG_HEAP_BEFORE;
         meshtastic_MeshPacket *p_decoded = packetPool.allocCopy(*p);
         DEBUG_HEAP_AFTER("Router::send", p_decoded);
+        if (!p_decoded) {
+            LOG_WARN("Failed to allocate decoded packet copy in Router::send");
+        }
 
         auto encodeResult = perhapsEncode(p);
         if (encodeResult != meshtastic_Routing_Error_NONE) {
@@ -602,7 +610,11 @@ ErrorCode Router::send(meshtastic_MeshPacket *p)
     }
 #endif
 
-    assert(iface); // This should have been detected already in sendLocal (or we just received a packet from outside)
+    if (!iface) {
+        LOG_ERROR("No interface configured for send!");
+        abortSendAndNak(meshtastic_Routing_Error_NO_INTERFACE, p);
+        return ERRNO_NO_INTERFACES;
+    }
     return iface->send(p);
 }
 
@@ -1464,6 +1476,9 @@ void Router::dispatchReceived(meshtastic_MeshPacket *p, RxSource src)
     DEBUG_HEAP_BEFORE;
     meshtastic_MeshPacket *p_encrypted = packetPool.allocCopy(*p);
     DEBUG_HEAP_AFTER("Router::handleReceived", p_encrypted);
+    if (!p_encrypted) {
+        LOG_WARN("Failed to allocate encrypted packet copy in Router::handleReceived");
+    }
 
     // Consume the decoded/authenticated handoff after preserving the exact encrypted packet and
     // before mutating any packet fields that participate in the exact cache match.

src/modules/MeshBeaconModule.cpp

@@ -286,7 +286,8 @@ void MeshBeaconBroadcastModule::sendBeaconPacket(meshtastic_MeshPacket *p, mesht
     const bool cryptoOverride =
         has_channel && overrideChannel && (overrideChannel->name[0] != '\0' || overrideChannel->psk.size > 0);
     if (!cryptoOverride) {
-        router->send(p);
+        if (router->send(p) == ERRNO_SHOULD_RELEASE)
+            packetPool.release(p);
         return;
     }
 
@@ -300,7 +301,8 @@ void MeshBeaconBroadcastModule::sendBeaconPacket(meshtastic_MeshPacket *p, mesht
     primary.settings = beaconChannelSettings(saved, targetPreset, overrideChannel);
     channels.fixupChannel(channels.getPrimaryIndex());
 
-    router->send(p); // encrypts with the beacon channel's key and stamps its hash
+    if (router->send(p) == ERRNO_SHOULD_RELEASE) // encrypts with the beacon channel's key and stamps its hash
+        packetPool.release(p);
 
     primary.settings = saved;
     channels.fixupChannel(channels.getPrimaryIndex());