[PATCH v2 1/4] ws2_32: Use getifaddrs() and if_nametoindex() directly in interface_bind().
In order to avoid accessing iphlpapi.
We will need to move this code to ntdll or the server, but we can't access iphlpapi from either. Even accessing a lower-level interface (i.e. ndis) is hard—we have no good way to enumerate ndis devices from either (without duplicating large parts of setupapi).
Signed-off-by: Zebediah Figura z.figura12@gmail.com --- In terms of ws2_32 complexity, this code paths seems no more complex, or even simpler. However, it omits the fallback path using SIOCGIFCON, making getifaddrs() a hard requirement for UDP binding. I don't know what if anything this risks losing support for, but current versions of all major operating systems seem to support it (BSD, Linux, Mac, Android), and we're gated on IP_BOUND_IF or IP_UNICAST_IF anyway.
v2: Intentionally link to and call the iphlpapi if_nametoindex(), which is a simple wrapper around libc anyway, by redefining the unix version. Eventually this code will be moved out of ws2_32 and this step will be unnecessary.
dlls/ws2_32/socket.c | 79 ++++++++++++++++++++---------------- dlls/ws2_32/ws2_32_private.h | 8 ++++ 2 files changed, 51 insertions(+), 36 deletions(-)
diff --git a/dlls/ws2_32/socket.c b/dlls/ws2_32/socket.c index bfe5ab186ee..ad2e3a1d683 100644 --- a/dlls/ws2_32/socket.c +++ b/dlls/ws2_32/socket.c @@ -1975,62 +1975,65 @@ static int WINAPI WS2_WSARecvMsg( SOCKET s, LPWSAMSG msg, LPDWORD lpNumberOfByte */ static BOOL interface_bind( SOCKET s, int fd, struct sockaddr *addr ) { +#if defined(HAVE_GETIFADDRS) && (defined(IP_BOUND_IF) || defined(LINUX_BOUND_IF)) struct sockaddr_in *in_sock = (struct sockaddr_in *) addr; in_addr_t bind_addr = in_sock->sin_addr.s_addr; - PIP_ADAPTER_INFO adapters = NULL, adapter; + struct ifaddrs *ifaddrs, *ifaddr; + unsigned int index; BOOL ret = FALSE; - DWORD adap_size; int enable = 1;
if (bind_addr == htonl(INADDR_ANY) || bind_addr == htonl(INADDR_LOOPBACK)) return FALSE; /* Not binding to a network adapter, special interface binding unnecessary. */ if (_get_fd_type(fd) != SOCK_DGRAM) return FALSE; /* Special interface binding is only necessary for UDP datagrams. */ - if (GetAdaptersInfo(NULL, &adap_size) != ERROR_BUFFER_OVERFLOW) - goto cleanup; - adapters = HeapAlloc(GetProcessHeap(), 0, adap_size); - if (adapters == NULL || GetAdaptersInfo(adapters, &adap_size) != NO_ERROR) - goto cleanup; - /* Search the IPv4 adapter list for the appropriate binding interface */ - for (adapter = adapters; adapter != NULL; adapter = adapter->Next) - { - in_addr_t adapter_addr = (in_addr_t) inet_addr(adapter->IpAddressList.IpAddress.String);
- if (bind_addr == adapter_addr) + if (getifaddrs( &ifaddrs ) < 0) return FALSE; + + for (ifaddr = ifaddrs; ifaddr != NULL; ifaddr = ifaddr->ifa_next) + { + if (ifaddr->ifa_addr && ifaddr->ifa_addr->sa_family == AF_INET + && ((struct sockaddr_in *)ifaddr->ifa_addr)->sin_addr.s_addr == bind_addr) { + index = if_nametoindex( ifaddr->ifa_name ); + if (!index) + { + ERR( "Unable to look up interface index for %s: %s\n", ifaddr->ifa_name, strerror( errno ) ); + continue; + } + #if defined(IP_BOUND_IF) /* IP_BOUND_IF sets both the incoming and outgoing restriction at once */ - if (setsockopt(fd, IPPROTO_IP, IP_BOUND_IF, &adapter->Index, sizeof(adapter->Index)) != 0) + if (setsockopt(fd, IPPROTO_IP, IP_BOUND_IF, &index, sizeof(index)) != 0) goto cleanup; ret = TRUE; #elif defined(LINUX_BOUND_IF) - in_addr_t ifindex = (in_addr_t) htonl(adapter->Index); - struct interface_filter specific_interface_filter; - struct sock_fprog filter_prog; + { + in_addr_t ifindex = (in_addr_t) htonl(index); + struct interface_filter specific_interface_filter; + struct sock_fprog filter_prog;
- if (setsockopt(fd, IPPROTO_IP, IP_UNICAST_IF, &ifindex, sizeof(ifindex)) != 0) - goto cleanup; /* Failed to suggest egress interface */ - specific_interface_filter = generic_interface_filter; - specific_interface_filter.iface_rule.k = adapter->Index; - specific_interface_filter.ip_rule.k = htonl(adapter_addr); - filter_prog.len = sizeof(generic_interface_filter)/sizeof(struct sock_filter); - filter_prog.filter = (struct sock_filter *) &specific_interface_filter; - if (setsockopt(fd, SOL_SOCKET, SO_ATTACH_FILTER, &filter_prog, sizeof(filter_prog)) != 0) - goto cleanup; /* Failed to specify incoming packet filter */ - ret = TRUE; -#else - FIXME("Broadcast packets on interface-bound sockets are not currently supported on this platform, " - "receiving broadcast packets will not work on socket %04lx.\n", s); + if (setsockopt(fd, IPPROTO_IP, IP_UNICAST_IF, &ifindex, sizeof(ifindex)) != 0) + goto cleanup; /* Failed to suggest egress interface */ + specific_interface_filter = generic_interface_filter; + specific_interface_filter.iface_rule.k = index; + specific_interface_filter.ip_rule.k = htonl(bind_addr); + filter_prog.len = sizeof(generic_interface_filter)/sizeof(struct sock_filter); + filter_prog.filter = (struct sock_filter *) &specific_interface_filter; + if (setsockopt(fd, SOL_SOCKET, SO_ATTACH_FILTER, &filter_prog, sizeof(filter_prog)) != 0) + goto cleanup; /* Failed to specify incoming packet filter */ + ret = TRUE; + } #endif if (ret) { EnterCriticalSection(&cs_if_addr_cache); - if (if_addr_cache_size <= adapter->Index) + if (if_addr_cache_size <= index) { unsigned int new_size; in_addr_t *new;
- new_size = max(if_addr_cache_size * 2, adapter->Index + 1); + new_size = max(if_addr_cache_size * 2, index + 1); if (!(new = heap_realloc(if_addr_cache, sizeof(*if_addr_cache) * new_size))) { ERR("No memory.\n"); @@ -2043,10 +2046,10 @@ static BOOL interface_bind( SOCKET s, int fd, struct sockaddr *addr ) if_addr_cache = new; if_addr_cache_size = new_size; } - if (if_addr_cache[adapter->Index] && if_addr_cache[adapter->Index] != adapter_addr) - WARN("Adapter addr for iface index %u has changed.\n", adapter->Index); + if (if_addr_cache[index] && if_addr_cache[index] != bind_addr) + WARN("Adapter addr for iface index %u has changed.\n", index);
- if_addr_cache[adapter->Index] = adapter_addr; + if_addr_cache[index] = bind_addr; LeaveCriticalSection(&cs_if_addr_cache); } break; @@ -2054,15 +2057,19 @@ static BOOL interface_bind( SOCKET s, int fd, struct sockaddr *addr ) } /* Will soon be switching to INADDR_ANY: permit address reuse */ if (ret && setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &enable, sizeof(enable)) == 0) - TRACE("Socket %04lx bound to interface index %d\n", s, adapter->Index); + TRACE("Socket %04lx bound to interface index %d\n", s, index); else ret = FALSE;
cleanup: if(!ret) ERR("Failed to bind to interface, receiving broadcast packets will not work on socket %04lx.\n", s); - HeapFree(GetProcessHeap(), 0, adapters); + freeifaddrs( ifaddrs ); return ret; +#else + FIXME( "Broadcast packets on interface-bound sockets are not currently supported on this platform.\n" ); + return FALSE; +#endif }
/*********************************************************************** diff --git a/dlls/ws2_32/ws2_32_private.h b/dlls/ws2_32/ws2_32_private.h index 527dbe903b4..c27c697a8b6 100644 --- a/dlls/ws2_32/ws2_32_private.h +++ b/dlls/ws2_32/ws2_32_private.h @@ -83,11 +83,18 @@ # include <resolv.h> #endif #ifdef HAVE_NET_IF_H +# define if_indextoname unix_if_indextoname +# define if_nametoindex unix_if_nametoindex # include <net/if.h> +# undef if_indextoname +# undef if_nametoindex #endif #ifdef HAVE_LINUX_FILTER_H # include <linux/filter.h> #endif +#ifdef HAVE_IFADDRS_H +# include <ifaddrs.h> +#endif
#ifdef HAVE_NETIPX_IPX_H # include <netipx/ipx.h> @@ -144,6 +151,7 @@ #include "winnt.h" #define USE_WC_PREFIX /* For CMSG_DATA */ #include "iphlpapi.h" +#include "netioapi.h" #include "ip2string.h" #include "wine/afd.h" #include "wine/server.h"
Signed-off-by: Zebediah Figura z.figura12@gmail.com --- dlls/ws2_32/socket.c | 26 +++++++++++++++++--------- dlls/ws2_32/tests/sock.c | 33 +++++++++++++++------------------ 2 files changed, 32 insertions(+), 27 deletions(-)
diff --git a/dlls/ws2_32/socket.c b/dlls/ws2_32/socket.c index ad2e3a1d683..ab736947807 100644 --- a/dlls/ws2_32/socket.c +++ b/dlls/ws2_32/socket.c @@ -3399,8 +3399,8 @@ INT WINAPI WSAIoctl(SOCKET s, DWORD code, LPVOID in_buff, DWORD in_size, LPVOID break; }
- case WS_SIO_ADDRESS_LIST_QUERY: - { + case WS_SIO_ADDRESS_LIST_QUERY: + { DWORD size;
TRACE("-> SIO_ADDRESS_LIST_QUERY request\n"); @@ -3415,17 +3415,19 @@ INT WINAPI WSAIoctl(SOCKET s, DWORD code, LPVOID in_buff, DWORD in_size, LPVOID if (GetAdaptersInfo(NULL, &size) == ERROR_BUFFER_OVERFLOW) { IP_ADAPTER_INFO *p, *table = HeapAlloc(GetProcessHeap(), 0, size); + NTSTATUS status = STATUS_SUCCESS; SOCKET_ADDRESS_LIST *sa_list; SOCKADDR_IN *sockaddr; SOCKET_ADDRESS *sa; unsigned int i; + DWORD ret = 0; DWORD num;
if (!table || GetAdaptersInfo(table, &size)) { HeapFree(GetProcessHeap(), 0, table); - status = WSAEINVAL; - break; + SetLastError( WSAEINVAL ); + return -1; }
for (p = table, num = 0; p; p = p->Next) @@ -3436,8 +3438,8 @@ INT WINAPI WSAIoctl(SOCKET s, DWORD code, LPVOID in_buff, DWORD in_size, LPVOID { *ret_size = total; HeapFree(GetProcessHeap(), 0, table); - status = WSAEFAULT; - break; + SetLastError( WSAEFAULT ); + return -1; }
sa_list = out_buff; @@ -3459,14 +3461,20 @@ INT WINAPI WSAIoctl(SOCKET s, DWORD code, LPVOID in_buff, DWORD in_size, LPVOID }
HeapFree(GetProcessHeap(), 0, table); + + ret = server_ioctl_sock( s, IOCTL_AFD_WINE_COMPLETE_ASYNC, &status, sizeof(status), + NULL, 0, ret_size, overlapped, completion ); + *ret_size = total; + SetLastError( ret ); + return ret ? -1 : 0; } else { WARN("unable to get IP address list\n"); - status = WSAEINVAL; + SetLastError( WSAEINVAL ); + return -1; } - break; - } + }
case WS_SIO_FLUSH: FIXME("SIO_FLUSH: stub.\n"); diff --git a/dlls/ws2_32/tests/sock.c b/dlls/ws2_32/tests/sock.c index 592baa806c3..e99faa4096b 100644 --- a/dlls/ws2_32/tests/sock.c +++ b/dlls/ws2_32/tests/sock.c @@ -9297,7 +9297,7 @@ static void test_address_list_query(void) size = 0; ret = WSAIoctl(s, SIO_ADDRESS_LIST_QUERY, NULL, 0, buffer, sizeof(buffer), &size, NULL, NULL); ok(!ret, "Got unexpected ret %d.\n", ret); - todo_wine ok(!WSAGetLastError(), "Got unexpected error %d.\n", WSAGetLastError()); + ok(!WSAGetLastError(), "Got unexpected error %d.\n", WSAGetLastError()); ok(size == expect_size, "Expected size %u, got %u.\n", expect_size, size);
expect_size = FIELD_OFFSET(SOCKET_ADDRESS_LIST, Address[address_list->iAddressCount]); @@ -9346,8 +9346,8 @@ static void test_address_list_query(void) ok(ret == -1, "Got unexpected ret %d.\n", ret); ok(WSAGetLastError() == WSAEFAULT, "Got unexpected error %d.\n", WSAGetLastError()); ok(size == expect_size, "Expected size %u, got %u.\n", expect_size, size); - todo_wine ok(overlapped.Internal == 0xdeadbeef, "Got status %#x.\n", (NTSTATUS)overlapped.Internal); - todo_wine ok(overlapped.InternalHigh == 0xdeadbeef, "Got size %Iu.\n", overlapped.InternalHigh); + ok(overlapped.Internal == 0xdeadbeef, "Got status %#x.\n", (NTSTATUS)overlapped.Internal); + ok(overlapped.InternalHigh == 0xdeadbeef, "Got size %Iu.\n", overlapped.InternalHigh);
overlapped.Internal = 0xdeadbeef; overlapped.InternalHigh = 0xdeadbeef; @@ -9367,8 +9367,8 @@ static void test_address_list_query(void) ok(ret == -1, "Got unexpected ret %d.\n", ret); ok(WSAGetLastError() == WSAEFAULT, "Got unexpected error %d.\n", WSAGetLastError()); ok(size == expect_size, "Expected size %u, got %u.\n", expect_size, size); - todo_wine ok(overlapped.Internal == 0xdeadbeef, "Got status %#x.\n", (NTSTATUS)overlapped.Internal); - todo_wine ok(overlapped.InternalHigh == 0xdeadbeef, "Got size %Iu.\n", overlapped.InternalHigh); + ok(overlapped.Internal == 0xdeadbeef, "Got status %#x.\n", (NTSTATUS)overlapped.Internal); + ok(overlapped.InternalHigh == 0xdeadbeef, "Got size %Iu.\n", overlapped.InternalHigh); ok(address_list->iAddressCount == 0xcccccccc, "Got %u addresses.\n", address_list->iAddressCount);
overlapped.Internal = 0xdeadbeef; @@ -9376,19 +9376,19 @@ static void test_address_list_query(void) size = 0xdeadbeef; ret = WSAIoctl(s, SIO_ADDRESS_LIST_QUERY, NULL, 0, buffer, sizeof(buffer), &size, &overlapped, NULL); ok(!ret, "Got unexpected ret %d.\n", ret); - todo_wine ok(!WSAGetLastError(), "Got unexpected error %d.\n", WSAGetLastError()); + ok(!WSAGetLastError(), "Got unexpected error %d.\n", WSAGetLastError()); ok(size == expect_size, "Expected size %u, got %u.\n", expect_size, size);
ret = GetQueuedCompletionStatus(port, &size, &key, &overlapped_ptr, 0); - todo_wine ok(ret, "Got error %u.\n", GetLastError()); - todo_wine ok(!size, "Got size %u.\n", size); + ok(ret, "Got error %u.\n", GetLastError()); + ok(!size, "Got size %u.\n", size); ok(overlapped_ptr == &overlapped, "Got overlapped %p.\n", overlapped_ptr); ok(!overlapped.Internal, "Got status %#x.\n", (NTSTATUS)overlapped.Internal); - todo_wine ok(!overlapped.InternalHigh, "Got size %Iu.\n", overlapped.InternalHigh); + ok(!overlapped.InternalHigh, "Got size %Iu.\n", overlapped.InternalHigh);
ret = GetQueuedCompletionStatus(port, &size, &key, &overlapped_ptr, 0); ok(!ret, "Expected failure.\n"); - todo_wine ok(GetLastError() == WAIT_TIMEOUT, "Got error %u.\n", GetLastError()); + ok(GetLastError() == WAIT_TIMEOUT, "Got error %u.\n", GetLastError());
closesocket(s); CloseHandle(port); @@ -9408,14 +9408,11 @@ static void test_address_list_query(void) ok(size == expect_size, "got size %u\n", size);
ret = SleepEx(0, TRUE); - todo_wine ok(ret == WAIT_IO_COMPLETION, "got %d\n", ret); - if (ret == WAIT_IO_COMPLETION) - { - ok(apc_count == 1, "APC was called %u times\n", apc_count); - ok(!apc_error, "got APC error %u\n", apc_error); - ok(!apc_size, "got APC size %u\n", apc_size); - ok(apc_overlapped == &overlapped, "got APC overlapped %p\n", apc_overlapped); - } + ok(ret == WAIT_IO_COMPLETION, "got %d\n", ret); + ok(apc_count == 1, "APC was called %u times\n", apc_count); + ok(!apc_error, "got APC error %u\n", apc_error); + ok(!apc_size, "got APC size %u\n", apc_size); + ok(apc_overlapped == &overlapped, "got APC overlapped %p\n", apc_overlapped);
closesocket(s); }
Hi,
While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check?
Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=91938
Your paranoid android.
=== wvistau64 (64 bit report) ===
ws2_32: sock.c:3826: Test failed: got error 10022 sock.c:3831: Test failed: got -1 sock.c:3833: Test failed: SIOCATMARK returned 0 sock.c:3838: Test failed: SIOCATMARK returned 0
=== debiant2 (32 bit report) ===
ws2_32: afd.c:336: Test failed: got flags 0x43 afd.c:342: Test failed: got flags 0x41
Signed-off-by: Zebediah Figura z.figura12@gmail.com --- dlls/ws2_32/socket.c | 3 ++- server/async.c | 5 ----- server/file.h | 1 - server/sock.c | 13 ++++++++++++- 4 files changed, 14 insertions(+), 8 deletions(-)
diff --git a/dlls/ws2_32/socket.c b/dlls/ws2_32/socket.c index ab736947807..3c7bfdf6edb 100644 --- a/dlls/ws2_32/socket.c +++ b/dlls/ws2_32/socket.c @@ -3639,9 +3639,10 @@ INT WINAPI WSAIoctl(SOCKET s, DWORD code, LPVOID in_buff, DWORD in_size, LPVOID
case WS_SIO_ADDRESS_LIST_CHANGE: { + int force_async = !!overlapped; DWORD ret;
- ret = server_ioctl_sock( s, IOCTL_AFD_WINE_ADDRESS_LIST_CHANGE, in_buff, in_size, + ret = server_ioctl_sock( s, IOCTL_AFD_WINE_ADDRESS_LIST_CHANGE, &force_async, sizeof(force_async), out_buff, out_size, ret_size, overlapped, completion ); SetLastError( ret ); return ret ? -1 : 0; diff --git a/server/async.c b/server/async.c index d6d3a8ad86a..4dedb27f3d8 100644 --- a/server/async.c +++ b/server/async.c @@ -557,11 +557,6 @@ struct thread *async_get_thread( struct async *async ) return async->thread; }
-int async_is_blocking( struct async *async ) -{ - return !async->event && !async->data.apc && !async->data.apc_context; -} - /* find the first pending async in queue */ struct async *find_pending_async( struct async_queue *queue ) { diff --git a/server/file.h b/server/file.h index ba97a833c71..b8bc7645b19 100644 --- a/server/file.h +++ b/server/file.h @@ -232,7 +232,6 @@ extern void fd_copy_completion( struct fd *src, struct fd *dst ); extern struct iosb *create_iosb( const void *in_data, data_size_t in_size, data_size_t out_size ); extern struct iosb *async_get_iosb( struct async *async ); extern struct thread *async_get_thread( struct async *async ); -extern int async_is_blocking( struct async *async ); extern struct async *find_pending_async( struct async_queue *queue ); extern void cancel_process_asyncs( struct process *process );
diff --git a/server/sock.c b/server/sock.c index 700d4c0761a..f2450fcc541 100644 --- a/server/sock.c +++ b/server/sock.c @@ -1882,7 +1882,17 @@ static int sock_ioctl( struct fd *fd, ioctl_code_t code, struct async *async ) }
case IOCTL_AFD_WINE_ADDRESS_LIST_CHANGE: - if ((sock->state & FD_WINE_NONBLOCKING) && async_is_blocking( async )) + { + int force_async; + + if (get_req_data_size() < sizeof(int)) + { + set_error( STATUS_BUFFER_TOO_SMALL ); + return 0; + } + force_async = *(int *)get_req_data(); + + if ((sock->state & FD_WINE_NONBLOCKING) && !force_async) { set_error( STATUS_DEVICE_NOT_READY ); return 0; @@ -1891,6 +1901,7 @@ static int sock_ioctl( struct fd *fd, ioctl_code_t code, struct async *async ) queue_async( &sock->ifchange_q, async ); set_error( STATUS_PENDING ); return 1; + }
case IOCTL_AFD_WINE_FIONBIO: if (get_req_data_size() < sizeof(int))
Signed-off-by: Zebediah Figura z.figura12@gmail.com --- dlls/ws2_32/socket.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/dlls/ws2_32/socket.c b/dlls/ws2_32/socket.c index 3c7bfdf6edb..a425905e5af 100644 --- a/dlls/ws2_32/socket.c +++ b/dlls/ws2_32/socket.c @@ -3188,6 +3188,10 @@ static DWORD server_ioctl_sock( SOCKET s, DWORD code, LPVOID in_buff, DWORD in_s if (!((ULONG_PTR)overlapped->hEvent & 1)) cvalue = overlapped; event = overlapped->hEvent; } + else + { + if (!(event = get_sync_event())) return GetLastError(); + }
if (completion) { @@ -3198,6 +3202,12 @@ static DWORD server_ioctl_sock( SOCKET s, DWORD code, LPVOID in_buff, DWORD in_s
status = NtDeviceIoControlFile( handle, event, apc, cvalue, piosb, code, in_buff, in_size, out_buff, out_size ); + if (status == STATUS_PENDING && !overlapped) + { + if (WaitForSingleObject( event, INFINITE ) == WAIT_FAILED) + return -1; + status = piosb->u.Status; + } if (status == STATUS_NOT_SUPPORTED) { FIXME("Unsupported ioctl %x (device=%x access=%x func=%x method=%x)\n",
Hi,
While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check?
Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=91940
Your paranoid android.
=== debiant2 (32 bit report) ===
ws2_32: afd.c:336: Test failed: got flags 0x43 afd.c:342: Test failed: got flags 0x41
-
Marvin -
Zebediah Figura