[RFC] util: split unix socket functions out of qemu-sockets

Since fccb744f41 (gdbstub: Try unlinking the unix socket before
binding) we use the unix_listen() function from linux-user which
causes complications when trying to build statically.

Fix this by splitting the unix functions into its own file and doing
the appropriate tweaks to the headers.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reported-by: Michael Tokarev <mjt@tls.msk.ru>
---
 include/qemu/sockets.h |   1 +
 util/socket-helpers.h  |  17 ++++
 util/qemu-sockets.c    | 199 +--------------------------------------
 util/unix-sockets.c    | 207 +++++++++++++++++++++++++++++++++++++++++
 util/meson.build       |   5 +-
 5 files changed, 231 insertions(+), 198 deletions(-)
 create mode 100644 util/socket-helpers.h
 create mode 100644 util/unix-sockets.c

On Tue, May 20, 2025 at 05:57:06PM +0100, Alex Bennée wrote:
> Since fccb744f41 (gdbstub: Try unlinking the unix socket before
> binding) we use the unix_listen() function from linux-user which
> causes complications when trying to build statically.
> 
> Fix this by splitting the unix functions into its own file and doing
> the appropriate tweaks to the headers.

I think I'd be more inclined to just revert the original problem
patch. The original stated problem only seems to need a single
'unlink' call being added in the gdb stub, avoiding the whole
problem with linking.  I'm not really a fan of arbitrarily
splitting the up the source files as a workaround.



> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Reported-by: Michael Tokarev <mjt@tls.msk.ru>
> ---
>  include/qemu/sockets.h |   1 +
>  util/socket-helpers.h  |  17 ++++
>  util/qemu-sockets.c    | 199 +--------------------------------------
>  util/unix-sockets.c    | 207 +++++++++++++++++++++++++++++++++++++++++
>  util/meson.build       |   5 +-
>  5 files changed, 231 insertions(+), 198 deletions(-)
>  create mode 100644 util/socket-helpers.h
>  create mode 100644 util/unix-sockets.c
> 
> diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h
> index c562690d89..578aef13cf 100644
> --- a/include/qemu/sockets.h
> +++ b/include/qemu/sockets.h
> @@ -65,6 +65,7 @@ int inet_connect_saddr(InetSocketAddress *saddr, Error **errp);
>  
>  NetworkAddressFamily inet_netfamily(int family);
>  
> +/* part of unix-sockets.c */
>  int unix_listen(const char *path, Error **errp);
>  int unix_connect(const char *path, Error **errp);
>  
> diff --git a/util/socket-helpers.h b/util/socket-helpers.h
> new file mode 100644
> index 0000000000..f72925148a
> --- /dev/null
> +++ b/util/socket-helpers.h
> @@ -0,0 +1,17 @@
> +/*
> + * Common helper functions for unix and qemu sockets
> + *
> + * (c) 2008 Gerd Hoffmann <kraxel@redhat.com>
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#ifndef _SOCKET_HELPERS_H_
> +#define _SOCKET_HELPERS_H_
> +
> +#include "qapi/qapi-visit-sockets.h"
> +
> +int unix_connect_saddr(UnixSocketAddress *saddr, Error **errp);
> +int unix_listen_saddr(UnixSocketAddress *saddr, int num, Error **errp);
> +
> +#endif /* _SOCKET_HELPERS_H_ */
> diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
> index 77477c1cd5..a5c3515082 100644
> --- a/util/qemu-sockets.c
> +++ b/util/qemu-sockets.c
> @@ -1,5 +1,5 @@
>  /*
> - *  inet and unix socket functions for qemu
> + *  inet socket functions for qemu
>   *
>   *  (c) 2008 Gerd Hoffmann <kraxel@redhat.com>
>   *
> @@ -30,6 +30,7 @@
>  #include "qapi/qobject-input-visitor.h"
>  #include "qapi/qobject-output-visitor.h"
>  #include "qemu/cutils.h"
> +#include "socket-helpers.h"
>  #include "trace.h"
>  
>  #ifndef AI_ADDRCONFIG
> @@ -853,202 +854,6 @@ static int vsock_parse(VsockSocketAddress *addr, const char *str,
>  }
>  #endif /* CONFIG_AF_VSOCK */
>  
> -static bool saddr_is_abstract(UnixSocketAddress *saddr)
> -{
> -#ifdef CONFIG_LINUX
> -    return saddr->abstract;
> -#else
> -    return false;
> -#endif
> -}
> -
> -static bool saddr_is_tight(UnixSocketAddress *saddr)
> -{
> -#ifdef CONFIG_LINUX
> -    return !saddr->has_tight || saddr->tight;
> -#else
> -    return false;
> -#endif
> -}
> -
> -static int unix_listen_saddr(UnixSocketAddress *saddr,
> -                             int num,
> -                             Error **errp)
> -{
> -    bool abstract = saddr_is_abstract(saddr);
> -    struct sockaddr_un un;
> -    int sock, fd;
> -    char *pathbuf = NULL;
> -    const char *path;
> -    size_t pathlen;
> -    size_t addrlen;
> -
> -    sock = qemu_socket(PF_UNIX, SOCK_STREAM, 0);
> -    if (sock < 0) {
> -        error_setg_errno(errp, errno, "Failed to create Unix socket");
> -        return -1;
> -    }
> -
> -    if (saddr->path[0] || abstract) {
> -        path = saddr->path;
> -    } else {
> -        path = pathbuf = g_strdup_printf("%s/qemu-socket-XXXXXX",
> -                                         g_get_tmp_dir());
> -    }
> -
> -    pathlen = strlen(path);
> -    if (pathlen > sizeof(un.sun_path) ||
> -        (abstract && pathlen > (sizeof(un.sun_path) - 1))) {
> -        error_setg(errp, "UNIX socket path '%s' is too long", path);
> -        error_append_hint(errp, "Path must be less than %zu bytes\n",
> -                          abstract ? sizeof(un.sun_path) - 1 :
> -                          sizeof(un.sun_path));
> -        goto err;
> -    }
> -
> -    if (pathbuf != NULL) {
> -        /*
> -         * This dummy fd usage silences the mktemp() insecure warning.
> -         * Using mkstemp() doesn't make things more secure here
> -         * though.  bind() complains about existing files, so we have
> -         * to unlink first and thus re-open the race window.  The
> -         * worst case possible is bind() failing, i.e. a DoS attack.
> -         */
> -        fd = mkstemp(pathbuf);
> -        if (fd < 0) {
> -            error_setg_errno(errp, errno,
> -                             "Failed to make a temporary socket %s", pathbuf);
> -            goto err;
> -        }
> -        close(fd);
> -    }
> -
> -    if (!abstract && unlink(path) < 0 && errno != ENOENT) {
> -        error_setg_errno(errp, errno,
> -                         "Failed to unlink socket %s", path);
> -        goto err;
> -    }
> -
> -    memset(&un, 0, sizeof(un));
> -    un.sun_family = AF_UNIX;
> -    addrlen = sizeof(un);
> -
> -    if (abstract) {
> -        un.sun_path[0] = '\0';
> -        memcpy(&un.sun_path[1], path, pathlen);
> -        if (saddr_is_tight(saddr)) {
> -            addrlen = offsetof(struct sockaddr_un, sun_path) + 1 + pathlen;
> -        }
> -    } else {
> -        memcpy(un.sun_path, path, pathlen);
> -    }
> -
> -    if (bind(sock, (struct sockaddr *) &un, addrlen) < 0) {
> -        error_setg_errno(errp, errno, "Failed to bind socket to %s", path);
> -        goto err;
> -    }
> -    if (listen(sock, num) < 0) {
> -        error_setg_errno(errp, errno, "Failed to listen on socket");
> -        goto err;
> -    }
> -
> -    g_free(pathbuf);
> -    return sock;
> -
> -err:
> -    g_free(pathbuf);
> -    close(sock);
> -    return -1;
> -}
> -
> -static int unix_connect_saddr(UnixSocketAddress *saddr, Error **errp)
> -{
> -    bool abstract = saddr_is_abstract(saddr);
> -    struct sockaddr_un un;
> -    int sock, rc;
> -    size_t pathlen;
> -    size_t addrlen;
> -
> -    if (saddr->path == NULL) {
> -        error_setg(errp, "unix connect: no path specified");
> -        return -1;
> -    }
> -
> -    sock = qemu_socket(PF_UNIX, SOCK_STREAM, 0);
> -    if (sock < 0) {
> -        error_setg_errno(errp, errno, "Failed to create socket");
> -        return -1;
> -    }
> -
> -    pathlen = strlen(saddr->path);
> -    if (pathlen > sizeof(un.sun_path) ||
> -        (abstract && pathlen > (sizeof(un.sun_path) - 1))) {
> -        error_setg(errp, "UNIX socket path '%s' is too long", saddr->path);
> -        error_append_hint(errp, "Path must be less than %zu bytes\n",
> -                          abstract ? sizeof(un.sun_path) - 1 :
> -                          sizeof(un.sun_path));
> -        goto err;
> -    }
> -
> -    memset(&un, 0, sizeof(un));
> -    un.sun_family = AF_UNIX;
> -    addrlen = sizeof(un);
> -
> -    if (abstract) {
> -        un.sun_path[0] = '\0';
> -        memcpy(&un.sun_path[1], saddr->path, pathlen);
> -        if (saddr_is_tight(saddr)) {
> -            addrlen = offsetof(struct sockaddr_un, sun_path) + 1 + pathlen;
> -        }
> -    } else {
> -        memcpy(un.sun_path, saddr->path, pathlen);
> -    }
> -    /* connect to peer */
> -    do {
> -        rc = 0;
> -        if (connect(sock, (struct sockaddr *) &un, addrlen) < 0) {
> -            rc = -errno;
> -        }
> -    } while (rc == -EINTR);
> -
> -    if (rc < 0) {
> -        error_setg_errno(errp, -rc, "Failed to connect to '%s'",
> -                         saddr->path);
> -        goto err;
> -    }
> -
> -    return sock;
> -
> - err:
> -    close(sock);
> -    return -1;
> -}
> -
> -/* compatibility wrapper */
> -int unix_listen(const char *str, Error **errp)
> -{
> -    UnixSocketAddress *saddr;
> -    int sock;
> -
> -    saddr = g_new0(UnixSocketAddress, 1);
> -    saddr->path = g_strdup(str);
> -    sock = unix_listen_saddr(saddr, 1, errp);
> -    qapi_free_UnixSocketAddress(saddr);
> -    return sock;
> -}
> -
> -int unix_connect(const char *path, Error **errp)
> -{
> -    UnixSocketAddress *saddr;
> -    int sock;
> -
> -    saddr = g_new0(UnixSocketAddress, 1);
> -    saddr->path = g_strdup(path);
> -    sock = unix_connect_saddr(saddr, errp);
> -    qapi_free_UnixSocketAddress(saddr);
> -    return sock;
> -}
> -
>  char *socket_uri(SocketAddress *addr)
>  {
>      switch (addr->type) {
> diff --git a/util/unix-sockets.c b/util/unix-sockets.c
> new file mode 100644
> index 0000000000..e271dd3e09
> --- /dev/null
> +++ b/util/unix-sockets.c
> @@ -0,0 +1,207 @@
> +/*
> + * unix socket functions for qemu
> + *
> + * (c) 2008 Gerd Hoffmann <kraxel@redhat.com>
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu/sockets.h"
> +#include "qapi/error.h"
> +
> +#include "socket-helpers.h"
> +
> +static bool saddr_is_abstract(UnixSocketAddress *saddr)
> +{
> +#ifdef CONFIG_LINUX
> +    return saddr->abstract;
> +#else
> +    return false;
> +#endif
> +}
> +
> +static bool saddr_is_tight(UnixSocketAddress *saddr)
> +{
> +#ifdef CONFIG_LINUX
> +    return !saddr->has_tight || saddr->tight;
> +#else
> +    return false;
> +#endif
> +}
> +
> +int unix_listen_saddr(UnixSocketAddress *saddr, int num, Error **errp)
> +{
> +    bool abstract = saddr_is_abstract(saddr);
> +    struct sockaddr_un un;
> +    int sock, fd;
> +    char *pathbuf = NULL;
> +    const char *path;
> +    size_t pathlen;
> +    size_t addrlen;
> +
> +    sock = qemu_socket(PF_UNIX, SOCK_STREAM, 0);
> +    if (sock < 0) {
> +        error_setg_errno(errp, errno, "Failed to create Unix socket");
> +        return -1;
> +    }
> +
> +    if (saddr->path[0] || abstract) {
> +        path = saddr->path;
> +    } else {
> +        path = pathbuf = g_strdup_printf("%s/qemu-socket-XXXXXX",
> +                                         g_get_tmp_dir());
> +    }
> +
> +    pathlen = strlen(path);
> +    if (pathlen > sizeof(un.sun_path) ||
> +        (abstract && pathlen > (sizeof(un.sun_path) - 1))) {
> +        error_setg(errp, "UNIX socket path '%s' is too long", path);
> +        error_append_hint(errp, "Path must be less than %zu bytes\n",
> +                          abstract ? sizeof(un.sun_path) - 1 :
> +                          sizeof(un.sun_path));
> +        goto err;
> +    }
> +
> +    if (pathbuf != NULL) {
> +        /*
> +         * This dummy fd usage silences the mktemp() insecure warning.
> +         * Using mkstemp() doesn't make things more secure here
> +         * though.  bind() complains about existing files, so we have
> +         * to unlink first and thus re-open the race window.  The
> +         * worst case possible is bind() failing, i.e. a DoS attack.
> +         */
> +        fd = mkstemp(pathbuf);
> +        if (fd < 0) {
> +            error_setg_errno(errp, errno,
> +                             "Failed to make a temporary socket %s", pathbuf);
> +            goto err;
> +        }
> +        close(fd);
> +    }
> +
> +    if (!abstract && unlink(path) < 0 && errno != ENOENT) {
> +        error_setg_errno(errp, errno,
> +                         "Failed to unlink socket %s", path);
> +        goto err;
> +    }
> +
> +    memset(&un, 0, sizeof(un));
> +    un.sun_family = AF_UNIX;
> +    addrlen = sizeof(un);
> +
> +    if (abstract) {
> +        un.sun_path[0] = '\0';
> +        memcpy(&un.sun_path[1], path, pathlen);
> +        if (saddr_is_tight(saddr)) {
> +            addrlen = offsetof(struct sockaddr_un, sun_path) + 1 + pathlen;
> +        }
> +    } else {
> +        memcpy(un.sun_path, path, pathlen);
> +    }
> +
> +    if (bind(sock, (struct sockaddr *) &un, addrlen) < 0) {
> +        error_setg_errno(errp, errno, "Failed to bind socket to %s", path);
> +        goto err;
> +    }
> +    if (listen(sock, num) < 0) {
> +        error_setg_errno(errp, errno, "Failed to listen on socket");
> +        goto err;
> +    }
> +
> +    g_free(pathbuf);
> +    return sock;
> +
> +err:
> +    g_free(pathbuf);
> +    close(sock);
> +    return -1;
> +}
> +
> +int unix_connect_saddr(UnixSocketAddress *saddr, Error **errp)
> +{
> +    bool abstract = saddr_is_abstract(saddr);
> +    struct sockaddr_un un;
> +    int sock, rc;
> +    size_t pathlen;
> +    size_t addrlen;
> +
> +    if (saddr->path == NULL) {
> +        error_setg(errp, "unix connect: no path specified");
> +        return -1;
> +    }
> +
> +    sock = qemu_socket(PF_UNIX, SOCK_STREAM, 0);
> +    if (sock < 0) {
> +        error_setg_errno(errp, errno, "Failed to create socket");
> +        return -1;
> +    }
> +
> +    pathlen = strlen(saddr->path);
> +    if (pathlen > sizeof(un.sun_path) ||
> +        (abstract && pathlen > (sizeof(un.sun_path) - 1))) {
> +        error_setg(errp, "UNIX socket path '%s' is too long", saddr->path);
> +        error_append_hint(errp, "Path must be less than %zu bytes\n",
> +                          abstract ? sizeof(un.sun_path) - 1 :
> +                          sizeof(un.sun_path));
> +        goto err;
> +    }
> +
> +    memset(&un, 0, sizeof(un));
> +    un.sun_family = AF_UNIX;
> +    addrlen = sizeof(un);
> +
> +    if (abstract) {
> +        un.sun_path[0] = '\0';
> +        memcpy(&un.sun_path[1], saddr->path, pathlen);
> +        if (saddr_is_tight(saddr)) {
> +            addrlen = offsetof(struct sockaddr_un, sun_path) + 1 + pathlen;
> +        }
> +    } else {
> +        memcpy(un.sun_path, saddr->path, pathlen);
> +    }
> +    /* connect to peer */
> +    do {
> +        rc = 0;
> +        if (connect(sock, (struct sockaddr *) &un, addrlen) < 0) {
> +            rc = -errno;
> +        }
> +    } while (rc == -EINTR);
> +
> +    if (rc < 0) {
> +        error_setg_errno(errp, -rc, "Failed to connect to '%s'",
> +                         saddr->path);
> +        goto err;
> +    }
> +
> +    return sock;
> +
> + err:
> +    close(sock);
> +    return -1;
> +}
> +
> +/* compatibility wrapper */
> +int unix_listen(const char *str, Error **errp)
> +{
> +    UnixSocketAddress *saddr;
> +    int sock;
> +
> +    saddr = g_new0(UnixSocketAddress, 1);
> +    saddr->path = g_strdup(str);
> +    sock = unix_listen_saddr(saddr, 1, errp);
> +    qapi_free_UnixSocketAddress(saddr);
> +    return sock;
> +}
> +
> +int unix_connect(const char *path, Error **errp)
> +{
> +    UnixSocketAddress *saddr;
> +    int sock;
> +
> +    saddr = g_new0(UnixSocketAddress, 1);
> +    saddr->path = g_strdup(path);
> +    sock = unix_connect_saddr(saddr, errp);
> +    qapi_free_UnixSocketAddress(saddr);
> +    return sock;
> +}
> diff --git a/util/meson.build b/util/meson.build
> index 1adff96ebd..dbcfbc6542 100644
> --- a/util/meson.build
> +++ b/util/meson.build
> @@ -87,7 +87,10 @@ if have_block or have_ga
>    util_ss.add(files(f'coroutine-@coroutine_backend@.c'))
>    util_ss.add(files('thread-pool.c', 'qemu-timer.c'))
>  endif
> -if have_block or have_ga or have_user
> +if have_user
> +  util_ss.add(files('unix-sockets.c'))
> +endif
> +if have_block or have_ga
>    util_ss.add(files('qemu-sockets.c'))
>  endif
>  if have_block
> -- 
> 2.39.5
> 

With regards,
Daniel

Thanks for looking into this!

On 2025-05-20 17:57, Alex Bennée wrote:
> Since fccb744f41 (gdbstub: Try unlinking the unix socket before
> binding) we use the unix_listen() function from linux-user which
> causes complications when trying to build statically.

Should this be in Fixes: tag?

> Fix this by splitting the unix functions into its own file and doing
> the appropriate tweaks to the headers.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Reported-by: Michael Tokarev <mjt@tls.msk.ru>
> ---
>  include/qemu/sockets.h |   1 +
>  util/socket-helpers.h  |  17 ++++
>  util/qemu-sockets.c    | 199 +--------------------------------------
>  util/unix-sockets.c    | 207 +++++++++++++++++++++++++++++++++++++++++
>  util/meson.build       |   5 +-
>  5 files changed, 231 insertions(+), 198 deletions(-)
>  create mode 100644 util/socket-helpers.h
>  create mode 100644 util/unix-sockets.c

[...]

> --- a/util/qemu-sockets.c
> +++ b/util/qemu-sockets.c
> @@ -1,5 +1,5 @@
>  /*
> - *  inet and unix socket functions for qemu
> + *  inet socket functions for qemu
>   *
>   *  (c) 2008 Gerd Hoffmann <kraxel@redhat.com>
>   *

Should we mention that static linking of this file is not supported?

With or without this:

Acked-by: Ilya Leoshkevich <iii@linux.ibm.com>