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.ben...@linaro.org>
> Reported-by: Michael Tokarev <m...@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 <kra...@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 <kra...@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 <kra...@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
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
