* [RFC PATCH] util: split unix socket functions out of qemu-sockets
@ 2025-05-20 16:57 Alex Bennée
2025-05-21 10:15 ` Daniel P. Berrangé
2025-05-22 14:36 ` Ilya Leoshkevich
0 siblings, 2 replies; 3+ messages in thread
From: Alex Bennée @ 2025-05-20 16:57 UTC (permalink / raw)
To: qemu-devel; +Cc: Alex Bennée, Michael Tokarev, Daniel P. Berrangé
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
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
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [RFC PATCH] util: split unix socket functions out of qemu-sockets
2025-05-20 16:57 [RFC PATCH] util: split unix socket functions out of qemu-sockets Alex Bennée
@ 2025-05-21 10:15 ` Daniel P. Berrangé
2025-05-22 14:36 ` Ilya Leoshkevich
1 sibling, 0 replies; 3+ messages in thread
From: Daniel P. Berrangé @ 2025-05-21 10:15 UTC (permalink / raw)
To: Alex Bennée; +Cc: qemu-devel, Michael Tokarev
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
--
|: 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 :|
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [RFC PATCH] util: split unix socket functions out of qemu-sockets
2025-05-20 16:57 [RFC PATCH] util: split unix socket functions out of qemu-sockets Alex Bennée
2025-05-21 10:15 ` Daniel P. Berrangé
@ 2025-05-22 14:36 ` Ilya Leoshkevich
1 sibling, 0 replies; 3+ messages in thread
From: Ilya Leoshkevich @ 2025-05-22 14:36 UTC (permalink / raw)
To: Alex Bennée; +Cc: qemu-devel, Michael Tokarev, Daniel P. Berrangé
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>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-05-22 14:37 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-20 16:57 [RFC PATCH] util: split unix socket functions out of qemu-sockets Alex Bennée
2025-05-21 10:15 ` Daniel P. Berrangé
2025-05-22 14:36 ` Ilya Leoshkevich
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).