kevin/meshtastic_firmware
1
0
Fork 0
Code Issues Pull Requests Actions 7 Packages Projects Releases Wiki Activity

fix(modules): null-check packetPool/clientNotificationPool/mqttClientProxyMessagePool allocations (#10951)

Browse Source 
* fix(modules): null-check packetPool/clientNotificationPool/mqttClientProxyMessagePool allocations

Follow-up to fix/meshpacket-alloc-null-checks (PR meshtastic/firmware#10948),
which fixed the core routing path (Router/NextHopRouter/ReliableRouter/
MeshService/RadioLibInterface) after reproducing a HardFault on STM32WL
hardware under real mesh traffic. This covers the same allocCopy()/
allocZeroed() unchecked-return pattern in lower-frequency paths that were
out of scope for that PR: SimRadio.cpp (portduino sim RX/TX), NodeInfoModule,
the Telemetry modules' power-saving-sleep notifications, MQTT (map report,
client-proxy messages, config-validation notifications), PositionModule,
SerialModule, and KeyVerificationModule.

Sites where the allocation result was already read back through an
existing null check downstream (e.g. Telemetry's lastMeasurementPacket,
SimRadio's receivingPacket, AdminModule::sendWarning) were left as-is -
verified safe, not touched.

Router::allocForSending() (and everything that funnels through it, e.g.
allocDataProtobuf()) remains unguarded and out of scope here too - it's
called from 30+ sites across the codebase and needs its own audit.

Assisted-by: Claude Sonnet 5 <noreply@anthropic.com>
Signed-off-by: Andrew Yong <me@ndoo.sg>

* fix(KeyVerificationModule): bound cn->message writes with snprintf

CodeRabbit flagged unbounded sprintf() writes into the fixed-size
ClientNotification.message buffer as a static-analysis nitpick while
reviewing this PR's allocation null-checks. Align these four sites with
the snprintf(dest, sizeof(dest), ...) pattern already used for the same
field elsewhere in this PR (SerialModule.cpp, MQTT.cpp).

Assisted-by: Claude Sonnet 5 <noreply@anthropic.com>
Signed-off-by: Andrew Yong <me@ndoo.sg>

---------

Signed-off-by: Andrew Yong <me@ndoo.sg>
Co-authored-by: Ben Meadors <benmmeadors@gmail.com>
This commit is contained in:
Andrew Yong
2026-07-09 10:34:58 -05:00
committed by GitHub
co-authored by GitHub Ben Meadors
parent 515fe8b94f
commit 6768bd08df
9 changed files with 112 additions and 79 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/mqtt/MQTT.cpp
+20 -10
View File
@@ -392,6 +392,8 @@ bool MQTT::publish(const char *topic, const char *payload, bool retained)
{
if (moduleConfig.mqtt.proxy_to_client_enabled) {
meshtastic_MqttClientProxyMessage *msg = mqttClientProxyMessagePool.allocZeroed();
if (!msg)
return false;
msg->which_payload_variant = meshtastic_MqttClientProxyMessage_text_tag;
strncpy(msg->topic, topic, sizeof(msg->topic));
msg->topic[sizeof(msg->topic) - 1] = '\0';
@@ -413,6 +415,8 @@ bool MQTT::publish(const char *topic, const uint8_t *payload, size_t length, boo
{
if (moduleConfig.mqtt.proxy_to_client_enabled) {
meshtastic_MqttClientProxyMessage *msg = mqttClientProxyMessagePool.allocZeroed();
if (!msg)
return false;
msg->which_payload_variant = meshtastic_MqttClientProxyMessage_data_tag;
strncpy(msg->topic, topic, sizeof(msg->topic));
msg->topic[sizeof(msg->topic) - 1] = '\0'; // Ensure null termination
@@ -595,11 +599,13 @@ bool MQTT::isValidConfig(const meshtastic_ModuleConfig_MQTTConfig &config, MQTTC
LOG_ERROR(warning);
#ifndef PIO_UNIT_TESTING
meshtastic_ClientNotification *cn = clientNotificationPool.allocZeroed();
cn->level = meshtastic_LogRecord_Level_ERROR;
cn->time = getValidTime(RTCQualityFromNet);
strncpy(cn->message, warning, sizeof(cn->message) - 1);
cn->message[sizeof(cn->message) - 1] = '\0'; // Ensure null termination
service->sendClientNotification(cn);
if (cn) {
cn->level = meshtastic_LogRecord_Level_ERROR;
cn->time = getValidTime(RTCQualityFromNet);
strncpy(cn->message, warning, sizeof(cn->message) - 1);
cn->message[sizeof(cn->message) - 1] = '\0'; // Ensure null termination
service->sendClientNotification(cn);
}
#endif
return false;
#endif
@@ -611,11 +617,13 @@ bool MQTT::isValidConfig(const meshtastic_ModuleConfig_MQTTConfig &config, MQTTC
LOG_ERROR(warning);
#ifndef PIO_UNIT_TESTING
meshtastic_ClientNotification *cn = clientNotificationPool.allocZeroed();
cn->level = meshtastic_LogRecord_Level_ERROR;
cn->time = getValidTime(RTCQualityFromNet);
strncpy(cn->message, warning, sizeof(cn->message) - 1);
cn->message[sizeof(cn->message) - 1] = '\0'; // Ensure null termination
service->sendClientNotification(cn);
if (cn) {
cn->level = meshtastic_LogRecord_Level_ERROR;
cn->time = getValidTime(RTCQualityFromNet);
strncpy(cn->message, warning, sizeof(cn->message) - 1);
cn->message[sizeof(cn->message) - 1] = '\0'; // Ensure null termination
service->sendClientNotification(cn);
}
#endif
return false;
}
@@ -746,6 +754,8 @@ void MQTT::perhapsReportToMap()
// Allocate MeshPacket and fill it
meshtastic_MeshPacket *mp = packetPool.allocZeroed();
if (!mp)
return;
mp->which_payload_variant = meshtastic_MeshPacket_decoded_tag;
mp->from = nodeDB->getNodeNum();
mp->to = NODENUM_BROADCAST;