Skip to content

MQTTAsync_addCommand() leaks the command when the list insert fails #1684

Description

@K-ANOY

I found a memory leak in MQTTAsync_addCommand(): when the underlying ListInsert() / ListAppend() fails (its internal ListElement allocation fails), the command is neither stored in the queue nor freed, so it leaks. Because this function is the single enqueue point for several public async APIs, the leak is systematic.

File: src/MQTTAsyncUtils.c

Function: MQTTAsync_addCommand

    /* CONNECT / DISCONNECT: insert at head */
    ListElement* result = ListInsert(MQTTAsync_commands, command, command_size, MQTTAsync_commands->first);
    if (result == NULL)
        rc = PAHO_MEMORY_ERROR;          /* command not stored, not freed -> leak */
    ...
    /* other commands: append */
    if (ListAppend(MQTTAsync_commands, command, command_size) == NULL)
    {
        rc = PAHO_MEMORY_ERROR;
        goto exit;                       /* command not stored, not freed -> leak */
    }

The key point is what ListAppend() / ListInsert() do on failure (src/LinkedList.c):

ListElement* ListAppend(List* aList, void* content, size_t size)
{
    ListElement* newel = malloc(sizeof(ListElement));
    if (newel)
        ListAppendNoMalloc(aList, content, newel, size);
    return newel;
}

When malloc(sizeof(ListElement)) fails, ListAppendNoMalloc() is never called, so content (the command) is not stored and not freed; the function just returns NULL. ListInsert() has the same semantics.

So on the result == NULL / ListAppend(...) == NULL paths, MQTTAsync_addCommand() sets rc = PAHO_MEMORY_ERROR and returns without freeing command. The callers simply propagate rc, so the caller-allocated command is leaked. For SUBSCRIBE / UNSUBSCRIBE / PUBLISH commands this also leaks the nested fields the caller attached (topics, payload, properties, destinationName).

Callers that enqueue through this path include MQTTAsync_connect(), MQTTAsync_reconnect(), MQTTAsync_subscribeMany(), MQTTAsync_unsubscribeMany(), MQTTAsync_send(), MQTTAsync_disconnect1(), and internal timeout/reconnect command creation.

A focused fix is to free the command (with its nested fields) when the insert fails, e.g.:

    if (ListAppend(MQTTAsync_commands, command, command_size) == NULL)
    {
        MQTTAsync_freeCommand(command);
        rc = PAHO_MEMORY_ERROR;
        goto exit;
    }

and the same for the ListInsert() branch. Note: the duplicate-CONNECT/DISCONNECT branch already calls MQTTAsync_freeCommand(command) and returns MQTTASYNC_COMMAND_IGNORED; the callers should be checked so this new free does not turn into a double free for any caller that already frees on error.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions