console-socket: fix truncated socket address in error log An error log in obmc-console-client showed truncated socket names, e.g., "@obmc-console.hos" instead of "@obmc-console.host1". Address length confusion by introducing and renaming relevant variables. While we're there, tidy up some of the arithmetic to use offsetof() against sun_path rather than sizeof() against sun_family. Change-Id: I7c574cc2a3415fd76122da995edad77d5eab27a2 Signed-off-by: Sean He <sean.he.wiwynn@gmail.com> Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>
diff --git a/console-client.c b/console-client.c index 67d163a..d585920 100644 --- a/console-client.c +++ b/console-client.c
@@ -219,7 +219,8 @@ const char *resolved_id = NULL; struct sockaddr_un addr; socket_path_t path; - ssize_t len; + ssize_t addrlen; + ssize_t pathlen; int rc; client->console_sd = socket(AF_UNIX, SOCK_STREAM, 0); @@ -233,8 +234,8 @@ memset(&addr, 0, sizeof(addr)); addr.sun_family = AF_UNIX; - len = console_socket_path(addr.sun_path, resolved_id); - if (len < 0) { + pathlen = console_socket_path(addr.sun_path, resolved_id); + if (pathlen < 0) { if (errno) { warn("Failed to configure socket: %s", strerror(errno)); } else { @@ -242,15 +243,19 @@ } goto cleanup; } + addrlen = offsetof(struct sockaddr_un, sun_path) + pathlen; - rc = connect(client->console_sd, (struct sockaddr *)&addr, - sizeof(addr) - sizeof(addr.sun_path) + len); + rc = connect(client->console_sd, (struct sockaddr *)&addr, addrlen); if (!rc) { return 0; } - console_socket_path_readable(&addr, len, path); - warn("Can't connect to console server '@%s'", path); + pathlen = console_socket_path_readable(&addr, addrlen, path); + if (pathlen > 0) { + warn("Can't connect to console server '@%s'", path); + } else { + warnx("Can't connect to console server with unreadable path\n"); + } cleanup: close(client->console_sd); return -1;
diff --git a/console-socket.c b/console-socket.c index 3873440..06da14c 100644 --- a/console-socket.c +++ b/console-socket.c
@@ -57,15 +57,14 @@ ssize_t console_socket_path_readable(const struct sockaddr_un *addr, size_t addrlen, socket_path_t path) { - const char *src = (const char *)addr; size_t len; if (addrlen > SSIZE_MAX) { return -EINVAL; } - len = addrlen - sizeof(addr->sun_family) - 1; - memcpy(path, src + sizeof(addr->sun_family) + 1, len); + len = addrlen - offsetof(struct sockaddr_un, sun_path) - 1; + memcpy(path, addr->sun_path + 1, len); path[len] = '\0'; return (ssize_t)len; /* strlen() style */
diff --git a/socket-handler.c b/socket-handler.c index c036b58..57aff3d 100644 --- a/socket-handler.c +++ b/socket-handler.c
@@ -470,8 +470,7 @@ goto err_free; } - addrlen = sizeof(addr) - sizeof(addr.sun_path) + len; - + addrlen = offsetof(struct sockaddr_un, sun_path) + len; rc = bind(sh->sd, (struct sockaddr *)&addr, addrlen); if (rc) { socket_path_t name;
diff --git a/test/meson.build b/test/meson.build index 9101049..ff15b56 100644 --- a/test/meson.build +++ b/test/meson.build
@@ -103,3 +103,17 @@ include_directories: '..', ), ) + +test( + 'test-console-socket-path', + executable( + 'test-console-socket-path', + 'test-console-socket-path.c', + '../console-socket.c', + c_args: [ + '-DSYSCONFDIR=""', + '-DCONSOLE_SOCKET_PREFIX="obmc-console"', + ], + include_directories: '..', + ), +)
diff --git a/test/test-console-socket-path.c b/test/test-console-socket-path.c new file mode 100644 index 0000000..0e6d7dc --- /dev/null +++ b/test/test-console-socket-path.c
@@ -0,0 +1,93 @@ +#include <assert.h> +#include <errno.h> +#include <limits.h> +#include <stddef.h> +#include <stdio.h> +#include <stdlib.h> +#include <string.h> +#include <sys/socket.h> +#include <sys/un.h> + +#include "console-server.h" + +/* Helper to calculate expected addrlen */ +static ssize_t expected_pathlen(const char *id) +{ + size_t len = + 1ul + strlen(CONSOLE_SOCKET_PREFIX) + strlen(".") + strlen(id); + return len > SSIZE_MAX ? -1 : (ssize_t)len; +} + +/* Test that a valid id returns full addrlen */ +static void test_console_socket_path_returns_pathlen(void) +{ + socket_path_t sun_path; + const char *id = "test-console"; + ssize_t pathlen; + + pathlen = console_socket_path(sun_path, id); + + assert(pathlen > 0); + assert(pathlen == expected_pathlen(id)); +} + +/* Test that NULL id returns -1 and sets errno to EINVAL */ +static void test_console_socket_path_null_id(void) +{ + socket_path_t sun_path; + ssize_t len; + + errno = 0; + len = console_socket_path(sun_path, NULL); + + assert(len == -1); + assert(errno == EINVAL); +} + +/* Test that an id that exceeds buffer limits returns -1 and sets errno to 0 */ +static void test_console_socket_path_id_too_long(void) +{ + socket_path_t sun_path; + /* Create an id that will exceed buffer size */ + char long_id[sizeof(socket_path_t) + 1]; + ssize_t len; + + memset(long_id, 'a', sizeof(long_id) - 1); + long_id[sizeof(long_id) - 1] = '\0'; + + errno = 0; + len = console_socket_path(sun_path, long_id); + + assert(len == -1); + assert(errno == 0); +} + +/* Test that the socket path contains the expected prefix and id */ +static void test_console_socket_path_content(void) +{ + socket_path_t sun_path; + const char *id = "test-abstract"; + char expected[sizeof(socket_path_t)]; + ssize_t addrlen; + + addrlen = console_socket_path(sun_path, id); + assert(addrlen > 0); + + /* Build expected path (without NUL prefix) */ + snprintf(expected, sizeof(expected), CONSOLE_SOCKET_PREFIX ".%s", id); + + /* First byte should be NUL for abstract socket */ + assert(sun_path[0] == '\0'); + /* Compare path content after NUL prefix */ + assert(strcmp(sun_path + 1, expected) == 0); +} + +int main(void) +{ + test_console_socket_path_returns_pathlen(); + test_console_socket_path_null_id(); + test_console_socket_path_id_too_long(); + test_console_socket_path_content(); + + return EXIT_SUCCESS; +}