Skip to content

Null-check packet allocations in allocForSending and its callers #11086

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
thebentern merged 3 commits into from
Jul 20, 2026
Merged

Null-check packet allocations in allocForSending and its callers #11086

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1c8d43e
Null-check packet allocations in allocForSending and its callers
caveman99 Jul 20, 2026
8b39df5
Null-check allocDataProtobuf, allocAckNak and allocErrorResponse callers
caveman99 Jul 20, 2026
5076999
Keep telemetry sleep scheduling on allocation failure
caveman99 Jul 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/graphics/niche/InkHUD/Applets/System/Menu/MenuApplet.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2614,6 +2614,8 @@ uint16_t InkHUD::MenuApplet::getSystemInfoPanelHeight()
void InkHUD::MenuApplet::sendText(NodeNum dest, ChannelIndex channel, const char *message)
{
meshtastic_MeshPacket *p = router->allocForSending();
if (!p)
return;
p->decoded.portnum = meshtastic_PortNum_TEXT_MESSAGE_APP;
p->to = dest;
p->channel = channel;
Expand Down
2 changes: 2 additions & 0 deletions src/mesh/MeshModule.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ meshtastic_MeshPacket *MeshModule::allocAckNak(meshtastic_Routing_Error err, Nod
// So we manually call pb_encode_to_bytes and specify routing port number
// auto p = allocDataProtobuf(c);
meshtastic_MeshPacket *p = router->allocForSending();
if (!p)
return nullptr;
Comment on lines 59 to +61

@coderabbitai coderabbitai Bot Jul 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win

Propagate the nullable result to every allocAckNak() caller.

This guard makes allocAckNak() return nullptr, but RoutingModule::sendAckNak() immediately dereferences p->want_ack, and allocErrorResponse() passes the result to setReplyTo() without checking it. Add failure handling at both callers before they touch the packet; otherwise the allocation-failure fix still leaves a crash path.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/mesh/MeshModule.cpp` around lines 59 - 61, Update every caller of
allocAckNak() to handle a nullptr result before accessing or passing the packet
onward. In RoutingModule::sendAckNak(), check p before dereferencing
p->want_ack; in allocErrorResponse(), check the allocation result before calling
setReplyTo(), preserving the existing failure behavior where appropriate.

p->decoded.portnum = meshtastic_PortNum_ROUTING_APP;
p->decoded.payload.size =
pb_encode_to_bytes(p->decoded.payload.bytes, sizeof(p->decoded.payload.bytes), &meshtastic_Routing_msg, &c);
Expand Down
2 changes: 2 additions & 0 deletions src/mesh/ProtobufModule.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ template <class T> class ProtobufModule : protected SinglePortModule
{
// Update our local node info with our position (even if we don't decide to update anyone else)
meshtastic_MeshPacket *p = allocDataPacket();
if (!p)
return nullptr;
Comment thread
coderabbitai[bot] marked this conversation as resolved.

p->decoded.payload.size =
pb_encode_to_bytes(p->decoded.payload.bytes, sizeof(p->decoded.payload.bytes), fields, &payload);
Expand Down
2 changes: 2 additions & 0 deletions src/mesh/Router.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,8 @@ PacketId generatePacketId()
meshtastic_MeshPacket *Router::allocForSending()
{
meshtastic_MeshPacket *p = packetPool.allocZeroed();
if (!p)
return nullptr;

p->which_payload_variant = meshtastic_MeshPacket_decoded_tag; // Assume payload is decoded at start.
p->from = nodeDB->getNodeNum();
Expand Down
2 changes: 2 additions & 0 deletions src/mesh/SinglePortModule.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ class SinglePortModule : public MeshModule
{
// Update our local node info with our position (even if we don't decide to update anyone else)
meshtastic_MeshPacket *p = router->allocForSending();
if (!p)
return nullptr;
p->decoded.portnum = ourPortNum;

return p;
Expand Down
24 changes: 24 additions & 0 deletions src/modules/AdminModule.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1389,6 +1389,9 @@ void AdminModule::handleGetOwner(const meshtastic_MeshPacket &req)
res.which_payload_variant = meshtastic_AdminMessage_get_owner_response_tag;
setPassKey(&res);
myReply = allocDataProtobuf(res);
if (!myReply) {
return;
}
if (req.pki_encrypted) {
myReply->pki_encrypted = true;
}
Expand Down Expand Up @@ -1472,6 +1475,9 @@ void AdminModule::handleGetConfig(const meshtastic_MeshPacket &req, const uint32
res.which_payload_variant = meshtastic_AdminMessage_get_config_response_tag;
setPassKey(&res);
myReply = allocDataProtobuf(res);
if (!myReply) {
return;
}
if (req.pki_encrypted) {
myReply->pki_encrypted = true;
}
Expand Down Expand Up @@ -1581,6 +1587,9 @@ void AdminModule::handleGetModuleConfig(const meshtastic_MeshPacket &req, const
res.which_payload_variant = meshtastic_AdminMessage_get_module_config_response_tag;
setPassKey(&res);
myReply = allocDataProtobuf(res);
if (!myReply) {
return;
}
if (req.pki_encrypted) {
myReply->pki_encrypted = true;
}
Expand Down Expand Up @@ -1608,6 +1617,9 @@ void AdminModule::handleGetNodeRemoteHardwarePins(const meshtastic_MeshPacket &r
}
setPassKey(&r);
myReply = allocDataProtobuf(r);
if (!myReply) {
return;
}
if (req.pki_encrypted) {
myReply->pki_encrypted = true;
}
Expand All @@ -1627,6 +1639,9 @@ void AdminModule::handleGetDeviceMetadata(const meshtastic_MeshPacket &req)
r.which_payload_variant = meshtastic_AdminMessage_get_device_metadata_response_tag;
setPassKey(&r);
myReply = allocDataProtobuf(r);
if (!myReply) {
return;
}
if (req.pki_encrypted) {
myReply->pki_encrypted = true;
}
Expand Down Expand Up @@ -1702,6 +1717,9 @@ void AdminModule::handleGetDeviceConnectionStatus(const meshtastic_MeshPacket &r
r.which_payload_variant = meshtastic_AdminMessage_get_device_connection_status_response_tag;
setPassKey(&r);
myReply = allocDataProtobuf(r);
if (!myReply) {
return;
}
if (req.pki_encrypted) {
myReply->pki_encrypted = true;
}
Expand All @@ -1716,6 +1734,9 @@ void AdminModule::handleGetChannel(const meshtastic_MeshPacket &req, uint32_t ch
r.which_payload_variant = meshtastic_AdminMessage_get_channel_response_tag;
setPassKey(&r);
myReply = allocDataProtobuf(r);
if (!myReply) {
return;
}
if (req.pki_encrypted) {
myReply->pki_encrypted = true;
}
Expand All @@ -1728,6 +1749,9 @@ void AdminModule::handleGetDeviceUIConfig(const meshtastic_MeshPacket &req)
r.which_payload_variant = meshtastic_AdminMessage_get_ui_config_response_tag;
r.get_ui_config_response = uiconfig;
myReply = allocDataProtobuf(r);
if (!myReply) {
return;
}
if (req.pki_encrypted) {
myReply->pki_encrypted = true;
}
Expand Down
2 changes: 2 additions & 0 deletions src/modules/CannedMessageModule.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1022,6 +1022,8 @@ void CannedMessageModule::sendText(NodeNum dest, ChannelIndex channel, const cha
lastDestSet = true;

meshtastic_MeshPacket *p = allocDataPacket();
if (!p)
return;
p->to = dest;
p->channel = channel;
p->want_ack = true;
Expand Down
8 changes: 8 additions & 0 deletions src/modules/DetectionSensorModule.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,10 @@ void DetectionSensorModule::sendDetectionMessage()
char *message = new char[40];
sprintf(message, "%s detected", moduleConfig.detection_sensor.name);
meshtastic_MeshPacket *p = allocDataPacket();
if (!p) {
delete[] message;
return;
}
p->want_ack = false;
p->decoded.payload.size = strlen(message);
memcpy(p->decoded.payload.bytes, message, p->decoded.payload.size);
Expand All @@ -144,6 +148,10 @@ void DetectionSensorModule::sendCurrentStateMessage(bool state)
char *message = new char[40];
sprintf(message, "%s state: %i", moduleConfig.detection_sensor.name, state);
meshtastic_MeshPacket *p = allocDataPacket();
if (!p) {
delete[] message;
return;
}
p->want_ack = false;
p->decoded.payload.size = strlen(message);
memcpy(p->decoded.payload.bytes, message, p->decoded.payload.size);
Expand Down
2 changes: 2 additions & 0 deletions src/modules/DropzoneModule.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,8 @@ meshtastic_MeshPacket *DropzoneModule::sendConditions()
// the dropzone is open
auto dropzoneStatus = analogRead(A1) < 100 ? "OPEN" : "CLOSED";
auto reply = allocDataPacket();
if (!reply)
return nullptr;

auto node = nodeDB->getMeshNode(nodeDB->getNodeNum());
if (sensor.hasSensor()) {
Expand Down
4 changes: 4 additions & 0 deletions src/modules/KeyVerificationModule.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,8 @@ bool KeyVerificationModule::sendInitialRequest(NodeNum remoteNode)
KeyVerification.hash1.size = 32;
memcpy(KeyVerification.hash1.bytes, owner.public_key.bytes, 32);
meshtastic_MeshPacket *p = allocDataProtobuf(KeyVerification);
if (!p)
return false;
p->to = remoteNode;
p->channel = 0;
// Only request PKI when we already hold the destination's key. Otherwise this first message goes out
Expand Down Expand Up @@ -318,6 +320,8 @@ void KeyVerificationModule::processSecurityNumber(uint32_t incomingNumber)
KeyVerification.hash1.size = 32;
memcpy(KeyVerification.hash1.bytes, hash1, 32);
meshtastic_MeshPacket *p = allocDataProtobuf(KeyVerification);
if (!p)
return;
p->to = currentRemoteNode;
p->channel = 0;
p->pki_encrypted = true;
Expand Down
2 changes: 2 additions & 0 deletions src/modules/NeighborInfoModule.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,8 @@ void NeighborInfoModule::sendNeighborInfo(NodeNum dest, bool wantReplies)
// only send neighbours if we have some to send
if (neighborInfo.neighbors_count > 0) {
meshtastic_MeshPacket *p = allocDataProtobuf(neighborInfo);
if (!p)
return;
p->to = dest;
p->decoded.want_response = wantReplies;
p->priority = meshtastic_MeshPacket_Priority_BACKGROUND;
Expand Down
4 changes: 4 additions & 0 deletions src/modules/PositionModule.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,8 @@ meshtastic_MeshPacket *PositionModule::allocAtakPli()
{
LOG_INFO("Send TAK V2 PLI packet");
meshtastic_MeshPacket *mp = allocDataPacket();
if (!mp)
return nullptr;
mp->decoded.portnum = meshtastic_PortNum_ATAK_PLUGIN_V2;

meshtastic_TAKPacketV2 takPacket = meshtastic_TAKPacketV2_init_zero;
Expand Down Expand Up @@ -572,6 +574,8 @@ int32_t PositionModule::runOnce()
void PositionModule::sendLostAndFoundText()
{
meshtastic_MeshPacket *p = allocDataPacket();
if (!p)
return;
p->to = NODENUM_BROADCAST;
char message[128];
int written = snprintf(message, sizeof(message), "🚨I'm lost! Lat / Lon: %f, %f\a", (lastGpsLatitude * 1e-7),
Expand Down
2 changes: 2 additions & 0 deletions src/modules/RangeTestModule.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,8 @@ int32_t RangeTestModule::runOnce()
void RangeTestModuleRadio::sendPayload(NodeNum dest, bool wantReplies)
{
meshtastic_MeshPacket *p = allocDataPacket();
if (!p)
return;
p->to = dest;
p->decoded.want_response = wantReplies;
p->hop_limit = 0;
Expand Down
2 changes: 2 additions & 0 deletions src/modules/ReplyBotModule.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,8 @@ void ReplyBotModule::sendDm(const meshtastic_MeshPacket &rx, const char *text)
if (!text)
return;
meshtastic_MeshPacket *p = allocDataPacket();
if (!p)
return;
p->to = rx.from;
p->channel = rx.channel;
p->want_ack = false;
Expand Down
4 changes: 3 additions & 1 deletion src/modules/ReplyModule.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@ meshtastic_MeshPacket *ReplyModule::allocReply()
#endif

const char *replyStr = "Message Received";
auto reply = allocDataPacket(); // Allocate a packet for sending
auto reply = allocDataPacket(); // Allocate a packet for sending
if (!reply)
return nullptr;
reply->decoded.payload.size = strlen(replyStr); // You must specify how many bytes are in the reply
memcpy(reply->decoded.payload.bytes, replyStr, reply->decoded.payload.size);

Expand Down
2 changes: 2 additions & 0 deletions src/modules/RoutingModule.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ void RoutingModule::sendAckNak(meshtastic_Routing_Error err, NodeNum to, PacketI
bool ackWantsAck)
{
auto p = allocAckNak(err, to, idFrom, chIndex, hopLimit);
if (!p)
return;

// Allow the caller to set want_ack on this ACK packet if it's important that the ACK be delivered reliably
p->want_ack = ackWantsAck;
Expand Down
4 changes: 4 additions & 0 deletions src/modules/SerialModule.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,8 @@ int32_t SerialModule::runOnce()
void SerialModule::sendTelemetry(meshtastic_Telemetry m)
{
meshtastic_MeshPacket *p = router->allocForSending();
if (!p)
return;
p->decoded.portnum = meshtastic_PortNum_TELEMETRY_APP;
p->decoded.payload.size =
pb_encode_to_bytes(p->decoded.payload.bytes, sizeof(p->decoded.payload.bytes), &meshtastic_Telemetry_msg, &m);
Expand Down Expand Up @@ -350,6 +352,8 @@ void SerialModuleRadio::sendPayload(NodeNum dest, bool wantReplies)
{
const meshtastic_Channel *ch = (boundChannel != NULL) ? &channels.getByName(boundChannel) : NULL;
meshtastic_MeshPacket *p = allocReply();
if (!p)
return;
p->to = dest;
if (ch != NULL) {
p->channel = ch->index;
Expand Down
2 changes: 2 additions & 0 deletions src/modules/SerialModule.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,8 @@ class SerialModuleRadio : public MeshModule
{
// Update our local node info with our position (even if we don't decide to update anyone else)
meshtastic_MeshPacket *p = router->allocForSending();
if (!p)
return nullptr;
p->decoded.portnum = ourPortNum;

return p;
Expand Down
2 changes: 2 additions & 0 deletions src/modules/StatusMessageModule.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ int32_t StatusMessageModule::runOnce()
strncpy(ourStatus.status, moduleConfig.statusmessage.node_status, sizeof(ourStatus.status));
ourStatus.status[sizeof(ourStatus.status) - 1] = '\0'; // ensure null termination
meshtastic_MeshPacket *p = allocDataPacket();
if (!p)
return 1000 * 12 * 60 * 60;
p->decoded.payload.size = pb_encode_to_bytes(p->decoded.payload.bytes, sizeof(p->decoded.payload.bytes),
meshtastic_StatusMessage_fields, &ourStatus);
p->to = NODENUM_BROADCAST;
Expand Down
6 changes: 6 additions & 0 deletions src/modules/StoreForwardModule.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,8 @@ meshtastic_MeshPacket *StoreForwardModule::preparePayload(NodeNum dest, uint32_t
(this->packetHistory[i].to == NODENUM_BROADCAST || this->packetHistory[i].to == dest)) {

meshtastic_MeshPacket *p = allocDataPacket();
if (!p)
return nullptr;

p->to = local ? this->packetHistory[i].to : dest; // PhoneAPI can handle original `to`
p->from = this->packetHistory[i].from;
Expand Down Expand Up @@ -304,6 +306,8 @@ meshtastic_MeshPacket *StoreForwardModule::preparePayload(NodeNum dest, uint32_t
void StoreForwardModule::sendMessage(NodeNum dest, const meshtastic_StoreAndForward &payload)
{
meshtastic_MeshPacket *p = allocDataProtobuf(payload);
if (!p)
return;

p->to = dest;

Expand Down Expand Up @@ -340,6 +344,8 @@ void StoreForwardModule::sendMessage(NodeNum dest, meshtastic_StoreAndForward_Re
void StoreForwardModule::sendErrorTextMessage(NodeNum dest, bool want_response)
{
meshtastic_MeshPacket *pr = allocDataPacket();
if (!pr)
return;
pr->to = dest;
pr->priority = meshtastic_MeshPacket_Priority_BACKGROUND;
pr->want_ack = false;
Expand Down
60 changes: 32 additions & 28 deletions src/modules/Telemetry/AirQualityTelemetry.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -464,35 +464,39 @@ bool AirQualityTelemetryModule::sendTelemetry(NodeNum dest, bool phoneOnly)
}

meshtastic_MeshPacket *p = allocDataProtobuf(m);
p->to = dest;
p->decoded.want_response = false;
if (config.device.role == meshtastic_Config_DeviceConfig_Role_SENSOR)
p->priority = meshtastic_MeshPacket_Priority_RELIABLE;
else
p->priority = meshtastic_MeshPacket_Priority_BACKGROUND;

// release previous packet before occupying a new spot
if (lastMeasurementPacket != nullptr)
packetPool.release(lastMeasurementPacket);

lastMeasurementPacket = packetPool.allocCopy(*p);
if (phoneOnly) {
LOG_INFO("Sending packet to phone");
service->sendToPhone(p);
if (!p) {
validTelemetry = false;
} else {
LOG_INFO("Sending packet to mesh");
service->sendToMesh(p, RX_SRC_LOCAL, true);

if (isPowerSavingSensor()) {
meshtastic_ClientNotification *notification = clientNotificationPool.allocZeroed();
if (notification) {
notification->level = meshtastic_LogRecord_Level_INFO;
notification->time = getValidTime(RTCQualityFromNet);
sprintf(notification->message, "Sending telemetry and sleeping for %us interval in a moment",
Default::getConfiguredOrDefaultMs(moduleConfig.telemetry.air_quality_interval,
default_telemetry_broadcast_interval_secs) /
1000U);
service->sendClientNotification(notification);
p->to = dest;
p->decoded.want_response = false;
if (config.device.role == meshtastic_Config_DeviceConfig_Role_SENSOR)
p->priority = meshtastic_MeshPacket_Priority_RELIABLE;
else
p->priority = meshtastic_MeshPacket_Priority_BACKGROUND;

// release previous packet before occupying a new spot
if (lastMeasurementPacket != nullptr)
packetPool.release(lastMeasurementPacket);

lastMeasurementPacket = packetPool.allocCopy(*p);
if (phoneOnly) {
LOG_INFO("Sending packet to phone");
service->sendToPhone(p);
} else {
LOG_INFO("Sending packet to mesh");
service->sendToMesh(p, RX_SRC_LOCAL, true);

if (isPowerSavingSensor()) {
meshtastic_ClientNotification *notification = clientNotificationPool.allocZeroed();
if (notification) {
notification->level = meshtastic_LogRecord_Level_INFO;
notification->time = getValidTime(RTCQualityFromNet);
sprintf(notification->message, "Sending telemetry and sleeping for %us interval in a moment",
Default::getConfiguredOrDefaultMs(moduleConfig.telemetry.air_quality_interval,
default_telemetry_broadcast_interval_secs) /
1000U);
service->sendClientNotification(notification);
}
}
}
}
Expand Down
4 changes: 4 additions & 0 deletions src/modules/Telemetry/DeviceTelemetry.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,8 @@ meshtastic_Telemetry DeviceTelemetryModule::getLocalStatsTelemetry()
void DeviceTelemetryModule::sendLocalStatsToPhone()
{
meshtastic_MeshPacket *p = allocDataProtobuf(getLocalStatsTelemetry());
if (!p)
return;
p->to = NODENUM_BROADCAST;
p->decoded.want_response = false;
p->priority = meshtastic_MeshPacket_Priority_BACKGROUND;
Expand All @@ -191,6 +193,8 @@ bool DeviceTelemetryModule::sendTelemetry(NodeNum dest, bool phoneOnly)
meshtastic_MeshPacket *p = allocDataProtobuf(telemetry);
DEBUG_HEAP_AFTER("DeviceTelemetryModule::sendTelemetry", p);

if (!p)
return false;
p->to = dest;
p->decoded.want_response = false;
p->priority = meshtastic_MeshPacket_Priority_BACKGROUND;
Expand Down
Loading
Loading