fix(router): null-check packetPool/clientNotificationPool allocations (#10948)
On platforms where these pools are heap-backed (MemoryDynamic - used whenever there isn't enough static RAM for a fixed pool, e.g. ARCH_STM32WL or BOARD_HAS_PSRAM), allocCopy()/allocZeroed() return nullptr on allocation failure and already log a warning, but most callers dereferenced the result unconditionally. Under real heap pressure this reliably produced a HardFault - reproduced on STM32WL hardware under sustained mesh traffic, including the RX entry point (RadioLibInterface::handleReceiveInterrupt) where every received packet is allocated. Adds null checks at all call sites that were missing one, mirroring the guard pattern already used correctly elsewhere in the same files (e.g. RadioInterface.cpp's sendErrorNotification). On allocation failure, callers now skip the send/retransmission/notification and log nothing further (the allocator already did) rather than crash. Assisted-by: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: Andrew Yong <me@ndoo.sg>
This commit is contained in:
@@ -111,7 +111,8 @@ int MeshService::handleFromRadio(const meshtastic_MeshPacket *mp)
|
|||||||
}
|
}
|
||||||
|
|
||||||
printPacket("Forwarding to phone", mp);
|
printPacket("Forwarding to phone", mp);
|
||||||
sendToPhone(packetPool.allocCopy(*mp));
|
if (auto *toPhone = packetPool.allocCopy(*mp))
|
||||||
|
sendToPhone(toPhone);
|
||||||
|
|
||||||
return 0;
|
return 0;
|
||||||
}
|
}
|
||||||
@@ -206,7 +207,8 @@ void MeshService::handleToRadio(meshtastic_MeshPacket &p)
|
|||||||
DEBUG_HEAP_BEFORE;
|
DEBUG_HEAP_BEFORE;
|
||||||
auto a = packetPool.allocCopy(p);
|
auto a = packetPool.allocCopy(p);
|
||||||
DEBUG_HEAP_AFTER("MeshService::handleToRadio", a);
|
DEBUG_HEAP_AFTER("MeshService::handleToRadio", a);
|
||||||
sendToMesh(a, RX_SRC_USER);
|
if (a)
|
||||||
|
sendToMesh(a, RX_SRC_USER);
|
||||||
|
|
||||||
bool loopback = false; // if true send any packet the phone sends back itself (for testing)
|
bool loopback = false; // if true send any packet the phone sends back itself (for testing)
|
||||||
if (loopback) {
|
if (loopback) {
|
||||||
@@ -226,6 +228,8 @@ bool MeshService::cancelSending(PacketId id)
|
|||||||
ErrorCode MeshService::sendQueueStatusToPhone(const meshtastic_QueueStatus &qs, ErrorCode res, uint32_t mesh_packet_id)
|
ErrorCode MeshService::sendQueueStatusToPhone(const meshtastic_QueueStatus &qs, ErrorCode res, uint32_t mesh_packet_id)
|
||||||
{
|
{
|
||||||
meshtastic_QueueStatus *copied = queueStatusPool.allocCopy(qs);
|
meshtastic_QueueStatus *copied = queueStatusPool.allocCopy(qs);
|
||||||
|
if (!copied)
|
||||||
|
return ERRNO_UNKNOWN;
|
||||||
|
|
||||||
copied->res = res;
|
copied->res = res;
|
||||||
copied->mesh_packet_id = mesh_packet_id;
|
copied->mesh_packet_id = mesh_packet_id;
|
||||||
@@ -266,7 +270,8 @@ void MeshService::sendToMesh(meshtastic_MeshPacket *p, RxSource src, bool ccToPh
|
|||||||
auto a = packetPool.allocCopy(*p);
|
auto a = packetPool.allocCopy(*p);
|
||||||
DEBUG_HEAP_AFTER("MeshService::sendToMesh", a);
|
DEBUG_HEAP_AFTER("MeshService::sendToMesh", a);
|
||||||
|
|
||||||
sendToPhone(a);
|
if (a)
|
||||||
|
sendToPhone(a);
|
||||||
}
|
}
|
||||||
|
|
||||||
// Router may ask us to release the packet if it wasn't sent
|
// Router may ask us to release the packet if it wasn't sent
|
||||||
|
|||||||
@@ -31,8 +31,10 @@ ErrorCode NextHopRouter::send(meshtastic_MeshPacket *p)
|
|||||||
|
|
||||||
// If it's from us, ReliableRouter already handles retransmissions if want_ack is set. If a next hop is set and hop limit is
|
// If it's from us, ReliableRouter already handles retransmissions if want_ack is set. If a next hop is set and hop limit is
|
||||||
// not 0 or want_ack is set, start retransmissions
|
// not 0 or want_ack is set, start retransmissions
|
||||||
if ((!isFromUs(p) || !p->want_ack) && p->next_hop != NO_NEXT_HOP_PREFERENCE && (p->hop_limit > 0 || p->want_ack))
|
if ((!isFromUs(p) || !p->want_ack) && p->next_hop != NO_NEXT_HOP_PREFERENCE && (p->hop_limit > 0 || p->want_ack)) {
|
||||||
startRetransmission(packetPool.allocCopy(*p)); // start retransmission for relayed packet
|
if (auto *copy = packetPool.allocCopy(*p))
|
||||||
|
startRetransmission(copy); // start retransmission for relayed packet
|
||||||
|
}
|
||||||
|
|
||||||
return Router::send(p);
|
return Router::send(p);
|
||||||
}
|
}
|
||||||
@@ -168,6 +170,8 @@ bool NextHopRouter::perhapsRebroadcast(const meshtastic_MeshPacket *p)
|
|||||||
// ambiguous next_hop byte is ever learned (sniffReceived) or originated (getNextHop).
|
// ambiguous next_hop byte is ever learned (sniffReceived) or originated (getNextHop).
|
||||||
if (p->next_hop == NO_NEXT_HOP_PREFERENCE || p->next_hop == nodeDB->getLastByteOfNodeNum(getNodeNum())) {
|
if (p->next_hop == NO_NEXT_HOP_PREFERENCE || p->next_hop == nodeDB->getLastByteOfNodeNum(getNodeNum())) {
|
||||||
meshtastic_MeshPacket *tosend = packetPool.allocCopy(*p); // keep a copy because we will be sending it
|
meshtastic_MeshPacket *tosend = packetPool.allocCopy(*p); // keep a copy because we will be sending it
|
||||||
|
if (!tosend)
|
||||||
|
return true;
|
||||||
LOG_INFO("Rebroadcast received message coming from %x", p->relay_node);
|
LOG_INFO("Rebroadcast received message coming from %x", p->relay_node);
|
||||||
|
|
||||||
// If exhausting hops, force hop_limit = 0 regardless of other logic
|
// If exhausting hops, force hop_limit = 0 regardless of other logic
|
||||||
@@ -397,7 +401,8 @@ int32_t NextHopRouter::doRetransmissions()
|
|||||||
trafficManagementModule->clearNextHop(p.packet->to);
|
trafficManagementModule->clearNextHop(p.packet->to);
|
||||||
}
|
}
|
||||||
#endif
|
#endif
|
||||||
FloodingRouter::send(packetPool.allocCopy(*p.packet));
|
if (auto *copy = packetPool.allocCopy(*p.packet))
|
||||||
|
FloodingRouter::send(copy);
|
||||||
} else {
|
} else {
|
||||||
#if NEXTHOP_EARLY_FLOOD_ON_UNVERIFIED
|
#if NEXTHOP_EARLY_FLOOD_ON_UNVERIFIED
|
||||||
// M4 (gated): if the route isn't proven healthy, don't spend a second directed
|
// M4 (gated): if the route isn't proven healthy, don't spend a second directed
|
||||||
@@ -411,18 +416,22 @@ int32_t NextHopRouter::doRetransmissions()
|
|||||||
meshtastic_NodeInfoLite *sentTo = nodeDB->getMeshNode(p.packet->to);
|
meshtastic_NodeInfoLite *sentTo = nodeDB->getMeshNode(p.packet->to);
|
||||||
if (sentTo)
|
if (sentTo)
|
||||||
sentTo->next_hop = NO_NEXT_HOP_PREFERENCE;
|
sentTo->next_hop = NO_NEXT_HOP_PREFERENCE;
|
||||||
FloodingRouter::send(packetPool.allocCopy(*p.packet));
|
if (auto *copy = packetPool.allocCopy(*p.packet))
|
||||||
|
FloodingRouter::send(copy);
|
||||||
} else {
|
} else {
|
||||||
NextHopRouter::send(packetPool.allocCopy(*p.packet));
|
if (auto *copy = packetPool.allocCopy(*p.packet))
|
||||||
|
NextHopRouter::send(copy);
|
||||||
}
|
}
|
||||||
#else
|
#else
|
||||||
NextHopRouter::send(packetPool.allocCopy(*p.packet));
|
if (auto *copy = packetPool.allocCopy(*p.packet))
|
||||||
|
NextHopRouter::send(copy);
|
||||||
#endif
|
#endif
|
||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
// Note: we call the superclass version because we don't want to have our version of send() add a new
|
// Note: we call the superclass version because we don't want to have our version of send() add a new
|
||||||
// retransmission record
|
// retransmission record
|
||||||
FloodingRouter::send(packetPool.allocCopy(*p.packet));
|
if (auto *copy = packetPool.allocCopy(*p.packet))
|
||||||
|
FloodingRouter::send(copy);
|
||||||
}
|
}
|
||||||
|
|
||||||
// Queue again
|
// Queue again
|
||||||
|
|||||||
+6
-4
@@ -3348,10 +3348,12 @@ bool NodeDB::updateUser(uint32_t nodeId, meshtastic_User &p, uint8_t channelInde
|
|||||||
"to regenerate your public keys.";
|
"to regenerate your public keys.";
|
||||||
LOG_WARN(warning, safeName);
|
LOG_WARN(warning, safeName);
|
||||||
meshtastic_ClientNotification *cn = clientNotificationPool.allocZeroed();
|
meshtastic_ClientNotification *cn = clientNotificationPool.allocZeroed();
|
||||||
cn->level = meshtastic_LogRecord_Level_WARNING;
|
if (cn) {
|
||||||
cn->time = getValidTime(RTCQualityFromNet);
|
cn->level = meshtastic_LogRecord_Level_WARNING;
|
||||||
snprintf(cn->message, sizeof(cn->message), warning, safeName);
|
cn->time = getValidTime(RTCQualityFromNet);
|
||||||
service->sendClientNotification(cn);
|
snprintf(cn->message, sizeof(cn->message), warning, safeName);
|
||||||
|
service->sendClientNotification(cn);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1640,6 +1640,8 @@ bool PhoneAPI::available()
|
|||||||
void PhoneAPI::sendNotification(meshtastic_LogRecord_Level level, uint32_t replyId, const char *message)
|
void PhoneAPI::sendNotification(meshtastic_LogRecord_Level level, uint32_t replyId, const char *message)
|
||||||
{
|
{
|
||||||
meshtastic_ClientNotification *cn = clientNotificationPool.allocZeroed();
|
meshtastic_ClientNotification *cn = clientNotificationPool.allocZeroed();
|
||||||
|
if (!cn)
|
||||||
|
return;
|
||||||
cn->has_reply_id = true;
|
cn->has_reply_id = true;
|
||||||
cn->reply_id = replyId;
|
cn->reply_id = replyId;
|
||||||
cn->level = meshtastic_LogRecord_Level_WARNING;
|
cn->level = meshtastic_LogRecord_Level_WARNING;
|
||||||
|
|||||||
@@ -663,6 +663,10 @@ void RadioLibInterface::handleReceiveInterrupt()
|
|||||||
// This allows the router and other apps on our node to sniff packets (usually routing) between other
|
// This allows the router and other apps on our node to sniff packets (usually routing) between other
|
||||||
// nodes.
|
// nodes.
|
||||||
meshtastic_MeshPacket *mp = packetPool.allocZeroed();
|
meshtastic_MeshPacket *mp = packetPool.allocZeroed();
|
||||||
|
if (!mp) {
|
||||||
|
airTime->logAirtime(RX_LOG, rxMsec);
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
// Keep the assigned fields in sync with src/mqtt/MQTT.cpp:onReceiveProto
|
// Keep the assigned fields in sync with src/mqtt/MQTT.cpp:onReceiveProto
|
||||||
mp->from = radioBuffer.header.from;
|
mp->from = radioBuffer.header.from;
|
||||||
|
|||||||
@@ -21,7 +21,8 @@ ErrorCode ReliableRouter::send(meshtastic_MeshPacket *p)
|
|||||||
auto copy = packetPool.allocCopy(*p);
|
auto copy = packetPool.allocCopy(*p);
|
||||||
DEBUG_HEAP_AFTER("ReliableRouter::send", copy);
|
DEBUG_HEAP_AFTER("ReliableRouter::send", copy);
|
||||||
|
|
||||||
startRetransmission(copy, NUM_RELIABLE_RETX);
|
if (copy)
|
||||||
|
startRetransmission(copy, NUM_RELIABLE_RETX);
|
||||||
}
|
}
|
||||||
|
|
||||||
/* If we have pending retransmissions, add the airtime of this packet to it, because during that time we cannot receive an
|
/* If we have pending retransmissions, add the airtime of this packet to it, because during that time we cannot receive an
|
||||||
|
|||||||
+10
-7
@@ -312,12 +312,15 @@ ErrorCode Router::send(meshtastic_MeshPacket *p)
|
|||||||
LOG_WARN("Duty cycle limit exceeded. Aborting send for now, you can send again in %d mins", silentMinutes);
|
LOG_WARN("Duty cycle limit exceeded. Aborting send for now, you can send again in %d mins", silentMinutes);
|
||||||
|
|
||||||
meshtastic_ClientNotification *cn = clientNotificationPool.allocZeroed();
|
meshtastic_ClientNotification *cn = clientNotificationPool.allocZeroed();
|
||||||
cn->has_reply_id = true;
|
if (cn) {
|
||||||
cn->reply_id = p->id;
|
cn->has_reply_id = true;
|
||||||
cn->level = meshtastic_LogRecord_Level_WARNING;
|
cn->reply_id = p->id;
|
||||||
cn->time = getValidTime(RTCQualityFromNet);
|
cn->level = meshtastic_LogRecord_Level_WARNING;
|
||||||
snprintf(cn->message, sizeof(cn->message), "Duty cycle limit exceeded. You can send again in %d mins", silentMinutes);
|
cn->time = getValidTime(RTCQualityFromNet);
|
||||||
service->sendClientNotification(cn);
|
snprintf(cn->message, sizeof(cn->message), "Duty cycle limit exceeded. You can send again in %d mins",
|
||||||
|
silentMinutes);
|
||||||
|
service->sendClientNotification(cn);
|
||||||
|
}
|
||||||
|
|
||||||
meshtastic_Routing_Error err = meshtastic_Routing_Error_DUTY_CYCLE_LIMIT;
|
meshtastic_Routing_Error err = meshtastic_Routing_Error_DUTY_CYCLE_LIMIT;
|
||||||
if (isFromUs(p)) { // only send NAK to API, not to the mesh
|
if (isFromUs(p)) { // only send NAK to API, not to the mesh
|
||||||
@@ -405,7 +408,7 @@ ErrorCode Router::send(meshtastic_MeshPacket *p)
|
|||||||
}
|
}
|
||||||
#if !MESHTASTIC_EXCLUDE_MQTT
|
#if !MESHTASTIC_EXCLUDE_MQTT
|
||||||
// Only publish to MQTT if we're the original transmitter of the packet
|
// Only publish to MQTT if we're the original transmitter of the packet
|
||||||
if (moduleConfig.mqtt.enabled && isFromUs(p) && mqtt) {
|
if (moduleConfig.mqtt.enabled && isFromUs(p) && mqtt && p_decoded) {
|
||||||
mqtt->onSend(*p, *p_decoded, chIndex);
|
mqtt->onSend(*p, *p_decoded, chIndex);
|
||||||
}
|
}
|
||||||
#endif
|
#endif
|
||||||
|
|||||||
Reference in New Issue
Block a user