[Bug]: Memory leaks in MQTT::onReceive

[esev]

Category

Other

Hardware

Not Applicable

Firmware Version

2.5.16.f81d3b0

Description

I believe there are two potential memory leaks in src/mqtt/MQTT.cpp MQTT::onReceive

1. firmware/src/mqtt/MQTT.cpp

   Line 148 in 761a99d

   return;

   Any pointers within the meshtastic_ServiceEnvelope that are non-NULL are leaked.

2. firmware/src/mqtt/MQTT.cpp

   Line 202 in 761a99d

   if (isToUs(p) || (tx && tx->has_user && rx && rx->has_user))

   The meshtastic_MeshPacket *p is leaked when the if statement evaluates to false

There are several places in MQTT::onReceive where calls to free/release are needed. If I may offer a suggestion; use std::unique_ptr to help ensure these are always freed/released on return

Example:

// make sure to free both strings and the MeshPacket (passing in NULL is acceptable)
struct ServiceEnvelopeCleaner {
    void operator()(meshtastic_ServiceEnvelope *e)
    {
        free(e->channel_id);
        free(e->gateway_id);
        free(e->packet);
    }
};
using ScopedServiceEnvelopeReleaser = std::unique_ptr<meshtastic_ServiceEnvelope, ServiceEnvelopeCleaner>;
struct PacketPoolMeshPacketReleaser {
    void operator()(meshtastic_MeshPacket *p)
    {
        packetPool.release(p);
    }
};
using PacketPoolMeshPacketPtr = std::unique_ptr<meshtastic_MeshPacket, PacketPoolMeshPacketReleaser>;

...

ScopedServiceEnvelopeReleaser cleanup_e(&e);

...

PacketPoolMeshPacketPtr p(packetPool.allocCopy(*e.packet));

...

router->enqueueReceivedMessage(p.release());

Relevant log output

No response

[thebentern]

Great analysis! Would you be willing to submit a PR making the proposed changes?

[esev]

Great analysis! Would you be willing to submit a PR making the proposed changes?

I would love to, and normally I would. But I can't currently sign the CLA due to restrictions at work. I'm working on resolving any conflicts there, but can't send PRs for now.

The above has been resolved. I'll send a PR.

[Matzebhv]

Same as here -> #5458

[leshniak]

Thank you @esev for the contribution, I'm already testing your changes on my device.

[Matzebhv]

I will test as soon as this pull is available in the coming alpha.