[PATCH v66 0/6] MR2786: Add support for AF_UNIX sockets
Windows 10 [received support](https://devblogs.microsoft.com/commandline/af_unix-comes-to-windows/) for AF_UNIX sockets in Insider Build 17063. This merge request adds basic support for AF_UNIX sockets to ws2_32 and wineserver. Of particular note is the difficulty in handling `sun_path`. Most of the functions that allow for translating Windows paths to Unix paths are not accessible from ws2_32. I considered the following options: * Pass the Windows path to wineserver and do the conversion there. * This is, as far as I can tell, not possible without major rearchitecting. wineserver does not have functions to translate Windows paths to Unix paths, for obvious reasons. * Obtain the current working directory of the requesting process and temporarily change directories to there. * This only handles relative paths and fails for absolute paths, UNC paths, etc. * Conditionally change directories based on whether the path is relative or not. * This is error-prone and wineserver does not have the requisite functions to do this cleanly. I ultimately decided to pass the translated Unix path to wineserver, which changes directories to `dirname(path)`. It then provides `bind` and `connect` with `basename(path)`. This is not threadsafe, but wineserver is not (currently) multithreaded. Abstract sockets are not fully supported by this patch, matching the behavior of Windows. -- v66: ws2_32/tests: Add test for AF_UNIX sockets. server: Fix getsockname() and accept() on AF_UNIX sockets. server: Introduce error when attempting to create a SOCK_DGRAM AF_UNIX socket. server: Allow for deletion of socket files. ws2_32: Add support for AF_UNIX sockets. ws2_32: Add afunix.h header. https://gitlab.winehq.org/wine/wine/-/merge_requests/2786
From: Ally Sommers <dropbear.sh(a)gmail.com> This header is needed for support of AF_UNIX sockets. --- include/Makefile.in | 1 + include/afunix.h | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+) create mode 100644 include/afunix.h diff --git a/include/Makefile.in b/include/Makefile.in index 40a71d45a1f..8b0187a6467 100644 --- a/include/Makefile.in +++ b/include/Makefile.in @@ -12,6 +12,7 @@ SOURCES = \ adshlp.h \ advpub.h \ af_irda.h \ + afunix.h \ amaudio.h \ amsi.idl \ amstream.idl \ diff --git a/include/afunix.h b/include/afunix.h new file mode 100644 index 00000000000..8118dda271b --- /dev/null +++ b/include/afunix.h @@ -0,0 +1,38 @@ +/* + * Copyright 2023 Ally Sommers + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA + */ + +#ifndef _WS2AFUNIX_ +#define _WS2AFUNIX_ + +#include "windef.h" +#include "ws2def.h" + +#ifdef USE_WS_PREFIX +# define WS(x) WS_##x +#else +# define WS(x) x +#endif + +#define UNIX_PATH_MAX 108 + +typedef struct WS(sockaddr_un) { + USHORT sun_family; + char sun_path[UNIX_PATH_MAX]; +} SOCKADDR_UN, *PSOCKADDR_UN; + +#endif /* _WS2AFUNIX_ */ \ No newline at end of file -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2786
From: Ally Sommers <dropbear.sh(a)gmail.com> This commit additionally modifies wineserver's sock_ioctl to handle the provided pathname by changing directories and then returning after the native call. This is NOT threadsafe, but wineserver is not multithreaded. --- dlls/ntdll/unix/socket.c | 4 + dlls/ws2_32/socket.c | 83 +++++++++++++++++-- dlls/ws2_32/ws2_32_private.h | 13 +++ server/sock.c | 152 ++++++++++++++++++++++++++++++++--- 4 files changed, 238 insertions(+), 14 deletions(-) diff --git a/dlls/ntdll/unix/socket.c b/dlls/ntdll/unix/socket.c index c1eba723307..343da71ca05 100644 --- a/dlls/ntdll/unix/socket.c +++ b/dlls/ntdll/unix/socket.c @@ -67,6 +67,8 @@ # define HAS_IRDA #endif +#include <sys/un.h> + #include "ntstatus.h" #define WIN32_NO_STATUS #include "windef.h" @@ -78,6 +80,7 @@ #include "ws2tcpip.h" #include "wsipx.h" #include "af_irda.h" +#include "afunix.h" #include "wine/afd.h" #include "unix_private.h" @@ -106,6 +109,7 @@ union unix_sockaddr #ifdef HAS_IRDA struct sockaddr_irda irda; #endif + struct sockaddr_un un; }; struct async_recv_ioctl diff --git a/dlls/ws2_32/socket.c b/dlls/ws2_32/socket.c index 9da3ccb285f..998101cc65a 100644 --- a/dlls/ws2_32/socket.c +++ b/dlls/ws2_32/socket.c @@ -167,6 +167,19 @@ static const WSAPROTOCOL_INFOW supported_protocols[] = .dwMessageSize = UINT_MAX, .szProtocol = L"SPX II", }, + { + .dwServiceFlags1 = XP1_GUARANTEED_DELIVERY | XP1_GUARANTEED_ORDER | XP1_IFS_HANDLES, + .dwProviderFlags = PFL_MATCHES_PROTOCOL_ZERO, + .ProviderId = {0xa00943d9, 0x9c2e, 0x4633, {0x9b, 0x59, 0x00, 0x57, 0xa3, 0x16, 0x09, 0x94}}, + .dwCatalogEntryId = 1007, + .ProtocolChain.ChainLen = 1, + .iVersion = 2, + .iAddressFamily = AF_UNIX, + .iMaxSockAddr = sizeof(struct sockaddr_un), + .iMinSockAddr = offsetof(struct sockaddr_un, sun_path), + .iSocketType = SOCK_STREAM, + .szProtocol = L"AF_UNIX", + }, }; DECLARE_CRITICAL_SECTION(cs_socket_list); @@ -226,6 +239,11 @@ const char *debugstr_sockaddr( const struct sockaddr *a ) addr, ((const SOCKADDR_IRDA *)a)->irdaServiceName); } + case AF_UNIX: + { + return wine_dbg_sprintf("{ family AF_UNIX, path %s }", + ((const SOCKADDR_UN *)a)->sun_path); + } default: return wine_dbg_sprintf("{ family %d }", a->sa_family); } @@ -1106,6 +1124,10 @@ int WINAPI bind( SOCKET s, const struct sockaddr *addr, int len ) HANDLE sync_event; NTSTATUS status; + const int bind_len = len; + char *unix_path = NULL; + int unix_varargs_size = 0; + TRACE( "socket %#Ix, addr %s\n", s, debugstr_sockaddr(addr) ); if (!addr) @@ -1148,6 +1170,14 @@ int WINAPI bind( SOCKET s, const struct sockaddr *addr, int len ) } break; + case AF_UNIX: + if (len < offsetof(struct sockaddr_un, sun_path)) + { + SetLastError( WSAEFAULT ); + return -1; + } + break; + default: FIXME( "unknown protocol %u\n", addr->sa_family ); SetLastError( WSAEAFNOSUPPORT ); @@ -1156,7 +1186,29 @@ int WINAPI bind( SOCKET s, const struct sockaddr *addr, int len ) if (!(sync_event = get_sync_event())) return -1; - params = malloc( sizeof(int) + len ); + if (addr->sa_family == AF_UNIX && *addr->sa_data) + { + struct sockaddr_un sun = { 0 }; + WCHAR *sun_pathW; + memcpy(&sun, addr, len); + if (strlen( sun.sun_path )) + { + sun_pathW = strdupAtoW( sun.sun_path ); + unix_path = wine_get_unix_file_name( sun_pathW ); + free( sun_pathW ); + if (!unix_path) + return SOCKET_ERROR; + } + else + { + unix_path = malloc(1); + *unix_path = '\0'; + } + len = sizeof(sun); + unix_varargs_size = strlen( unix_path ); + } + + params = malloc( sizeof(int) + len + unix_varargs_size ); ret_addr = malloc( len ); if (!params || !ret_addr) { @@ -1166,10 +1218,14 @@ int WINAPI bind( SOCKET s, const struct sockaddr *addr, int len ) return -1; } params->unknown = 0; - memcpy( ¶ms->addr, addr, len ); + if (addr->sa_family == AF_UNIX) + memset( ¶ms->addr, 0, len ); + memcpy( ¶ms->addr, addr, bind_len ); + if (unix_path) + memcpy( (char *)¶ms->addr + len, unix_path, unix_varargs_size ); status = NtDeviceIoControlFile( (HANDLE)s, sync_event, NULL, NULL, &io, IOCTL_AFD_BIND, - params, sizeof(int) + len, ret_addr, len ); + params, sizeof(int) + len + unix_varargs_size, ret_addr, len ); if (status == STATUS_PENDING) { if (WaitForSingleObject( sync_event, INFINITE ) == WAIT_FAILED) @@ -1182,6 +1238,7 @@ int WINAPI bind( SOCKET s, const struct sockaddr *addr, int len ) free( params ); free( ret_addr ); + free( unix_path ); SetLastError( NtStatusToWSAError( status ) ); return status ? -1 : 0; @@ -1222,11 +1279,24 @@ int WINAPI connect( SOCKET s, const struct sockaddr *addr, int len ) HANDLE sync_event; NTSTATUS status; + char *unix_path = NULL; + int unix_varargs_size = 0; + TRACE( "socket %#Ix, addr %s, len %d\n", s, debugstr_sockaddr(addr), len ); if (!(sync_event = get_sync_event())) return -1; - if (!(params = malloc( sizeof(*params) + len ))) + if (addr->sa_family == AF_UNIX && *addr->sa_data) + { + WCHAR *sun_pathW = strdupAtoW(addr->sa_data); + unix_path = wine_get_unix_file_name(sun_pathW); + free(sun_pathW); + if (!unix_path) + return SOCKET_ERROR; + unix_varargs_size = strlen(unix_path); + } + + if (!(params = malloc( sizeof(*params) + len + unix_varargs_size ))) { SetLastError( ERROR_NOT_ENOUGH_MEMORY ); return -1; @@ -1234,10 +1304,13 @@ int WINAPI connect( SOCKET s, const struct sockaddr *addr, int len ) params->addr_len = len; params->synchronous = TRUE; memcpy( params + 1, addr, len ); + if (unix_path) + memcpy( (char *)(params + 1) + len, unix_path, unix_varargs_size ); status = NtDeviceIoControlFile( (HANDLE)s, sync_event, NULL, NULL, &io, IOCTL_AFD_WINE_CONNECT, - params, sizeof(*params) + len, NULL, 0 ); + params, sizeof(*params) + len + unix_varargs_size, NULL, 0 ); free( params ); + free( unix_path ); if (status == STATUS_PENDING) { if (wait_event_alertable( sync_event ) == WAIT_FAILED) return -1; diff --git a/dlls/ws2_32/ws2_32_private.h b/dlls/ws2_32/ws2_32_private.h index a009a1eefd3..bef532a6ff9 100644 --- a/dlls/ws2_32/ws2_32_private.h +++ b/dlls/ws2_32/ws2_32_private.h @@ -44,6 +44,7 @@ #include "mstcpip.h" #include "af_irda.h" #include "winnt.h" +#include "afunix.h" #define USE_WC_PREFIX /* For CMSG_DATA */ #include "iphlpapi.h" #include "ip2string.h" @@ -71,6 +72,18 @@ static inline char *strdupWtoA( const WCHAR *str ) return ret; } +static inline WCHAR *strdupAtoW( const char *str ) +{ + WCHAR *ret = NULL; + if (str) + { + DWORD len = MultiByteToWideChar(CP_ACP, 0, str, -1, NULL, 0); + if ((ret = malloc( len * sizeof(WCHAR) ))) + MultiByteToWideChar(CP_ACP, 0, str, -1, ret, len); + } + return ret; +} + static const char magic_loopback_addr[] = {127, 12, 34, 56}; const char *debugstr_sockaddr( const struct sockaddr *addr ) DECLSPEC_HIDDEN; diff --git a/server/sock.c b/server/sock.c index f774cded733..9f1c2cf2d38 100644 --- a/server/sock.c +++ b/server/sock.c @@ -84,6 +84,8 @@ # define HAS_IRDA #endif +#include <sys/un.h> + #include "ntstatus.h" #define WIN32_NO_STATUS #include "windef.h" @@ -94,6 +96,7 @@ #include "ws2tcpip.h" #include "wsipx.h" #include "af_irda.h" +#include "afunix.h" #include "wine/afd.h" #include "wine/rbtree.h" @@ -117,6 +120,7 @@ union win_sockaddr struct WS_sockaddr_in6 in6; struct WS_sockaddr_ipx ipx; SOCKADDR_IRDA irda; + struct WS_sockaddr_un un; }; union unix_sockaddr @@ -130,6 +134,7 @@ union unix_sockaddr #ifdef HAS_IRDA struct sockaddr_irda irda; #endif + struct sockaddr_un un; }; static struct list poll_list = LIST_INIT( poll_list ); @@ -677,6 +682,9 @@ static socklen_t get_unix_sockaddr_any( union unix_sockaddr *uaddr, int ws_famil uaddr->irda.sir_family = AF_IRDA; return sizeof(uaddr->irda); #endif + case WS_AF_UNIX: + uaddr->un.sun_family = AF_UNIX; + return sizeof(uaddr->un); default: return 0; } @@ -1767,6 +1775,7 @@ static int get_unix_family( int family ) #ifdef AF_IRDA case WS_AF_IRDA: return AF_IRDA; #endif + case WS_AF_UNIX: return AF_UNIX; case WS_AF_UNSPEC: return AF_UNSPEC; default: return -1; } @@ -2563,8 +2572,13 @@ static void sock_ioctl( struct fd *fd, ioctl_code_t code, struct async *async ) if (listen( unix_fd, params->backlog ) < 0) { - set_error( sock_get_ntstatus( errno ) ); - return; + /* Due to the way we handle the Windows AF_UNIX bind edge case, we also need to + * ignore listen's error. */ + if (!(errno == EINVAL && sock->family == WS_AF_UNIX && !*sock->addr.un.sun_path)) + { + set_error( sock_get_ntstatus( errno ) ); + return; + } } sock->state = SOCK_LISTENING; @@ -2634,7 +2648,55 @@ static void sock_ioctl( struct fd *fd, ioctl_code_t code, struct async *async ) break; } - unix_len = sockaddr_to_unix( addr, params->addr_len, &unix_addr ); + if (sock->family == WS_AF_UNIX) + { + if (*addr->sa_data) + { + int unix_path_len = get_req_data_size() - sizeof(*params) - params->addr_len; + char *unix_path; + char *base_name; + + if (!(unix_path = mem_alloc( unix_path_len + 1 ))) + return; + + memcpy( unix_path, (char *)(params + 1) + params->addr_len, unix_path_len ); + unix_path[unix_path_len] = '\0'; + + base_name = strrchr(unix_path, '/'); + if (base_name) + { + if (base_name != unix_path) + (++base_name)[-1] = '\0'; + } + else + base_name = unix_path; + + if (chdir( unix_path ) == -1) + { + set_error( sock_get_ntstatus( errno ) ); + free( unix_path ); + return; + } + + send_len -= unix_path_len; + unix_len = sizeof(unix_addr.un); + memset( &unix_addr.un, 0, sizeof(unix_addr.un) ); + unix_addr.un.sun_family = AF_UNIX; + memcpy( unix_addr.un.sun_path, base_name, strlen( base_name ) ); + free( unix_path ); + } + else + { + /* Contrary to documentation, Windows does not currently support abstract Unix + * sockets. connect() throws WSAEINVAL if sun_family is AF_UNIX and sun_path + * begins with '\0', even though bind() will succeed. */ + set_win32_error( WSAEINVAL ); + return; + } + } + else + unix_len = sockaddr_to_unix( addr, params->addr_len, &unix_addr ); + if (!unix_len) { set_error( STATUS_INVALID_ADDRESS ); @@ -2656,6 +2718,9 @@ static void sock_ioctl( struct fd *fd, ioctl_code_t code, struct async *async ) ret = connect( unix_fd, &unix_addr.addr, unix_len ); } + if (sock->family == WS_AF_UNIX && *addr->sa_data) + fchdir(server_dir_fd); + if (ret < 0 && errno != EINPROGRESS) { set_error( sock_get_ntstatus( errno ) ); @@ -2911,6 +2976,7 @@ static void sock_ioctl( struct fd *fd, ioctl_code_t code, struct async *async ) data_size_t in_size; socklen_t unix_len; int v6only = 1; + int unix_path_len = 0; /* the ioctl is METHOD_NEITHER, so ntdll gives us the output buffer as * input */ @@ -2920,8 +2986,10 @@ static void sock_ioctl( struct fd *fd, ioctl_code_t code, struct async *async ) return; } in_size = get_req_data_size() - get_reply_max_size(); + if (params->addr.sa_family == WS_AF_UNIX) + unix_path_len = in_size - sizeof(params->unknown) - sizeof(struct WS_sockaddr_un); if (in_size < offsetof(struct afd_bind_params, addr.sa_data) - || get_reply_max_size() < in_size - sizeof(int)) + || get_reply_max_size() < in_size - sizeof(int) - unix_path_len) { set_error( STATUS_INVALID_PARAMETER ); return; @@ -2933,7 +3001,47 @@ static void sock_ioctl( struct fd *fd, ioctl_code_t code, struct async *async ) return; } - unix_len = sockaddr_to_unix( ¶ms->addr, in_size - sizeof(int), &unix_addr ); + if (sock->family == WS_AF_UNIX) + { + if (*params->addr.sa_data) + { + char *unix_path; + char *base_name; + + if (!(unix_path = mem_alloc( unix_path_len + 1 ))) + return; + + memcpy( unix_path, (char *)(¶ms->addr) + sizeof(struct WS_sockaddr_un), unix_path_len ); + unix_path[unix_path_len] = '\0'; + + base_name = strrchr(unix_path, '/'); + if (base_name) + { + if (base_name != unix_path) + (++base_name)[-1] = '\0'; + } + else + base_name = unix_path; + + if (chdir( unix_path ) == -1) + { + free( unix_path ); + set_error( sock_get_ntstatus( errno ) ); + return; + } + + memset( &unix_addr.un, 0, sizeof(unix_addr.un) ); + memcpy( unix_addr.un.sun_path, base_name, strlen( base_name ) ); + free( unix_path ); + } + else + memset(unix_addr.un.sun_path, 0, sizeof(unix_addr.un.sun_path)); + unix_addr.un.sun_family = AF_UNIX; + unix_len = sizeof(unix_addr.un); + } + else + unix_len = sockaddr_to_unix( ¶ms->addr, in_size - sizeof(int), &unix_addr ); + if (!unix_len) { set_error( STATUS_INVALID_ADDRESS ); @@ -2977,8 +3085,16 @@ static void sock_ioctl( struct fd *fd, ioctl_code_t code, struct async *async ) if (errno == EADDRINUSE && sock->reuseaddr) errno = EACCES; - set_error( sock_get_ntstatus( errno ) ); - return; + /* Windows' AF_UNIX implementation has an edge case allowing for a socket to bind to + * an empty path. Linux doesn't, so it throws EINVAL. We check for this situation + * here and avoid early-exiting if it's the case. */ + if (!(errno == EINVAL && sock->family == WS_AF_UNIX && !*params->addr.sa_data)) + { + set_error( sock_get_ntstatus( errno ) ); + if (sock->family == WS_AF_UNIX && *params->addr.sa_data) + fchdir(server_dir_fd); + return; + } } sock->bound = 1; @@ -2990,13 +3106,23 @@ static void sock_ioctl( struct fd *fd, ioctl_code_t code, struct async *async ) * actual unix address */ if (bind_addr.addr.sa_family == AF_INET) bind_addr.in.sin_addr = unix_addr.in.sin_addr; - sock->addr_len = sockaddr_from_unix( &bind_addr, &sock->addr.addr, sizeof(sock->addr) ); + if (bind_addr.addr.sa_family == AF_UNIX) + { + sock->addr.un.sun_family = WS_AF_UNIX; + memcpy(sock->addr.un.sun_path, params->addr.sa_data, sizeof(sock->addr.un.sun_path)); + sock->addr_len = sizeof(sock->addr.un); + } + else + sock->addr_len = sockaddr_from_unix( &bind_addr, &sock->addr.addr, sizeof(sock->addr) ); } update_addr_usage( sock, &bind_addr, v6only ); if (get_reply_max_size() >= sock->addr_len) set_reply_data( &sock->addr, sock->addr_len ); + + if (sock->family == WS_AF_UNIX && *params->addr.sa_data) + fchdir(server_dir_fd); return; } @@ -3013,7 +3139,15 @@ static void sock_ioctl( struct fd *fd, ioctl_code_t code, struct async *async ) return; } - set_reply_data( &sock->addr, sock->addr_len ); + if (sock->family == WS_AF_UNIX) + { + if (*sock->addr.un.sun_path) + set_reply_data( &sock->addr, sizeof(sock->addr.un.sun_family) + strlen(sock->addr.un.sun_path) + 1 ); + else + set_reply_data( &sock->addr, sizeof(sock->addr.un) ); + } + else + set_reply_data( &sock->addr, sock->addr_len ); return; case IOCTL_AFD_WINE_GETPEERNAME: -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2786
From: Ally Sommers <dropbear.sh(a)gmail.com> Deleting the socket file is a common pattern with AF_UNIX sockets, and is analogous to unbinding. --- server/fd.c | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/server/fd.c b/server/fd.c index 8576882aaa9..cbc4605634e 100644 --- a/server/fd.c +++ b/server/fd.c @@ -1939,6 +1939,19 @@ struct fd *open_fd( struct fd *root, const char *name, struct unicode_str nt_nam fd->unix_fd = open( name, O_RDONLY | (flags & ~(O_TRUNC | O_CREAT | O_EXCL)), *mode ); } + /* POSIX requires that open(2) throws EOPNOTSUPP when `path` is a Unix + * socket. *BSD throws EOPNOTSUPP in this case and the additional case of + * O_SHLOCK or O_EXLOCK being passed when `path` resides on a filesystem + * without lock support. + * + * Contrary to POSIX, Linux returns ENXIO in this case, so we also check + * that error code here. */ + if (errno == EOPNOTSUPP || errno == ENXIO) + { + if (!stat(name, &st) && S_ISSOCK(st.st_mode) && (options & FILE_DELETE_ON_CLOSE)) + goto skip_open_fail; + } + if (fd->unix_fd == -1) { /* check for trailing slash on file path */ @@ -1950,6 +1963,7 @@ struct fd *open_fd( struct fd *root, const char *name, struct unicode_str nt_nam } } +skip_open_fail: fd->nt_name = dup_nt_name( root, nt_name, &fd->nt_namelen ); fd->unix_name = NULL; if ((path = dup_fd_name( root, name ))) @@ -1961,11 +1975,12 @@ struct fd *open_fd( struct fd *root, const char *name, struct unicode_str nt_nam closed_fd->unix_fd = fd->unix_fd; closed_fd->disp_flags = 0; closed_fd->unix_name = fd->unix_name; - fstat( fd->unix_fd, &st ); + if (fd->unix_fd != -1) + fstat( fd->unix_fd, &st ); *mode = st.st_mode; - /* only bother with an inode for normal files and directories */ - if (S_ISREG(st.st_mode) || S_ISDIR(st.st_mode)) + /* only bother with an inode for normal files, directories, and socket files */ + if (S_ISREG(st.st_mode) || S_ISDIR(st.st_mode) || S_ISSOCK(st.st_mode)) { unsigned int err; struct inode *inode = get_inode( st.st_dev, st.st_ino, fd->unix_fd ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2786
From: Ally Sommers <dropbear.sh(a)gmail.com> --- server/sock.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/server/sock.c b/server/sock.c index 9f1c2cf2d38..e0ca87c207d 100644 --- a/server/sock.c +++ b/server/sock.c @@ -1868,6 +1868,12 @@ static int init_socket( struct sock *sock, int family, int type, int protocol ) return -1; } + if (unix_family == AF_UNIX && unix_type == SOCK_DGRAM) + { + set_win32_error(WSAEAFNOSUPPORT); + return -1; + } + sockfd = socket( unix_family, unix_type, unix_protocol ); #ifdef linux -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2786
From: Ally Sommers <dropbear.sh(a)gmail.com> --- server/sock.c | 72 ++++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 60 insertions(+), 12 deletions(-) diff --git a/server/sock.c b/server/sock.c index e0ca87c207d..e3afd90fd1c 100644 --- a/server/sock.c +++ b/server/sock.c @@ -2053,11 +2053,32 @@ static struct sock *accept_socket( struct sock *sock ) unix_len = sizeof(unix_addr); if (!getsockname( acceptfd, &unix_addr.addr, &unix_len )) { - acceptsock->addr_len = sockaddr_from_unix( &unix_addr, &acceptsock->addr.addr, sizeof(acceptsock->addr) ); + if (sock->family == WS_AF_UNIX) + { + acceptsock->addr_len = sock->addr_len; + acceptsock->addr.un = sock->addr.un; + } + else + { + acceptsock->addr_len = sockaddr_from_unix( &unix_addr, + &acceptsock->addr.addr, + sizeof(acceptsock->addr) ); + } + if (!getpeername( acceptfd, &unix_addr.addr, &unix_len )) - acceptsock->peer_addr_len = sockaddr_from_unix( &unix_addr, - &acceptsock->peer_addr.addr, - sizeof(acceptsock->peer_addr) ); + { + if (sock->family == WS_AF_UNIX) + { + acceptsock->peer_addr_len = sizeof( sock->peer_addr.un ); + acceptsock->peer_addr.un = sock->peer_addr.un; + } + else + { + acceptsock->peer_addr_len = sockaddr_from_unix( &unix_addr, + &acceptsock->peer_addr.addr, + sizeof(acceptsock->peer_addr) ); + } + } } } @@ -2116,11 +2137,31 @@ static int accept_into_socket( struct sock *sock, struct sock *acceptsock ) unix_len = sizeof(unix_addr); if (!getsockname( get_unix_fd( newfd ), &unix_addr.addr, &unix_len )) { - acceptsock->addr_len = sockaddr_from_unix( &unix_addr, &acceptsock->addr.addr, sizeof(acceptsock->addr) ); + if (sock->family == WS_AF_UNIX) + { + acceptsock->addr_len = sock->addr_len; + acceptsock->addr.un = sock->addr.un; + } + else + { + acceptsock->addr_len = sockaddr_from_unix( &unix_addr, + &acceptsock->addr.addr, + sizeof(acceptsock->addr) ); + } if (!getpeername( get_unix_fd( newfd ), &unix_addr.addr, &unix_len )) - acceptsock->peer_addr_len = sockaddr_from_unix( &unix_addr, - &acceptsock->peer_addr.addr, - sizeof(acceptsock->peer_addr) ); + { + if (sock->family == WS_AF_UNIX) + { + acceptsock->peer_addr_len = sizeof( sock->peer_addr.un ); + acceptsock->peer_addr.un = sock->peer_addr.un; + } + else + { + acceptsock->peer_addr_len = sockaddr_from_unix( &unix_addr, + &acceptsock->peer_addr.addr, + sizeof(acceptsock->peer_addr) ); + } + } } clear_error(); @@ -2737,10 +2778,17 @@ static void sock_ioctl( struct fd *fd, ioctl_code_t code, struct async *async ) allow_fd_caching( sock->fd ); unix_len = sizeof(unix_addr); - getsockname( unix_fd, &unix_addr.addr, &unix_len ); - sock->addr_len = sockaddr_from_unix( &unix_addr, &sock->addr.addr, sizeof(sock->addr) ); - sock->peer_addr_len = sockaddr_from_unix( &peer_addr, &sock->peer_addr.addr, sizeof(sock->peer_addr)); - + if (sock->family == WS_AF_UNIX) + { + sock->peer_addr.un = *(struct WS_sockaddr_un *)addr; + sock->peer_addr_len = sizeof(struct WS_sockaddr_un); + } + else + { + getsockname( unix_fd, &unix_addr.addr, &unix_len ); + sock->addr_len = sockaddr_from_unix( &unix_addr, &sock->addr.addr, sizeof(sock->addr) ); + sock->peer_addr_len = sockaddr_from_unix( &peer_addr, &sock->peer_addr.addr, sizeof(sock->peer_addr)); + } sock->bound = 1; if (!ret) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2786
From: Ally Sommers <dropbear.sh(a)gmail.com> --- dlls/ws2_32/tests/sock.c | 292 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 292 insertions(+) diff --git a/dlls/ws2_32/tests/sock.c b/dlls/ws2_32/tests/sock.c index f9cc1482c41..4cd0964b4f5 100644 --- a/dlls/ws2_32/tests/sock.c +++ b/dlls/ws2_32/tests/sock.c @@ -29,6 +29,7 @@ #include <iphlpapi.h> #include <ws2tcpip.h> #include <wsipx.h> +#include <afunix.h> #include <wsnwlink.h> #include <mswsock.h> #include <mstcpip.h> @@ -14032,6 +14033,296 @@ static void test_select_after_WSAEventSelect(void) closesocket(client); } +static void test_afunix(void) +{ + SOCKET listener, client, server = 0; + SOCKADDR_UN addr = { AF_UNIX, "test_afunix.sock" }; + char serverBuf[] = "ws2_32/AF_UNIX socket test"; + char clientBuf[sizeof(serverBuf)] = { 0 }; + + char paths[4][sizeof(addr.sun_path)] = { "./tmp.sock", "../tmp.sock" }; + char dosPath[sizeof(addr.sun_path)]; + WCHAR dosWidePath[sizeof(addr.sun_path)]; + UNICODE_STRING ntPath = { 0 }; + SOCKADDR_UN outAddr = { 0 }; + int outAddrSize = sizeof(outAddr); + SOCKADDR_UN truncatedAddr = { 0 }; + ULONG zero = 0; + ULONG one = 1; + int ret; + + /* Test connection and send/recv */ + listener = socket(AF_UNIX, SOCK_STREAM, 0); + if (listener == INVALID_SOCKET && GetLastError() == WSAEAFNOSUPPORT) + { + win_skip("AF_UNIX sockets are unsupported, skipping...\n"); + return; + } + + ok(listener != INVALID_SOCKET, "Could not create Unix socket: %lu\n", + GetLastError()); + + ret = bind(listener, (SOCKADDR *)&addr, 0); + ok(ret && GetLastError() == WSAEFAULT, "Incorrect error: %lu\n", GetLastError()); + ret = bind(listener, (SOCKADDR *)&addr, 2); + ok(!ret, "Could not bind Unix socket: %lu\n", GetLastError()); + ret = listen(listener, 1); + ok(!ret, "Could not listen on Unix socket: %lu\n", GetLastError()); + closesocket(listener); + + listener = socket(AF_UNIX, SOCK_STREAM, 0); + ok(listener != INVALID_SOCKET, "Could not create Unix socket: %lu\n", + GetLastError()); + ret = bind(listener, (SOCKADDR *)&addr, 3); + ok(!ret, "Could not bind Unix socket: %lu\n", GetLastError()); + + memcpy(&truncatedAddr, &addr, 3); + ret = getsockname(listener, (SOCKADDR *)&outAddr, &outAddrSize); + ok(!ret, "Could not get info on Unix socket: %lu\n", GetLastError()); + ok(!memcmp(truncatedAddr.sun_path, outAddr.sun_path, sizeof(addr.sun_path)), + "getsockname returned incorrect path '%s' for provided path '%s'\n", + outAddr.sun_path, + truncatedAddr.sun_path); + ok(outAddrSize == sizeof(outAddr.sun_family) + strlen(outAddr.sun_path) + 1, + "getsockname returned incorrect size '%d' for provided path '%s'\n", + outAddrSize, + truncatedAddr.sun_path); + closesocket(listener); + ret = DeleteFileA(truncatedAddr.sun_path); + ok(ret, "DeleteFileA on socket file failed: %lu\n", GetLastError()); + ok(GetFileAttributesA(truncatedAddr.sun_path) == INVALID_FILE_ATTRIBUTES && + GetLastError() == ERROR_FILE_NOT_FOUND, + "Failed to delete socket file at path '%s'\n", + truncatedAddr.sun_path); + + listener = socket(AF_UNIX, SOCK_STREAM, 0); + ok(listener != INVALID_SOCKET, "Could not create Unix socket: %lu\n", + GetLastError()); + + ret = bind(listener, (SOCKADDR *)&addr, sizeof(SOCKADDR_UN)); + ok(!ret, "Could not bind Unix socket: %lu\n", GetLastError()); + + ret = listen(listener, 1); + ok(!ret, "Could not listen on Unix socket: %lu\n", GetLastError()); + + client = socket(AF_UNIX, SOCK_STREAM, 0); + ok(client != INVALID_SOCKET, "Failed to create second Unix socket: %lu\n", + GetLastError()); + + ret = ioctlsocket(client, FIONBIO, &one); + ok(!ret, "Could not set AF_UNIX socket to nonblocking: %lu; skipping connection\n", GetLastError()); + if (!ret) + { + ret = connect(client, (SOCKADDR *)&addr, sizeof(addr)); + ok(!ret || (ret == SOCKET_ERROR && GetLastError() == WSAEWOULDBLOCK), + "Error when connecting to Unix socket: %lu\n", + GetLastError()); + server = accept(listener, NULL, NULL); + ok(server != INVALID_SOCKET, "Could not accept Unix socket connection: %lu\n", + GetLastError()); + ret = ioctlsocket(client, FIONBIO, &zero); + ok(!ret, "Could not set AF_UNIX socket to blocking: %lu\n", GetLastError()); + } + + ret = send(server, serverBuf, sizeof(serverBuf), 0); + ok(ret == sizeof(serverBuf), "Incorrect return value from send: %d\n", ret); + ret = recv(client, clientBuf, sizeof(serverBuf), 0); + ok(ret == sizeof(serverBuf), "Incorrect return value from recv: %d\n", ret); + ok(!memcmp(serverBuf, clientBuf, sizeof(serverBuf)), "Data mismatch over Unix socket\n"); + + memset(clientBuf, 0, sizeof(clientBuf)); + + ret = sendto(server, serverBuf, sizeof(serverBuf), 0, NULL, 0); + ok(ret == sizeof(serverBuf), "Incorrect return value from sendto: %d\n", ret); + ret = recvfrom(client, clientBuf, sizeof(serverBuf), 0, NULL, 0); + ok(ret == sizeof(serverBuf), "Incorrect return value from recvfrom: %d\n", ret); + ok(!memcmp(serverBuf, clientBuf, sizeof(serverBuf)), "Data mismatch over Unix socket\n"); + + memset(serverBuf, 0, sizeof(serverBuf)); + + ret = send(client, clientBuf, sizeof(clientBuf), 0); + ok(ret == sizeof(clientBuf), "Incorrect return value from send: %d\n", ret); + ret = recv(server, serverBuf, sizeof(clientBuf), 0); + ok(ret == sizeof(serverBuf), "Incorrect return value from recv: %d\n", ret); + ok(!memcmp(serverBuf, clientBuf, sizeof(clientBuf)), "Data mismatch over Unix socket\n"); + + memset(serverBuf, 0, sizeof(serverBuf)); + + ret = sendto(client, clientBuf, sizeof(clientBuf), 0, NULL, 0); + ok(ret == sizeof(clientBuf), "Incorrect return value from sendto: %d\n", ret); + ret = recvfrom(server, serverBuf, sizeof(clientBuf), 0, NULL, 0); + ok(ret == sizeof(serverBuf), "Incorrect return value from recvfrom: %d\n", ret); + ok(!memcmp(serverBuf, clientBuf, sizeof(clientBuf)), "Data mismatch over Unix socket\n"); + + closesocket(listener); + closesocket(client); + closesocket(server); + + /* Test socket file deletion */ + ret = DeleteFileA("test_afunix.sock"); + ok(ret, "DeleteFileA on socket file failed: %lu\n", GetLastError()); + ok(GetFileAttributesA("test_afunix.sock") == INVALID_FILE_ATTRIBUTES && + GetLastError() == ERROR_FILE_NOT_FOUND, + "Failed to delete socket file at path '%s'\n", + addr.sun_path); + + /* Test failure modes */ + listener = socket(AF_UNIX, SOCK_STREAM, 0); + ok(listener != INVALID_SOCKET, "Could not create Unix socket: %lu\n", + GetLastError()); + ret = bind(listener, (SOCKADDR *)&addr, sizeof(SOCKADDR_UN)); + ok(!ret, "Could not bind Unix socket to path '%s': %lu\n", addr.sun_path, GetLastError()); + closesocket(listener); + listener = socket(AF_UNIX, SOCK_STREAM, 0); + ok(listener != INVALID_SOCKET, "Could not create Unix socket: %lu\n", + GetLastError()); + ret = bind(listener, (SOCKADDR *)&addr, sizeof(SOCKADDR_UN)); + ok(ret && GetLastError() == WSAEADDRINUSE, + "Bound Unix socket to path '%s' despite existing socket file: %lu\n", + addr.sun_path, + GetLastError()); + closesocket(listener); + ret = DeleteFileA(addr.sun_path); + ok(ret, "DeleteFileA on socket file failed: %lu\n", GetLastError()); + ok(GetFileAttributesA("test_afunix.sock") == INVALID_FILE_ATTRIBUTES && + GetLastError() == ERROR_FILE_NOT_FOUND, + "Failed to delete socket file at path '%s'\n", + addr.sun_path); + + /* Test different path types (relative, NT, etc.) */ + GetTempPathA(sizeof(paths[0]) - 1, paths[2]); + strcat(paths[2], "tmp.sock"); + MultiByteToWideChar(CP_ACP, 0, paths[2], -1, dosWidePath, sizeof(dosPath)); + RtlDosPathNameToNtPathName_U(dosWidePath, &ntPath, NULL, NULL); + RtlUnicodeToMultiByteN(paths[3], sizeof(addr.sun_path) - 1, NULL, ntPath.Buffer, ntPath.Length); + + for (int i = 0; i < sizeof(paths) / sizeof(paths[0]); i++) + { + memcpy(addr.sun_path, paths[i], sizeof(paths[i])); + + listener = socket(AF_UNIX, SOCK_STREAM, 0); + ok(listener != INVALID_SOCKET, "Could not create Unix socket: %lu\n", + GetLastError()); + + ret = bind(listener, (SOCKADDR *)&addr, sizeof(SOCKADDR_UN)); + ok(!ret, "Could not bind Unix socket to path '%s': %lu\n", addr.sun_path, GetLastError()); + + ret = listen(listener, 1); + ok(!ret, "Could not listen on Unix socket: %lu\n", GetLastError()); + + client = socket(AF_UNIX, SOCK_STREAM, 0); + ok(client != INVALID_SOCKET, "Failed to create second Unix socket: %lu\n", + GetLastError()); + + ret = ioctlsocket(client, FIONBIO, &one); + ok(!ret, "Could not set AF_UNIX socket to nonblocking: %lu; skipping connection\n", GetLastError()); + if (!ret) + { + ret = connect(client, (SOCKADDR *)&addr, sizeof(addr)); + ok(!ret || (ret == SOCKET_ERROR && GetLastError() == WSAEWOULDBLOCK), + "Error when connecting to Unix socket: %lu\n", + GetLastError()); + server = accept(listener, NULL, NULL); + ok(server != INVALID_SOCKET, "Could not accept Unix socket connection: %lu\n", + GetLastError()); + ret = ioctlsocket(client, FIONBIO, &zero); + ok(!ret, "Could not set AF_UNIX socket to blocking: %lu\n", GetLastError()); + } + + memset(&outAddr, 0, sizeof(outAddr)); + outAddrSize = sizeof(outAddr); + ret = getsockname(listener, (SOCKADDR *)&outAddr, &outAddrSize); + ok(!ret, "Could not get info on Unix socket: %lu\n", GetLastError()); + ok(!memcmp(addr.sun_path, outAddr.sun_path, sizeof(addr.sun_path)), + "getsockname returned incorrect path '%s' for provided path '%s'\n", + outAddr.sun_path, + addr.sun_path); + ok(outAddrSize == sizeof(outAddr.sun_family) + strlen(outAddr.sun_path) + 1, + "getsockname returned incorrect size '%d' for provided path '%s'\n", + outAddrSize, + addr.sun_path); + + memset(&outAddr, 0, sizeof(outAddr)); + outAddrSize = sizeof(outAddr); + ret = getsockname(client, (SOCKADDR *)&outAddr, &outAddrSize); + ok(!ret, "Could not get info on Unix socket: %lu\n", GetLastError()); + ok(!memcmp((char[108]){0}, outAddr.sun_path, sizeof(addr.sun_path)), + "getsockname returned incorrect path '%s' for provided path '%s'\n", + outAddr.sun_path, + addr.sun_path); + ok(outAddrSize == sizeof(outAddr), + "getsockname returned incorrect size '%d' for provided path '%s'\n", + outAddrSize, + addr.sun_path); + + memset(&outAddr, 0, sizeof(outAddr)); + outAddrSize = sizeof(outAddr); + ret = getsockname(server, (SOCKADDR *)&outAddr, &outAddrSize); + ok(!ret, "Could not get info on Unix socket: %lu\n", GetLastError()); + ok(!memcmp(addr.sun_path, outAddr.sun_path, sizeof(addr.sun_path)), + "getsockname returned incorrect path '%s' for provided path '%s'\n", + outAddr.sun_path, + addr.sun_path); + ok(outAddrSize == sizeof(outAddr.sun_family) + strlen(outAddr.sun_path) + 1, + "getsockname returned incorrect size '%d' for provided path '%s'\n", + outAddrSize, + addr.sun_path); + + memset(&outAddr, 0, sizeof(outAddr)); + outAddrSize = sizeof(outAddr); + ret = getpeername(listener, (SOCKADDR *)&outAddr, &outAddrSize); + ok(ret == -1, "Got info on Unix socket: %lu\n", GetLastError()); + ok(GetLastError() == WSAENOTCONN, + "Incorrect error returned from getpeername on Unix socket: %ld\n", + GetLastError()); + ok(!memcmp((char[108]){0}, outAddr.sun_path, sizeof(addr.sun_path)), + "getpeername returned incorrect path '%s' for provided path '%s'\n", + outAddr.sun_path, + addr.sun_path); + ok(outAddrSize == sizeof(outAddr), + "getpeername returned incorrect size '%d' for provided path '%s'\n", + outAddrSize, + addr.sun_path); + + memset(&outAddr, 0, sizeof(outAddr)); + outAddrSize = sizeof(outAddr); + ret = getpeername(client, (SOCKADDR *)&outAddr, &outAddrSize); + ok(!ret, "Could not get info on Unix socket: %lu\n", GetLastError()); + ok(!memcmp(addr.sun_path, outAddr.sun_path, sizeof(addr.sun_path)), + "getpeername returned incorrect path '%s' for provided path '%s'\n", + outAddr.sun_path, + addr.sun_path); + ok(outAddrSize == sizeof(outAddr), + "getpeername returned incorrect size '%d' for provided path '%s'\n", + outAddrSize, + addr.sun_path); + + memset(&outAddr, 0, sizeof(outAddr)); + outAddrSize = sizeof(outAddr); + ret = getpeername(server, (SOCKADDR *)&outAddr, &outAddrSize); + ok(!ret, "Could not get info on Unix socket: %lu\n", GetLastError()); + ok(!memcmp((char[108]){0}, outAddr.sun_path, sizeof(addr.sun_path)), + "getpeername returned incorrect path '%s' for provided path '%s'\n", + outAddr.sun_path, + addr.sun_path); + ok(outAddrSize == sizeof(outAddr), + "getpeername returned incorrect size '%d' for provided path '%s'\n", + outAddrSize, + addr.sun_path); + + closesocket(listener); + closesocket(client); + closesocket(server); + + ret = DeleteFileA(addr.sun_path); + ok(ret, "DeleteFileA on socket file failed: %lu\n", GetLastError()); + ok(GetFileAttributesA(addr.sun_path) == INVALID_FILE_ATTRIBUTES && + GetLastError() == ERROR_FILE_NOT_FOUND, + "Failed to delete socket file at path '%s'\n", + addr.sun_path); + } +} + START_TEST( sock ) { int i; @@ -14114,6 +14405,7 @@ START_TEST( sock ) test_icmp(); test_connect_udp(); test_tcp_sendto_recvfrom(); + test_afunix(); /* There is apparently an obscure interaction between this test and * test_WSAGetOverlappedResult(). -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2786
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=139434 Your paranoid android. === w1064_2qxl (64 bit report) === ws2_32: sock.c:13021: Test failed: wait timed out sock.c:13037: Test failed: wait timed out sock.c:13050: Test failed: got size 5 sock.c:13051: Test failed: got "datad" sock.c:12885: Test failed: got size 0 sock.c:12886: Test failed: got "" sock.c:12885: Test failed: got size 0 sock.c:12886: Test failed: got "" === w10pro64_zh_CN (64 bit report) === ws2_32: sock.c:6894: Test failed: wait timed out sock.c:250: Test failed: WSAPoll() returned -1
I don't want to pressure anyone, but it could be great if this was merged before wine 9.0 This MR could easily be game changing for wine compatibility tools, as `AF_UNIX` will make PE <-> unix communication much easier. I think `AF_UNIX` would be very useful for the new wow64 especially, as the new wow64 mode makes `unixlib` harder to use than before. Some possible examples where it could be useful are: - support DiscordRPC from PE side only as Discord use a unix socket for that. - making a PE implementation of `steamapi.dll` without dealing with unixlib+wow64 stuff I'm sure there will be more in the future, but I just listed the one that would benefits the most that I had in the top of my mind. I'm ready to pay to make this MR here be processed much faster to make sure it makes it for wine 9.0 -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2786#note_50912
On Sat Nov 4 17:11:07 2023 +0000, Loïc Rebmeister wrote:
I don't want to pressure anyone, but it could be great if this was merged before wine 9.0 This MR could easily be game changing for wine compatibility tools, as `AF_UNIX` will make PE <-> unix communication much easier. I think `AF_UNIX` would be very useful for the new wow64 especially, as the new wow64 mode makes `unixlib` harder to use than before. Some possible examples where it could be useful are: - support DiscordRPC from PE side only as Discord use a unix socket for that. - making a PE implementation of `steamapi.dll` without dealing with unixlib+wow64 stuff I'm sure there will be more in the future, but I just listed the one that would benefits the most that I had in the top of my mind. I'm ready to pay to make this MR here be processed much faster to make sure it makes it for wine 9.0 Not that we don't want AF_UNIX sockets in Wine, but I'll again point out that anyone wanting to communicate with a Unix process using a byte stream should be able to use a named FIFO or a TCP loopback socket instead.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2786#note_50927
On Sat Nov 4 17:11:07 2023 +0000, Zebediah Figura wrote:
Not that we don't want AF_UNIX sockets in Wine, but I'll again point out that anyone wanting to communicate with a Unix process using a byte stream should be able to use a named FIFO or a TCP loopback socket instead. Yeah, it's a thing too, but with socket you can only reserve a port.
Using a file instead of a 16bit number is better, mainly it doesn't risk to conflict cause of port collision. And I know that no one want to prevent AF_UNIX, what I meant is I just want this MR to be in for wine 9.0 I think 50€ is good enough for someone to find time to see about this MR, I know code can take a lot of time to manage. If not, I am always open to discuss about price, the only thing I need is a link to send the money. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2786#note_50931
Yeah, it's a thing too, but with socket you can only reserve a port.
Using a file instead of a 16bit number is better, mainly it doesn't risk to conflict cause of port collision.
I've never heard of IP port exhaustion being a problem in practice, but even if it is, a named FIFO doesn't have that problem. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2786#note_50948
On Sun Nov 5 02:23:55 2023 +0000, Zebediah Figura wrote:
Yeah, it's a thing too, but with socket you can only reserve a port.
Using a file instead of a 16bit number is better, mainly it doesn't risk to conflict cause of port collision. I've never heard of IP port exhaustion being a problem in practice, but even if it is, a named FIFO doesn't have that problem. Not really an "exhaustion" problem, more like conflicts for the same number twice, also ip is often shared across containers.
Also I checked about FIFO, and windows doesn't have an api that would match with unix implementation and that could be used for communication. (Source: <https://github.com/python/cpython/issues/103510>) While I agree using IP is an alternative for unix socket, sockets are also exposed to local network and other users. Anyway, I'm offering money, even if `AF_UNIX` is not used I'm not going back on my word for the money. I guess I'm getting overhyped about everything on wine cause of my health isn't allowing me to make code for wine. I really want to help wine move forward, and @zfigura I'm really grateful of all the work you put into wine. I know I may look like someone that doesn't know anything, but I watch commit made on wine almost every day. I have health issues for many years, even before I made any commit on wine, my health is just getting too bad I guess. And @julliard I see the work you put into wine too, I think you are putting too much work on yourself. I know you take your weekends off, as I watch activity on wine, but you should delegate some of the work you do to other peoples. I think adding gitlab labels that trusted peoples can add like `approved`/`need more work`/`need review` would help. I know @zfigura you worked on wine server on similar stuff before. I know I'm getting a bit off-topic, but I thought it was important for me to tell how I am grateful for the work of everyone involved here. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2786#note_50963
On Sun Nov 5 14:06:36 2023 +0000, Loïc Rebmeister wrote:
Not really an "exhaustion" problem, more like conflicts for the same number twice, also ip is often shared across containers. Also I checked about FIFO, and windows doesn't have an api that would match with unix implementation and that could be used for communication. (Source: <https://github.com/python/cpython/issues/103510>) While I agree using IP is an alternative for unix socket, sockets are also exposed to local network and other users. Anyway, I'm offering money, even if `AF_UNIX` is not used I'm not going back on my word for the money. I guess I'm getting overhyped about everything on wine cause of my health isn't allowing me to make code for wine. I really want to help wine move forward, and @zfigura I'm really grateful of all the work you put into wine. I know I may look like someone that doesn't know anything, but I watch commit made on wine almost every day. I have health issues for many years, even before I made any commit on wine, my health is just getting too bad I guess. And @julliard I see the work you put into wine too, I think you are putting too much work on yourself. I know you take your weekends off, as I watch activity on wine, but you should delegate some of the work you do to other peoples. I think adding gitlab labels that trusted peoples can add like `approved`/`need more work`/`need review` would help. I know @zfigura you worked on wine server on similar stuff before. I know I'm getting a bit off-topic, but I thought it was important for me to tell how I am grateful for the work of everyone involved here. Zebediah is right; there are other ways to cross the Wine/Unix barrier that work on master. Anyone that does have a pressing need/desire for AF_UNIX sockets in a program under Wine can apply this patch to their local build, which is what I'm currently doing. I'm keeping it free of merge conflicts (this is what the last two updates were) but otherwise it's complete as far as I'm aware.
While it would be nice to have this merged at some point, there's no rush in my opinion, and I'm certain there are more important merge requests that impact existing/commonly-used programs. Winsock code is touched rarely, so it's also not much work on my end to resolve the few merge conflicts that do crop up. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2786#note_50964
Also I checked about FIFO, and windows doesn't have an api that would match with unix implementation and that could be used for communication. (Source: https://github.com/python/cpython/issues/103510)
You can't create a FIFO from PE code, but you should be able to create one from unix code and then just do normal ReadFile / WriteFile from PE code. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2786#note_50965
On Sun Nov 5 16:46:45 2023 +0000, Zebediah Figura wrote: > > Also I checked about FIFO, and windows doesn't have an api that would > match with unix implementation and that could be used for communication. > (Source: https://github.com/python/cpython/issues/103510) > You can't create a FIFO from PE code, but you should be able to create > one from unix code and then just do normal ReadFile / WriteFile from PE code. After review I see the following: - unixlib is not easy to use outside of wine itself - FIFO is not better as it require unixlib - socket is a big no no as sockets can't filter by user id AF_UNIX is easy to use, and filter by user id by default because it's a file. I already tried to play with the wine toolchain in the past. And I was unable to make to make it load required stuff to make something useful. DXVK doesn't use wine toolchain, and wineasio doesn't use unixlib. As the old way `something.dll.so` will no longer work properly with `wow64`, and unixlib being too hard to use outside of the wine codebase, `AF_UNIX` will be the **only** safe way to do PE unix IPC outside of wine codebase. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2786#note_51557
* unixlib is not easy to use outside of wine itself * FIFO is not better as it require unixlib
I'm not talking about unixlib. The whole point of this exercise is to communicate between a Wine process, and a host process that is not run through Wine. I'm saying you can create a named FIFO from the host Unix process and open it from the Wine process like a normal file. For that matter, you can do the same thing with a non-abstract named Unix socket, which we can just write to as if they were normal character devices—you don't need AF_UNIX support in Wine for that.
* socket is a big no no as sockets can't filter by user id
It took me a second to realize what you mean. I assume what you're trying to say is "IP sockets won't work, because we want to prevent other users on the same machine from accessing the socket". That makes sense as a restriction. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2786#note_51567
What is a state for this, are we still looking to merge this at some point? What are the remaining issues to address? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2786#note_67669
On Tue Apr 23 14:37:57 2024 +0000, Anton Romanov wrote:
What is a state for this, are we still looking to merge this at some point? What are the remaining issues to address? Seems like the PR is in a left out state, without a clear approval or disapproval, this is sadly very common.
I think a ACK/NACK system to allow prominent Wine contributors/developers to triage MR with Labels early would speed up Wine development significantly without changing anything about how MR are merged. _(This idea is at least better than having to ping everyone as the only option to revive an MR)_ @dropbear what is the status on this? @julliard it seems like you are quite a very busy person, and I know that @zfigura managed a lot of network stuff in Wine in the past. Would you be ok letting @zfigura handle this MR and notify you when this MR is ready, so to save you time and allow you to review the MR only when everything is supposed to be fully ready? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2786#note_68576
Seems like the PR is in a left out state, without a clear approval or disapproval, this is sadly very common.
I think a ACK/NACK system to allow prominent Wine contributors/developers to triage MR with Labels early would speed up Wine development significantly without changing anything about how MR are merged. _(This idea is at least better than having to ping everyone as the only option to revive an MR)_
I don't think I understand what you mean. How would that improve anything?
Would you be ok letting @zfigura handle this MR and notify you when this MR is ready, so to save you time and allow you to review the MR only when everything is supposed to be fully ready?
As I said in [1], I think that this merge request is in good shape, except for 4/7 which is less related to sockets. I agree that patch is ugly, but on the other hand I don't see a clearly better approach. [1] https://gitlab.winehq.org/wine/wine/-/merge_requests/2786#note_38573 -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2786#note_68604
On Tue Apr 23 17:31:36 2024 +0000, Elizabeth Figura wrote:
Seems like the PR is in a left out state, without a clear approval or disapproval, this is sadly very common.
I think a ACK/NACK system to allow prominent Wine contributors/developers to triage MR with Labels early would speed up Wine development significantly without changing anything about how MR are merged. _(This idea is at least better than having to ping everyone as the only option to revive an MR)_ I don't think I understand what you mean. How would that improve anything? Would you be ok letting @zfigura handle this MR and notify you when this MR is ready, so to save you time and allow you to review the MR only when everything is supposed to be fully ready? As I said in [1], I think that this merge request is in good shape, except for 4/7 which is less related to sockets. I agree that patch is ugly, but on the other hand I don't see a clearly better approach. [1] https://gitlab.winehq.org/wine/wine/-/merge_requests/2786#note_38573 A system that allow developers to label explicitly in the huge list of MR the one considered ready would be great to save time, this is a ugly patchwork solution to get around only one person merging MR, because the Wine project is so large I don't think anyone can handle it alone anymore.
What I also have been noticing recently is a lot of new developers are submitting patches without knowing how to name commits properly. For the socket delete patch, I think it's fine, it work around kernel limitations, and I think live patching the kernel to make it delete socket properly is quite ridiculous, even if a patch is upstream to BSD and Linux, Wine still support MacOS and LTS distro would not get the socket delete patches for year as they often run old kernels. Socket delete commit: https://gitlab.winehq.org/wine/wine/-/merge_requests/2786/diffs?commit_id=22... @zfigura If you have an idea, even tho I think the current code is fine, just send me how you want to be paid, I'm not insanely rich, but if you think you can rework the delete socket file in a way more fitting to be put into Wine, just go for it. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2786#note_68720
A system that allow developers to label explicitly in the huge list of MR the one considered ready would be great to save time, this is a ugly patchwork solution to get around only one person merging MR, because the Wine project is so large I don't think anyone can handle it alone anymore.
I still don't understand. Any commit that is submitted should be considered ready by the submitter (well, there's the "draft" status, but I still don't understand the point of that). Any commit that's been approved by a reviewer is considered ready by that reviewer. What else is necessary? What do you think is the problem that's preventing commits from being applied?
What I also have been noticing recently is a lot of new developers are submitting patches without knowing how to name commits properly.
There's been some of that, though I don't see what the relevance is?
@zfigura If you have an idea, even tho I think the current code is fine, just send me how you want to be paid, I'm not insanely rich, but if you think you can rework the delete socket file in a way more fitting to be put into Wine, just go for it.
Like I said, I don't see a clear way to make the deletion code better. If I did I would have proposed it, no compensation necessary. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2786#note_68760
On Wed Apr 24 17:48:28 2024 +0000, Elizabeth Figura wrote:
A system that allow developers to label explicitly in the huge list of MR the one considered ready would be great to save time, this is a ugly patchwork solution to get around only one person merging MR, because the Wine project is so large I don't think anyone can handle it alone anymore. I still don't understand. Any commit that is submitted should be considered ready by the submitter (well, there's the "draft" status, but I still don't understand the point of that). Any commit that's been approved by a reviewer is considered ready by that reviewer. What else is necessary? What do you think is the problem that's preventing commits from being applied? What I also have been noticing recently is a lot of new developers are submitting patches without knowing how to name commits properly. There's been some of that, though I don't see what the relevance is? @zfigura If you have an idea, even tho I think the current code is fine, just send me how you want to be paid, I'm not insanely rich, but if you think you can rework the delete socket file in a way more fitting to be put into Wine, just go for it. Like I said, I don't see a clear way to make the deletion code better. If I did I would have proposed it, no compensation necessary. This patch still works on master with all tests passing and the software I use it for still works as expected. I think it would be great for this to be merged, but the deletion code not being up to scratch is an understandable barrier. The first version was even worse.
However, at the time I did spend a while looking at ways to get deletion working, and this was the least problematic solution I could come up with. I unfortunately can't really do much more than that unless a more specific criticism than "it's ugly" (which, again, I agree with) is levied. A nominally cleaner solution would probably introduce a lot of extra code just for this one special case, which seems undesired based on feedback for a previous patch of mine, so I'm hesitant to even attempt that. If anyone has any ideas, though, I'm all ears. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2786#note_68798
participants (7)
-
Ally Sommers -
Ally Sommers (＠dropbear)
-
Anton Romanov (＠theli.ua)
-
Elizabeth Figura (＠zfigura)
-
Loïc Rebmeister
-
Marvin -
Zebediah Figura (＠zfigura)