Re: [PATCH v4 04/11] osdep: implement qemu_socketpair() for win32

["Daniel P. Berrangé" <berrange@redhat.com>]

On Mon, Mar 06, 2023 at 04:27:44PM +0400, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> Manually implement a socketpair() function, using UNIX sockets and
> simple peer credential checking.
> 
> QEMU doesn't make much use of socketpair, beside vhost-user which is not
> available for win32 at this point. However, I intend to use it for
> writing some new portable tests.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  include/qemu/sockets.h |   2 -
>  util/oslib-win32.c     | 110 +++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 110 insertions(+), 2 deletions(-)
> 
> diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h
> index 2b0698a7c9..d935fd80da 100644
> --- a/include/qemu/sockets.h
> +++ b/include/qemu/sockets.h
> @@ -15,7 +15,6 @@ int inet_aton(const char *cp, struct in_addr *ia);
>  bool fd_is_socket(int fd);
>  int qemu_socket(int domain, int type, int protocol);
>  
> -#ifndef WIN32
>  /**
>   * qemu_socketpair:
>   * @domain: specifies a communication domain, such as PF_UNIX
> @@ -30,7 +29,6 @@ int qemu_socket(int domain, int type, int protocol);
>   * Return 0 on success.
>   */
>  int qemu_socketpair(int domain, int type, int protocol, int sv[2]);
> -#endif
>  
>  int qemu_accept(int s, struct sockaddr *addr, socklen_t *addrlen);
>  /*
> diff --git a/util/oslib-win32.c b/util/oslib-win32.c
> index 29a667ae3d..16f8a67f7e 100644
> --- a/util/oslib-win32.c
> +++ b/util/oslib-win32.c
> @@ -310,6 +310,116 @@ bool qemu_socket_unselect(int sockfd, Error **errp)
>      return qemu_socket_select(sockfd, NULL, 0, errp);
>  }
>  
> +int qemu_socketpair(int domain, int type, int protocol, int sv[2])
> +{
> +    struct sockaddr_un addr = {
> +        0,
> +    };
> +    socklen_t socklen;
> +    int listener = -1;
> +    int client = -1;
> +    int server = -1;
> +    g_autofree char *path = NULL;
> +    int tmpfd;
> +    u_long arg;
> +    int ret = -1;
> +
> +    g_return_val_if_fail(sv != NULL, -1);
> +
> +    addr.sun_family = AF_UNIX;
> +    socklen = sizeof(addr);
> +
> +    tmpfd = g_file_open_tmp(NULL, &path, NULL);
> +    if (tmpfd == -1 || !path) {
> +        errno = EACCES;
> +        goto out;
> +    }
> +
> +    close(tmpfd);
> +
> +    if (strlen(path) >= sizeof(addr.sun_path)) {
> +        errno = EINVAL;
> +        goto out;
> +    }
> +
> +    strncpy(addr.sun_path, path, sizeof(addr.sun_path) - 1);
> +
> +    listener = socket(domain, type, protocol);
> +    if (listener == -1) {
> +        goto out;
> +    }
> +
> +    if (DeleteFile(path) == 0 && GetLastError() != ERROR_FILE_NOT_FOUND) {
> +        errno = EACCES;
> +        goto out;
> +    }
> +    g_clear_pointer(&path, g_free);
> +
> +    if (bind(listener, (struct sockaddr *)&addr, socklen) == -1) {
> +        goto out;
> +    }
> +
> +    if (listen(listener, 1) == -1) {
> +        goto out;
> +    }
> +
> +    client = socket(domain, type, protocol);
> +    if (client == -1) {
> +        goto out;
> +    }
> +
> +    arg = 1;
> +    if (ioctlsocket(client, FIONBIO, &arg) != NO_ERROR) {
> +        goto out;
> +    }
> +
> +    if (connect(client, (struct sockaddr *)&addr, socklen) == -1 &&
> +        WSAGetLastError() != WSAEWOULDBLOCK) {
> +        goto out;
> +    }
> +
> +    server = accept(listener, NULL, NULL);
> +    if (server == -1) {
> +        goto out;
> +    }

In theory at this point 'client' if connect() returned WSAEWOULDBLOCK,
then at this point it should be fully connected. I wonder if that is
actually guaranteed though, or should we do something to validate
there's no race condition ?

> +
> +    arg = 0;
> +    if (ioctlsocket(client, FIONBIO, &arg) != NO_ERROR) {
> +        goto out;
> +    }
> +
> +    arg = 0;
> +    if (ioctlsocket(client, SIO_AF_UNIX_GETPEERPID, &arg) != NO_ERROR) {
> +        goto out;
> +    }

Maybe this will force a synchronization point ?

Alteratively select() + getsockopt(SO_ERROR) is what we used to
do to check for connect() completion (logic removed now but can
be seen in b2587932582333197c88bf663785b19f441989d7)



> +
> +    if (arg != GetCurrentProcessId()) {
> +        errno = EPERM;
> +        goto out;
> +    }
> +
> +    sv[0] = server;
> +    server = -1;
> +    sv[1] = client;
> +    client = -1;
> +    ret = 0;
> +
> +out:
> +    if (listener != -1) {
> +        close(listener);
> +    }
> +    if (client != -1) {
> +        close(client);
> +    }
> +    if (server != -1) {
> +        close(server);
> +    }
> +    if (path) {
> +        DeleteFile(path);
> +    }
> +    return ret;
> +}
> +
>  #undef connect
>  int qemu_connect_wrap(int sockfd, const struct sockaddr *addr,
>                        socklen_t addrlen)
> -- 
> 2.39.2
> 

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 :|