Skip to content

Fix freeaddrinfo(NULL) in _modbus_tcp_pi_connect#853

Open
NeriaIfrah wants to merge 1 commit into
stephane:masterfrom
NeriaIfrah:fix/tcp-pi-connect-freeaddrinfo-null
Open

Fix freeaddrinfo(NULL) in _modbus_tcp_pi_connect#853
NeriaIfrah wants to merge 1 commit into
stephane:masterfrom
NeriaIfrah:fix/tcp-pi-connect-freeaddrinfo-null

Conversation

@NeriaIfrah
Copy link
Copy Markdown

@NeriaIfrah NeriaIfrah commented May 18, 2026

Summary

Completes the fix from #831 / 26dc8a5 by applying the same NULL-guard
to the client-side mirror function _modbus_tcp_pi_connect, which has
identical error-handling code and was overlooked when the server-side
function was patched.

Closes #852.

Root cause

_modbus_tcp_pi_connect in src/modbus-tcp.c calls
freeaddrinfo(ai_list) unconditionally when getaddrinfo() fails:

ai_list = NULL;
rc = getaddrinfo(ctx_tcp_pi->node, ctx_tcp_pi->service, &ai_hints, &ai_list);
if (rc != 0) {
    if (ctx->debug) { ... }
    freeaddrinfo(ai_list);   /* may be NULL */
    errno = ECONNREFUSED;
    return -1;
}

POSIX leaves the contents of *res unspecified on getaddrinfo()
failure, and freeaddrinfo(NULL) is undefined behaviour. The
practical breakdown:

libc freeaddrinfo(NULL) behaviour
glibc Silently tolerated (latent)
uClibc Mostly tolerated
musl SIGSEGV

Alpine, OpenWrt, Void (musl variant), and most musl-based Buildroot/
Yocto images are therefore vulnerable. The trigger is not a malformed
packet — it's a hostname that fails to resolve, which is routine
operational reality on industrial gateways during DHCP/DNS warm-up,
transient DNS outages, or configuration typos.

See #852 for the full bug report, reproduction steps, and impact analysis.

Fix

--- a/src/modbus-tcp.c
+++ b/src/modbus-tcp.c
@@ -437,7 +437,9 @@ static int _modbus_tcp_pi_connect(modbus_t *ctx)
             fprintf(stderr, "Error returned by getaddrinfo: %d\n", rc);
 #endif
         }
-        freeaddrinfo(ai_list);
+        if (ai_list != NULL) {
+            freeaddrinfo(ai_list);
+        }
         errno = ECONNREFUSED;
         return -1;
     }

When getaddrinfo() fails in _modbus_tcp_pi_connect, the error path
calls freeaddrinfo(ai_list) unconditionally. POSIX leaves the contents
of *res unspecified on getaddrinfo() failure, and freeaddrinfo(NULL)
is undefined behaviour. On musl libc this causes a segfault, so a
hostname that fails to resolve crashes the host application instead
of returning -1 with errno=ECONNREFUSED as documented.

This is the same defect that was fixed for the server-side mirror
function modbus_tcp_pi_listen in commit 26dc8a5 ("Check if ai_list
is null before freeing it (stephane#831)"). The client-side mirror has the
byte-for-byte identical error-handling code and was overlooked.

Apply the same guard pattern to _modbus_tcp_pi_connect to complete
the fix on the client path. Affects Alpine, OpenWrt, and other
musl-based deployments, which are core libmodbus targets.

Closes #<NEW_ISSUE_NUMBER>
@cla-bot
Copy link
Copy Markdown

cla-bot Bot commented May 18, 2026

We require contributors to sign our Contributor License Agreement. In order for us to review and merge your code, please fill https://forms.gle/5635zjphDo5JEJQSA to get added. Your document will be manually checked by the maintainer. Be patient...

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.

Segfault on musl: _modbus_tcp_pi_connect calls freeaddrinfo(NULL) on getaddrinfo failure (mirror of #831)

1 participant