Skip to content

Router::allocForSending: null-check packet pool allocation #10261

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

Open
nightjoker7 wants to merge 4 commits into
base: develop
Choose a base branch
from
Open

Router::allocForSending: null-check packet pool allocation #10261

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6a77c93
Router::allocForSending: null-check packet pool allocation
nightjoker7 Apr 23, 2026
931f2ed
Update src/mesh/Router.cpp
thebentern Apr 23, 2026
43bb584
Audit allocForSending() callers + null-check unsafe ones
nightjoker7 Apr 23, 2026
ac1dddc
Address Copilot caller-audit follow-ups: ProtobufModule null-safe + s…
nightjoker7 Apr 24, 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
4 changes: 4 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 @@ -1993,6 +1993,10 @@ uint16_t InkHUD::MenuApplet::getSystemInfoPanelHeight()
void InkHUD::MenuApplet::sendText(NodeNum dest, ChannelIndex channel, const char *message)
{
meshtastic_MeshPacket *p = router->allocForSending();
if (!p) {
LOG_WARN("MenuApplet::sendText: packet pool exhausted, dropping message");
return;
}
p->decoded.portnum = meshtastic_PortNum_TEXT_MESSAGE_APP;
p->to = dest;
p->channel = channel;
Expand Down
8 changes: 8 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,12 @@ 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) {
// Router::allocForSending / MemoryPool::alloc already LOG_WARN on exhaustion;
// don't double-log. An unthrottled WARN here would spam during an ACK/NAK storm
// — which is exactly the scenario that drives the pool to empty in the first place.
return nullptr;
}
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 All @@ -79,6 +85,8 @@ meshtastic_MeshPacket *MeshModule::allocErrorResponse(meshtastic_Routing_Error e
uint8_t channelIndex =
p->which_payload_variant == meshtastic_MeshPacket_decoded_tag ? p->channel : channels.getPrimaryIndex();
auto r = allocAckNak(err, getFrom(p), p->id, channelIndex);
if (!r)
return nullptr;

setReplyTo(r, *p);

Expand Down
8 changes: 7 additions & 1 deletion src/mesh/ProtobufModule.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,14 @@ template <class T> class ProtobufModule : protected SinglePortModule
*/
meshtastic_MeshPacket *allocDataProtobuf(const T &payload)
{
// Update our local node info with our position (even if we don't decide to update anyone else)
// allocDataPacket() now returns nullptr on packet-pool exhaustion (since
// Router::allocForSending was made null-safe in #10261). Propagate the nullptr
// to the caller rather than dereferencing `p->decoded`. All current callers
// either null-check the return or forward it to a helper that does — see the
// caller audit in the #10261 PR description.
meshtastic_MeshPacket *p = allocDataPacket();
if (!p)
return nullptr;

p->decoded.payload.size =
pb_encode_to_bytes(p->decoded.payload.bytes, sizeof(p->decoded.payload.bytes), fields, &payload);
Expand Down
3 changes: 3 additions & 0 deletions src/mesh/Router.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,9 @@ 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;
Comment on lines 33 to 39
Copy link

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

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

allocDataPacket() can now return nullptr when the packet pool is exhausted. Many existing callers (including ProtobufModule::allocDataProtobuf() and several modules that call allocDataPacket() directly) assume a non-null return and will still dereference the pointer, so the pool-exhaustion crash is not fully addressed. Either update the common helper(s) to propagate/handle nullptr safely (e.g., make allocDataProtobuf() return nullptr when allocation fails) and/or update call sites to check for nullptr before dereferencing.

Copilot uses AI. Check for mistakes.
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 @@ -309,6 +309,10 @@ int32_t SerialModule::runOnce()
void SerialModule::sendTelemetry(meshtastic_Telemetry m)
{
meshtastic_MeshPacket *p = router->allocForSending();
if (!p) {
LOG_WARN("SerialModule::sendTelemetry: packet pool exhausted, dropping");
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
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
12 changes: 12 additions & 0 deletions src/mqtt/MQTT.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,10 @@ inline void onReceiveJson(byte *payload, size_t length)

// construct protobuf data packet using TEXT_MESSAGE, send it to the mesh
meshtastic_MeshPacket *p = router->allocForSending();
if (!p) {
LOG_WARN("MQTT downlink sendtext dropped: packet pool exhausted");
return;
}
Comment on lines 191 to +195
Copy link

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

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

In the sendtext downlink path, a packet is allocated before validating jsonPayloadStr.length(). If the payload is too long, the code logs and drops but never releases the allocated packet back to the pool, which can permanently reduce available pool slots and make exhaustion more likely. Ensure the allocated packet is released (or avoid allocating until after length validation) on the oversize-payload drop path.

Copilot uses AI. Check for mistakes.
p->decoded.portnum = meshtastic_PortNum_TEXT_MESSAGE_APP;
if (json.find("channel") != json.end() && json["channel"]->IsNumber() &&
(json["channel"]->AsNumber() < channels.getNumChannels()))
Expand All @@ -202,7 +206,11 @@ inline void onReceiveJson(byte *payload, size_t length)
p->decoded.payload.size = jsonPayloadStr.length();
service->sendToMesh(p, RX_SRC_LOCAL);
} else {
// Release the allocated packet back to the pool — `p` would otherwise leak,
// permanently reducing the number of available slots for every future
// send attempt.
LOG_WARN("Received MQTT json payload too long, drop");
packetPool.release(p);
}
} else if (json["type"]->AsString().compare("sendposition") == 0 && json["payload"]->IsObject()) {
// invent the "sendposition" type for a valid envelope
Expand All @@ -220,6 +228,10 @@ inline void onReceiveJson(byte *payload, size_t length)

// construct protobuf data packet using POSITION, send it to the mesh
meshtastic_MeshPacket *p = router->allocForSending();
if (!p) {
LOG_WARN("MQTT downlink sendposition dropped: packet pool exhausted");
return;
}
p->decoded.portnum = meshtastic_PortNum_POSITION_APP;
if (json.find("channel") != json.end() && json["channel"]->IsNumber() &&
(json["channel"]->AsNumber() < channels.getNumChannels()))
Expand Down