Skip to content

Fix two c-ares resource leaks in SRV support - #3666

Open
szarta wants to merge 2 commits into
eclipse-mosquitto:masterfrom
szarta:fix/srv-ares-resource-cleanup
Open

Fix two c-ares resource leaks in SRV support#3666
szarta wants to merge 2 commits into
eclipse-mosquitto:masterfrom
szarta:fix/srv-ares-resource-cleanup

Conversation

@szarta

@szarta szarta commented Jun 16, 2026

Copy link
Copy Markdown

Summary

Two related memory leaks in SRV (WITH_SRV) support, both reproduced and
fixed:

  1. Per-lookup leaksrv_callback() never frees the
    struct ares_srv_reply list returned by ares_parse_srv_reply().
  2. Per-client leak — the c-ares channel mosq->achan created by
    mosquitto_connect_srv() (ares_init) is never released; ares_destroy()
    is called nowhere in the library.

Two commits, one per file (lib/srv_mosq.c, lib/libmosquitto.c).

Bug 1 — reply list leaked in srv_callback (lib/srv_mosq.c)

if(status == ARES_SUCCESS){
    status = ares_parse_srv_reply(abuf, alen, &reply);
    if(status == ARES_SUCCESS){
        mosquitto_connect(mosq, reply->host, reply->port, mosq->keepalive);
    }
}else{ ... }

ares_parse_srv_reply() allocates a list into reply that the c-ares API
requires the caller to release with ares_free_data(); it is never freed
(ares_free_data appears nowhere in the tree). Every successful SRV lookup
leaks the reply list.

Bug 2 — c-ares channel leaked (lib/libmosquitto.c)

mosquitto_connect_srv() does ares_init(&mosq->achan). The channel is used by
the event loop (ares_fds/ares_process in lib/loop.c) but ares_destroy()
is never called, and mosquitto__destroy() does not touch achan. Every client
that uses SRV connection leaks the whole channel (tens of KB) until process
exit; repeatedly creating/destroying SRV clients leaks one channel each.

Reproduction

WITH_SRV build, a resolvable _mqtt._tcp.<domain> SRV record, and a minimal
client that calls mosquitto_connect_srv() then pumps the loop so the resolver
callback fires:

valgrind --leak-check=full --show-leak-kinds=all ./srv_client <domain>

Validation (valgrind, c-ares 1.18.1)

unpatched:            definitely lost: 74,320 bytes in 2 blocks
  - 73 (56+17) bytes  ->  ares_parse_srv_reply -> srv_callback (srv_mosq.c)   [bug 1]
  - 74,264 bytes      ->  ares_init -> mosquitto_connect_srv (srv_mosq.c)     [bug 2]

bug 1 fix only:       definitely lost: 74,264 bytes in 1 blocks   (channel remains)
bug 2 fix only:       definitely lost: 73 bytes                   (reply remains)
both fixes:           no SRV-related "definitely lost" blocks

Fixes

lib/srv_mosq.c — free the reply once consumed (reply is NULL-initialised and
ares_free_data(NULL) is a no-op, safe on the parse-failure sub-branch):

 		if(status == ARES_SUCCESS){
 			mosquitto_connect(mosq, reply->host, reply->port, mosq->keepalive);
 		}
+		ares_free_data(reply);
 	}else{

lib/libmosquitto.c — destroy the channel in mosquitto__destroy() (mosq is
calloc'd, so achan is NULL when SRV was unused; <ares.h> is already
included under WITH_SRV):

 	if(!mosq){
 		return;
 	}
+#ifdef WITH_SRV
+	if(mosq->achan){
+		ares_destroy(mosq->achan);
+		mosq->achan = NULL;
+	}
+#endif

Disclosure: these fixes were identified with the help of AI-assisted static
analysis and the description was drafted with AI assistance. Both defects were
reproduced and validated under valgrind (output above), and I have reviewed and
verified the changes myself and sign off on them under the DCO/ECA.

On a successful SRV lookup, ares_parse_srv_reply() allocates a
struct ares_srv_reply list into reply, which srv_callback() consumes but
never releases. c-ares requires this to be freed with ares_free_data(),
which appears nowhere in the tree, so every successful SRV-based
connection (mosquitto_connect_srv) leaks the reply list.

Free reply with ares_free_data() once consumed. reply is initialised to
NULL and ares_free_data(NULL) is a no-op, so this is safe on the
parse-failure sub-branch as well. WITH_SRV builds only.

Signed-off-by: Brandon Arrendondo <barrendo@gmail.com>
@szarta szarta closed this Jun 16, 2026
@szarta szarta reopened this Jun 16, 2026
mosquitto_connect_srv() initialises a c-ares resolver channel with
ares_init(&mosq->achan), but mosquitto__destroy() never releases it and
ares_destroy() is called nowhere in the library. Every client that uses
SRV-based connection leaks the entire c-ares channel (tens of KB) until
process exit; an application that repeatedly creates and destroys SRV
clients leaks it each time.

Destroy the channel in mosquitto__destroy() when it has been initialised.
mosq is calloc'd so achan is NULL when SRV was never used, making the
guard safe. WITH_SRV builds only.

Signed-off-by: Brandon Arrendondo <barrendo@gmail.com>
@szarta
szarta force-pushed the fix/srv-ares-resource-cleanup branch from 1905a30 to bf1b735 Compare June 16, 2026 01:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant