Skip to content

[Windows][3.2.0] Valid SOCKET handles are incorrectly rejected by FD_SETSIZE checks #863

Description

@sunmooncode

Description

On Windows, libmodbus 3.2.0 can reject valid TCP sockets before calling
connect() because it compares the numeric value of a Windows SOCKET
handle against FD_SETSIZE.

Windows SOCKET values are opaque handles. They are not POSIX-style file
descriptor indexes and may legitimately be much larger than FD_SETSIZE.

This causes modbus_connect() to return -1 even when the server is reachable.

Environment

  • OS: Windows 11 x64
  • Compiler: MSVC 14.50
  • libmodbus: 3.2.0
  • Transport: Modbus TCP
  • Installation: vcpkg
  • FD_SETSIZE: 64

Example socket handles observed on the affected machine:

2360
2364
2368
2372
2376

All of these are valid Windows socket handles, but all are numerically greater
than 64.

Regression

Using the same executable and configuration:

  • libmodbus 3.1.11: TCP connection succeeds
  • libmodbus 3.2.0: TCP connection fails
  • Replacing only modbus-5.dll 3.2.0 with 3.1.11 restores connectivity

The failure can also be reproduced against a loopback TCP listener, so it is
not related to the PLC, routing, firewall, or remote server.

Relevant code

In src/modbus-tcp.c, _modbus_tcp_connect() contains:

s = socket(PF_INET, flags, 0);
if (s < 0) {
    return -1;
}

if (s >= FD_SETSIZE) {
    if (ctx->debug) {
        fprintf(
            stderr,
            "ERROR Socket descriptor %d exceeds FD_SETSIZE (%d)\n",
            s,
            FD_SETSIZE);
    }
    close(s);
    errno = EINVAL;
    return -1;
}

A similar check exists in _connect():

FD_ZERO(&wset);
if (sockfd >= FD_SETSIZE) {
    errno = EINVAL;
    return -1;
}
FD_SET(sockfd, &wset);

There are additional numeric FD_SETSIZE checks in the same file.

Why this is incorrect on Windows

On POSIX systems, file descriptors are small integer indexes, so checking the
descriptor value against FD_SETSIZE may be meaningful when using select().

On Winsock, SOCKET is an opaque handle. Its numeric value does not represent
an index into fd_set.

Windows FD_SET stores socket handles in an array and is limited by the number
of handles stored, not by each handle's numeric value. The first argument of
Winsock select() is also ignored.

Therefore, a condition such as:

socket_handle >= FD_SETSIZE

does not indicate an invalid socket on Windows.

Expected behavior

modbus_connect() should attempt the TCP connection when socket() returns a
valid Windows SOCKET, regardless of its numeric value.

Actual behavior

modbus_connect() returns -1 before attempting the TCP connection whenever
the Windows socket handle is numerically greater than or equal to
FD_SETSIZE.

Minimal reproduction

Start any TCP listener on localhost port 1502, then run:

#include <errno.h>
#include <modbus.h>
#include <stdio.h>

int main(void)
{
    modbus_t *ctx = modbus_new_tcp("127.0.0.1", 1502);
    if (ctx == NULL) {
        return 1;
    }

    modbus_set_debug(ctx, TRUE);

    int result = modbus_connect(ctx);
    printf(
        "modbus_connect result=%d errno=%d error=%s\n",
        result,
        errno,
        modbus_strerror(errno));

    if (result == 0) {
        modbus_close(ctx);
    }

    modbus_free(ctx);
    return result == 0 ? 0 : 1;
}

With libmodbus 3.2.0 on the affected Windows system, the listener receives no
connection and modbus_connect() returns -1.

With libmodbus 3.1.11, the listener accepts the connection.

Suggested fix

Do not compare the numeric value of a Windows SOCKET against FD_SETSIZE.

For example, guard these checks so they only apply to non-Windows platforms:

#ifndef OS_WIN32
if (s >= FD_SETSIZE) {
    close(s);
    errno = EINVAL;
    return -1;
}
#endif

The same treatment may be required for the other numeric FD_SETSIZE checks
in src/modbus-tcp.c.

On Windows, code should instead ensure that the number of sockets inserted into
an fd_set does not exceed its capacity.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions