Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: pipe_overlong_path fails on AIX #4231

Open
abmusse opened this issue Nov 17, 2023 · 4 comments
Open

test: pipe_overlong_path fails on AIX #4231

abmusse opened this issue Nov 17, 2023 · 4 comments
Labels

Comments

@abmusse
Copy link
Contributor

abmusse commented Nov 17, 2023

  • Version: Current v1.x branch
  • Platform: AIX

Discovered this failure while running the CI for a pull request:

Ref: #4229

https://ci.nodejs.org/job/libuv-test-commit-aix/nodes=aix72-ppc64/2279/console

not ok 199 - pipe_overlong_path
# exit code 393222
# Output from process `pipe_overlong_path`:
# Assertion failed in test/test-pipe-bind-error.c on line 175: `UV_EINVAL == uv_pipe_bind2(&pipe, path, sizeof(path), UV_PIPE_NO_TRUNCATE)` (-22 == -86)

ASSERT_EQ(UV_EINVAL,

Looks like errno 86 maps to ENAMETOOLONG on AIX.

Looks like this passes as expected on IBM i:

ok 199 - pipe_overlong_path

https://ci.nodejs.org/job/libuv-test-commit-aix/nodes=aix72-ppc64/2279/console

@richardlau richardlau added the aix label Nov 17, 2023
@bnoordhuis
Copy link
Member

What is the size of struct sockaddr_un.sun_path on AIX?

@abmusse
Copy link
Contributor Author

abmusse commented Dec 21, 2023

@bnoordhuis

What is the size of struct sockaddr_un.sun_path on AIX?

Looks like its set to 1024 or 1023

From: /usr/include/sys/un.h

#if defined(COMPAT_43) && !defined(_KERNEL)
struct  sockaddr_un {
        ushort_t        sun_family;     /* AF_UNIX */
        char        sun_path[1024]; /* changed from 104 to PATH_MAX to support long user names. This value should be ch
anged manually if PATH_MAX changes in future */
};
#else
struct  sockaddr_un {
        uchar_t     sun_len;            /* sockaddr len including null */
        sa_family_t sun_family;         /* AF_UNIX */
        char        sun_path[1023]; /* changed from 104 to PATH_MAX to support long user names. This value should be ch
anged manually if PATH_MAX changes in future */
};
#endif /* COMPAT_43 && !_KERNEL */

@abmusse
Copy link
Contributor Author

abmusse commented Jun 5, 2024

@bnoordhuis @richardlau

I got around to adding some debug print statements to get a better understanding of what is going on. My changes are here for ref: https://github.com/libuv/libuv/compare/v1.x...abmusse:libuv:debug-pipe-overlong-path?expand=1

not ok 200 - pipe_overlong_path
# exit code 393222
# Output from process `pipe_overlong_path`:
# Calling uv_pipe_init
# 
# Calling uv_pipe_bind2
# 
# flags & UV_PIPE_NO_TRUNCATE
# 
# namelen: 512
# 
# saddr.sun_path: 1023
# 
# malloc pipe_fname
# 
# failed to bind sockfs
# 
# goto err_socket
# 
# In err_socket, err: -86
# 

ref: https://ci.nodejs.org/job/libuv-test-commit-aix/nodes=aix72-ppc64/lastBuild/console

I confirmed that saddr.sun_path is set to 1023.

The namelen being passed in test case is 512 which is why its skipping the check

libuv/src/unix/pipe.c

Lines 84 to 86 in 6be130e

if (flags & UV_PIPE_NO_TRUNCATE)
if (namelen > sizeof(saddr.sun_path))
return UV_EINVAL;

The weird part is that bind fails when setting saddr.sun_path to the long name.

libuv/src/unix/pipe.c

Lines 116 to 127 in 6be130e

memset(&saddr, 0, sizeof saddr);
memcpy(&saddr.sun_path, name, namelen);
saddr.sun_family = AF_UNIX;
if (bind(sockfd, (struct sockaddr*)&saddr, sizeof saddr)) {
err = UV__ERR(errno);
/* Convert ENOENT to EACCES for compatibility with Windows. */
if (err == UV_ENOENT)
err = UV_EACCES;
uv__close(sockfd);
goto err_socket;

We get back errno 86 which looks like it maps to ENAMETOOLONG on AIX.

From my previous comment:

#if defined(COMPAT_43) && !defined(_KERNEL)
struct  sockaddr_un {
        ushort_t        sun_family;     /* AF_UNIX */
        char        sun_path[1024]; /* changed from 104 to PATH_MAX to support long user names. This value should be ch
anged manually if PATH_MAX changes in future */
};
#else
struct  sockaddr_un {
        uchar_t     sun_len;            /* sockaddr len including null */
        sa_family_t sun_family;         /* AF_UNIX */
        char        sun_path[1023]; /* changed from 104 to PATH_MAX to support long user names. This value should be ch
anged manually if PATH_MAX changes in future */
};
#endif /* COMPAT_43 && !_KERNEL */

It seems like on AIX sun_path was increased from support for long user names?

I think we need to find the true max allowed name length for a socket since the current sun_path of 1023 looks too big.

@abmusse
Copy link
Contributor Author

abmusse commented Jun 6, 2024

As a follow up I made a re-producible program in C

// unix-sock.c
// gcc -o unix-sock unix-sock.c
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <sys/socket.h>
#include <sys/un.h>

int main() {
    int sockfd;
    struct sockaddr_un addr;
    printf( "addr.sun_path: %lu\n\n", sizeof(addr.sun_path));

    char SOCKET_PATH[512];
    memset(SOCKET_PATH, '@', sizeof(SOCKET_PATH));
    
    // Create a Unix domain socket
    sockfd = socket(AF_UNIX, SOCK_STREAM, 0);
    if (sockfd == -1) {
        perror("socket");
        exit(EXIT_FAILURE);
    }

    // Clear the structure
    memset(&addr, 0, sizeof(struct sockaddr_un));

    // Assign the socket family and the socket path
    addr.sun_family = AF_UNIX;
    strncpy(addr.sun_path, SOCKET_PATH, sizeof(addr.sun_path) - 1);

    // Bind the socket
    if (bind(sockfd, (struct sockaddr *)&addr, sizeof(struct sockaddr_un)) == -1) {
        perror("bind");
        close(sockfd);
        exit(EXIT_FAILURE);
    }

    printf("Socket bound to %s\n", SOCKET_PATH);

    close(sockfd);

    return 0;
}

I then compiled and ran it on AIX:

$ gcc -o unix-sock unix-sock.c 
$ ./unix-sock 
addr.sun_path: 1023

bind: File name too long

For fun I then compiled and ran it on IBM i to see if there would be a diff in behavior:

$ gcc -o unix-sock unix-sock.c
$ ./unix-sock                                                                                                  
addr.sun_path: 126                                                                                                    
                                                                                                                       
Socket bound to @@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@
@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@
@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@
@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@
@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@                                                                   

There is a def a diff in behavior in relation to File name max length.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants