[PATCH v2 0/6] MR1111: winsock: Always use Unix SO_REUSEADDR on TCP sockets.
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=50955 The problem which this patchset aims to solve is (sometimes) huge timeout on TCP listening port availability on Linux after the listening socket was closed one or another way. By default, the listening port will also be blocked for the whole time accepted socket waits through lengthy timeouts (sometimes; most surely when the listening process with an active connection was force killed, but not limited to this condition). BSD SO_REUSEADDR socket option is aimed mainly to avoid that extra wait on binding already closed listening socket address. From [1]: "Indicates that the rules used in validating addresses supplied in a bind(2) call should allow reuse of local addresses. For AF_INET sockets this means that a socket may bind, except when there is an active listening socket bound to the address.". Unix SO_REUSEADDR does not really allow reusing address in the Winsock sense. It will just allow to ditch the timeout (which is always the case on Windows without any specific options). Unfortunately it is not the only effect of the option. It still won't allow listening on the address simultaneously (unlike Winsock SO_REUSEADDR which allows simultaneous listening), or binding to an address which is being listened. But it will allow to bind different sockets for the same address which is not the Winsock behaviour when Winsock SO_REUSEADDR is set. So the patchset enables SO_REUSEADDR on every TCP socket and introduces the bound address tracking which will allow to return an error from bind() when needed. Not related to this patchset, but Winsock SO_REUSEADDR is somewhat closer to BSD SO_REUSEPORT, although is different in a way that _REUSEPORT will load balance connections between listeners while with Winsock _REUSEADDR the connections will always go to the first listener. I hope that the bound addresses tracking introduced in these patches may be reused in the future. E. g., maybe it might be helpful on the way of implementing the todos introduced by my extended tests (those todos are not related to this patchset and exist both with and without it). 1. https://man7.org/linux/man-pages/man7/socket.7.html -- v2: server: Set Unix SO_REUSEADDR on all the TCP sockets. server: Track SO_REUSEADDR value. ntdll: Move SO_REUSEADDR handling to server. ws2_32/tests: Also test TCP6 in test_so_reuseaddr(). ws2_32/tests: Add tests for reusing address without SO_REUSEADDR. ws2_32/tests: Make test_so_reuseaddr() more conclusive. https://gitlab.winehq.org/wine/wine/-/merge_requests/1111
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/ws2_32/tests/sock.c | 102 +++++++++++++++++++++------------------ 1 file changed, 55 insertions(+), 47 deletions(-) diff --git a/dlls/ws2_32/tests/sock.c b/dlls/ws2_32/tests/sock.c index adf16012951..879e4f7ddf7 100644 --- a/dlls/ws2_32/tests/sock.c +++ b/dlls/ws2_32/tests/sock.c @@ -2016,76 +2016,84 @@ static void test_set_getsockopt(void) static void test_so_reuseaddr(void) { struct sockaddr_in saddr; - SOCKET s1,s2; - unsigned int rc,reuse; + unsigned int rc, reuse; + SOCKET s1, s2, s3, s4; int size; - DWORD err; saddr.sin_family = AF_INET; saddr.sin_port = htons(SERVERPORT+1); saddr.sin_addr.s_addr = inet_addr("127.0.0.1"); + /* Test with SO_REUSEADDR on second socket only. */ + s1=socket(AF_INET, SOCK_STREAM, 0); + ok(s1 != INVALID_SOCKET, "got error %d.\n", WSAGetLastError()); + + rc = bind(s1, (struct sockaddr*)&saddr, sizeof(saddr)); + ok(!rc, "got error %d.\n", WSAGetLastError()); + + s2 = socket(AF_INET, SOCK_STREAM, 0); + ok(s2 != INVALID_SOCKET, "got error %d.\n", WSAGetLastError()); + + reuse = 1; + rc = setsockopt(s2, SOL_SOCKET, SO_REUSEADDR, (char*)&reuse, sizeof(reuse)); + ok(!rc, "got error %d.\n", WSAGetLastError()); + + rc = bind(s2, (struct sockaddr*)&saddr, sizeof(saddr)); + ok(rc == SOCKET_ERROR, "got rc %d.\n", rc); + ok(WSAGetLastError() == WSAEACCES, "got error %d.\n", WSAGetLastError()); + + closesocket(s1); + closesocket(s2); + + /* Test with SO_REUSEADDR on both sockets. */ s1=socket(AF_INET, SOCK_STREAM, 0); - ok(s1!=INVALID_SOCKET, "socket() failed error: %d\n", WSAGetLastError()); + ok(s1 != INVALID_SOCKET, "got error %d.\n", WSAGetLastError()); + + reuse = 1; + rc = setsockopt(s1, SOL_SOCKET, SO_REUSEADDR, (char*)&reuse, sizeof(reuse)); + ok(!rc, "got error %d.\n", WSAGetLastError()); + rc = bind(s1, (struct sockaddr*)&saddr, sizeof(saddr)); - ok(rc!=SOCKET_ERROR, "bind(s1) failed error: %d\n", WSAGetLastError()); + ok(!rc, "got error %d.\n", WSAGetLastError()); - s2=socket(AF_INET, SOCK_STREAM, 0); - ok(s2!=INVALID_SOCKET, "socket() failed error: %d\n", WSAGetLastError()); + s2 = socket(AF_INET, SOCK_STREAM, 0); + ok(s2 != INVALID_SOCKET, "got error %d.\n", WSAGetLastError()); reuse=0x1234; size=sizeof(reuse); - rc=getsockopt(s2, SOL_SOCKET, SO_REUSEADDR, (char*)&reuse, &size ); - ok(rc==0 && reuse==0,"wrong result in getsockopt(SO_REUSEADDR): rc=%d reuse=%d\n",rc,reuse); + rc=getsockopt(s2, SOL_SOCKET, SO_REUSEADDR, (char*)&reuse, &size); + ok(!rc && !reuse,"got rc %d, reuse %d.\n", rc, reuse); rc = bind(s2, (struct sockaddr*)&saddr, sizeof(saddr)); - ok(rc==SOCKET_ERROR, "bind() succeeded\n"); + ok(rc == SOCKET_ERROR, "got rc %d, error %d.\n", rc, WSAGetLastError()); reuse = 1; rc = setsockopt(s2, SOL_SOCKET, SO_REUSEADDR, (char*)&reuse, sizeof(reuse)); - ok(rc==0, "setsockopt() failed error: %d\n", WSAGetLastError()); + ok(!rc, "got error %d.\n", WSAGetLastError()); - /* On Win2k3 and above, all SO_REUSEADDR seems to do is to allow binding to - * a port immediately after closing another socket on that port, so - * basically following the BSD socket semantics here. */ rc = bind(s2, (struct sockaddr*)&saddr, sizeof(saddr)); - if(rc==0) - { - int s3=socket(AF_INET, SOCK_STREAM, 0), s4; - - /* If we could bind again in the same port this is Windows version <= XP. - * Lets test if we can really connect to one of them. */ - set_blocking(s1, FALSE); - set_blocking(s2, FALSE); - rc = listen(s1, 1); - ok(!rc, "listen() failed with error: %d\n", WSAGetLastError()); - rc = listen(s2, 1); - ok(!rc, "listen() failed with error: %d\n", WSAGetLastError()); - rc = connect(s3, (struct sockaddr*)&saddr, sizeof(saddr)); - ok(!rc, "connecting to accepting socket failed %d\n", WSAGetLastError()); - - /* the delivery of the connection is random so we need to try on both sockets */ - size = sizeof(saddr); - s4 = accept(s1, (struct sockaddr*)&saddr, &size); - if(s4 == INVALID_SOCKET) - s4 = accept(s2, (struct sockaddr*)&saddr, &size); - ok(s4 != INVALID_SOCKET, "none of the listening sockets could get the connection\n"); + ok(!rc, "got error %d.\n", WSAGetLastError()); - closesocket(s1); - closesocket(s3); - closesocket(s4); - } - else - { - err = WSAGetLastError(); - ok(err==WSAEACCES, "expected 10013, got %ld\n", err); + s3 = socket(AF_INET, SOCK_STREAM, 0); + ok(s3 != INVALID_SOCKET, "got error %d.\n", WSAGetLastError()); - closesocket(s1); - rc = bind(s2, (struct sockaddr*)&saddr, sizeof(saddr)); - ok(rc==0, "bind() failed error: %d\n", WSAGetLastError()); - } + /* Test if we can really connect to one of them. */ + rc = listen(s1, 1); + ok(!rc, "got error %d.\n", WSAGetLastError()); + rc = listen(s2, 1); + todo_wine ok(!rc, "got error %d.\n", WSAGetLastError()); + rc = connect(s3, (struct sockaddr*)&saddr, sizeof(saddr)); + ok(!rc, "got error %d.\n", WSAGetLastError()); + + /* The connection is delivered to the first socket. */ + size = sizeof(saddr); + s4 = accept(s1, (struct sockaddr*)&saddr, &size); + ok(s4 != INVALID_SOCKET, "got error %d.\n", WSAGetLastError()); + closesocket(s1); closesocket(s2); + closesocket(s3); + closesocket(s4); } #define IP_PKTINFO_LEN (sizeof(WSACMSGHDR) + WSA_CMSG_ALIGN(sizeof(struct in_pktinfo))) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1111
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/ws2_32/tests/sock.c | 56 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/dlls/ws2_32/tests/sock.c b/dlls/ws2_32/tests/sock.c index 879e4f7ddf7..53d5b611c7d 100644 --- a/dlls/ws2_32/tests/sock.c +++ b/dlls/ws2_32/tests/sock.c @@ -2088,12 +2088,68 @@ static void test_so_reuseaddr(void) /* The connection is delivered to the first socket. */ size = sizeof(saddr); s4 = accept(s1, (struct sockaddr*)&saddr, &size); + saddr.sin_port = htons(SERVERPORT + 1); ok(s4 != INVALID_SOCKET, "got error %d.\n", WSAGetLastError()); + closesocket(s1); closesocket(s2); closesocket(s3); closesocket(s4); + + /* Test binding and listening on INADDR_ANY together with 127.0.0.1. */ + s1 = socket(AF_INET, SOCK_STREAM, 0); + ok(s1 != INVALID_SOCKET, "got error %d.\n", WSAGetLastError()); + + saddr.sin_addr.s_addr = htonl(INADDR_ANY); + rc = bind(s1, (struct sockaddr *)&saddr, sizeof(saddr)); + ok(!rc, "got error %d.\n", WSAGetLastError()); + + rc = listen(s1, 1); + ok(!rc, "got error %d.\n", WSAGetLastError()); + + s2 = socket(AF_INET, SOCK_STREAM, 0); + ok(s2 != INVALID_SOCKET, "got error %d.\n", WSAGetLastError()); + + saddr.sin_addr.s_addr = inet_addr("127.0.0.1"); + rc = bind(s2, (struct sockaddr *)&saddr, sizeof(saddr)); + todo_wine ok(!rc, "got error %d.\n", WSAGetLastError()); + + rc = listen(s2, 1); + todo_wine ok(!rc, "got error %d.\n", WSAGetLastError()); + + s3 = socket(AF_INET, SOCK_STREAM, 0); + ok(s3 != INVALID_SOCKET, "got error %d.\n", WSAGetLastError()); + + rc = connect(s3, (struct sockaddr *)&saddr, sizeof(saddr)); + ok(!rc, "got error %d.\n", WSAGetLastError()); + + size = sizeof(saddr); + s4 = accept(s2, (struct sockaddr *)&saddr, &size); + saddr.sin_port = htons(SERVERPORT + 1); + todo_wine ok(s4 != INVALID_SOCKET, "got error %d.\n", WSAGetLastError()); + + closesocket(s1); + closesocket(s2); + closesocket(s3); + closesocket(s4); + + /* Test binding to INADDR_ANY on two sockets. */ + s1 = socket(AF_INET, SOCK_STREAM, 0); + ok(s1 != INVALID_SOCKET, "got error %d.\n", WSAGetLastError()); + + saddr.sin_addr.s_addr = htonl(INADDR_ANY); + rc = bind(s1, (struct sockaddr *)&saddr, sizeof(saddr)); + ok(!rc, "got error %d.\n", WSAGetLastError()); + + s2 = socket(AF_INET, SOCK_STREAM, 0); + ok(s2 != INVALID_SOCKET, "got error %d.\n", WSAGetLastError()); + + rc = bind(s2, (struct sockaddr*)&saddr, sizeof(saddr)); + ok(rc == SOCKET_ERROR && WSAGetLastError() == WSAEADDRINUSE, "got rc %d, error %d.\n", rc, WSAGetLastError()); + + closesocket(s1); + closesocket(s2); } #define IP_PKTINFO_LEN (sizeof(WSACMSGHDR) + WSA_CMSG_ALIGN(sizeof(struct in_pktinfo))) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1111
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/ws2_32/tests/sock.c | 219 ++++++++++++++++++++++----------------- 1 file changed, 122 insertions(+), 97 deletions(-) diff --git a/dlls/ws2_32/tests/sock.c b/dlls/ws2_32/tests/sock.c index 53d5b611c7d..62ab5789c7e 100644 --- a/dlls/ws2_32/tests/sock.c +++ b/dlls/ws2_32/tests/sock.c @@ -2015,141 +2015,166 @@ static void test_set_getsockopt(void) static void test_so_reuseaddr(void) { - struct sockaddr_in saddr; + static struct sockaddr_in6 saddr_in6_any, saddr_in6_loopback; + static struct sockaddr_in saddr_in_any, saddr_in_loopback; + + static const struct + { + int domain; + struct sockaddr *addr_any; + struct sockaddr *addr_loopback; + socklen_t addrlen; + } + tests[] = + { + { AF_INET, (struct sockaddr *)&saddr_in_any, (struct sockaddr *)&saddr_in_loopback, sizeof(saddr_in_any) }, + { AF_INET6, (struct sockaddr *)&saddr_in6_any, (struct sockaddr *)&saddr_in6_loopback, sizeof(saddr_in6_any) }, + }; unsigned int rc, reuse; + struct sockaddr saddr; SOCKET s1, s2, s3, s4; + unsigned int i; int size; - saddr.sin_family = AF_INET; - saddr.sin_port = htons(SERVERPORT+1); - saddr.sin_addr.s_addr = inet_addr("127.0.0.1"); + saddr_in_any.sin_family = AF_INET; + saddr_in_any.sin_port = htons(SERVERPORT + 1); + saddr_in_any.sin_addr.s_addr = htonl(INADDR_ANY); + saddr_in_loopback = saddr_in_any; + saddr_in_loopback.sin_addr.s_addr = htonl(INADDR_LOOPBACK); + + saddr_in6_any.sin6_family = AF_INET6; + saddr_in6_any.sin6_port = htons(SERVERPORT + 1); + memset( &saddr_in6_any.sin6_addr, 0, sizeof(saddr_in6_any.sin6_addr) ); + saddr_in6_loopback = saddr_in6_any; + inet_pton(AF_INET6, "::1", &saddr_in6_loopback.sin6_addr); + + for (i = 0; i < ARRAY_SIZE(tests); ++i) + { + winetest_push_context("test %u", i); - /* Test with SO_REUSEADDR on second socket only. */ - s1=socket(AF_INET, SOCK_STREAM, 0); - ok(s1 != INVALID_SOCKET, "got error %d.\n", WSAGetLastError()); + /* Test with SO_REUSEADDR on second socket only. */ + s1=socket(tests[i].domain, SOCK_STREAM, 0); + ok(s1 != INVALID_SOCKET, "got error %d.\n", WSAGetLastError()); - rc = bind(s1, (struct sockaddr*)&saddr, sizeof(saddr)); - ok(!rc, "got error %d.\n", WSAGetLastError()); + rc = bind(s1, tests[i].addr_loopback, tests[i].addrlen); + ok(!rc, "got error %d.\n", WSAGetLastError()); - s2 = socket(AF_INET, SOCK_STREAM, 0); - ok(s2 != INVALID_SOCKET, "got error %d.\n", WSAGetLastError()); + s2 = socket(tests[i].domain, SOCK_STREAM, 0); + ok(s2 != INVALID_SOCKET, "got error %d.\n", WSAGetLastError()); - reuse = 1; - rc = setsockopt(s2, SOL_SOCKET, SO_REUSEADDR, (char*)&reuse, sizeof(reuse)); - ok(!rc, "got error %d.\n", WSAGetLastError()); + reuse = 1; + rc = setsockopt(s2, SOL_SOCKET, SO_REUSEADDR, (char*)&reuse, sizeof(reuse)); + ok(!rc, "got error %d.\n", WSAGetLastError()); - rc = bind(s2, (struct sockaddr*)&saddr, sizeof(saddr)); - ok(rc == SOCKET_ERROR, "got rc %d.\n", rc); - ok(WSAGetLastError() == WSAEACCES, "got error %d.\n", WSAGetLastError()); + rc = bind(s2, tests[i].addr_loopback, tests[i].addrlen); + ok(rc == SOCKET_ERROR, "got rc %d.\n", rc); + ok(WSAGetLastError() == WSAEACCES, "got error %d.\n", WSAGetLastError()); - closesocket(s1); - closesocket(s2); + closesocket(s1); + closesocket(s2); - /* Test with SO_REUSEADDR on both sockets. */ - s1=socket(AF_INET, SOCK_STREAM, 0); - ok(s1 != INVALID_SOCKET, "got error %d.\n", WSAGetLastError()); + /* Test with SO_REUSEADDR on both sockets. */ + s1 = socket(tests[i].domain, SOCK_STREAM, 0); + ok(s1 != INVALID_SOCKET, "got error %d.\n", WSAGetLastError()); - reuse = 1; - rc = setsockopt(s1, SOL_SOCKET, SO_REUSEADDR, (char*)&reuse, sizeof(reuse)); - ok(!rc, "got error %d.\n", WSAGetLastError()); + reuse = 1; + rc = setsockopt(s1, SOL_SOCKET, SO_REUSEADDR, (char*)&reuse, sizeof(reuse)); + ok(!rc, "got error %d.\n", WSAGetLastError()); - rc = bind(s1, (struct sockaddr*)&saddr, sizeof(saddr)); - ok(!rc, "got error %d.\n", WSAGetLastError()); + rc = bind(s1, tests[i].addr_loopback, tests[i].addrlen); + ok(!rc, "got error %d.\n", WSAGetLastError()); - s2 = socket(AF_INET, SOCK_STREAM, 0); - ok(s2 != INVALID_SOCKET, "got error %d.\n", WSAGetLastError()); + s2 = socket(tests[i].domain, SOCK_STREAM, 0); + ok(s2 != INVALID_SOCKET, "got error %d.\n", WSAGetLastError()); - reuse=0x1234; - size=sizeof(reuse); - rc=getsockopt(s2, SOL_SOCKET, SO_REUSEADDR, (char*)&reuse, &size); - ok(!rc && !reuse,"got rc %d, reuse %d.\n", rc, reuse); + reuse = 0x1234; + size = sizeof(reuse); + rc = getsockopt(s2, SOL_SOCKET, SO_REUSEADDR, (char*)&reuse, &size); + ok(!rc && !reuse,"got rc %d, reuse %d.\n", rc, reuse); - rc = bind(s2, (struct sockaddr*)&saddr, sizeof(saddr)); - ok(rc == SOCKET_ERROR, "got rc %d, error %d.\n", rc, WSAGetLastError()); + rc = bind(s2, tests[i].addr_loopback, tests[i].addrlen); + ok(rc == SOCKET_ERROR, "got rc %d, error %d.\n", rc, WSAGetLastError()); - reuse = 1; - rc = setsockopt(s2, SOL_SOCKET, SO_REUSEADDR, (char*)&reuse, sizeof(reuse)); - ok(!rc, "got error %d.\n", WSAGetLastError()); + reuse = 1; + rc = setsockopt(s2, SOL_SOCKET, SO_REUSEADDR, (char*)&reuse, sizeof(reuse)); + ok(!rc, "got error %d.\n", WSAGetLastError()); - rc = bind(s2, (struct sockaddr*)&saddr, sizeof(saddr)); - ok(!rc, "got error %d.\n", WSAGetLastError()); + rc = bind(s2, tests[i].addr_loopback, tests[i].addrlen); + ok(!rc, "got error %d.\n", WSAGetLastError()); - s3 = socket(AF_INET, SOCK_STREAM, 0); - ok(s3 != INVALID_SOCKET, "got error %d.\n", WSAGetLastError()); + s3 = socket(tests[i].domain, SOCK_STREAM, 0); + ok(s3 != INVALID_SOCKET, "got error %d.\n", WSAGetLastError()); - /* Test if we can really connect to one of them. */ - rc = listen(s1, 1); - ok(!rc, "got error %d.\n", WSAGetLastError()); - rc = listen(s2, 1); - todo_wine ok(!rc, "got error %d.\n", WSAGetLastError()); - rc = connect(s3, (struct sockaddr*)&saddr, sizeof(saddr)); - ok(!rc, "got error %d.\n", WSAGetLastError()); + /* Test if we can really connect to one of them. */ + rc = listen(s1, 1); + ok(!rc, "got error %d.\n", WSAGetLastError()); + rc = listen(s2, 1); + todo_wine ok(!rc, "got error %d.\n", WSAGetLastError()); + rc = connect(s3, tests[i].addr_loopback, tests[i].addrlen); + ok(!rc, "got error %d.\n", WSAGetLastError()); - /* The connection is delivered to the first socket. */ - size = sizeof(saddr); - s4 = accept(s1, (struct sockaddr*)&saddr, &size); - saddr.sin_port = htons(SERVERPORT + 1); - ok(s4 != INVALID_SOCKET, "got error %d.\n", WSAGetLastError()); + /* The connection is delivered to the first socket. */ + size = tests[i].addrlen; + s4 = accept(s1, &saddr, &size); + ok(s4 != INVALID_SOCKET, "got error %d.\n", WSAGetLastError()); + closesocket(s1); + closesocket(s2); + closesocket(s3); + closesocket(s4); - closesocket(s1); - closesocket(s2); - closesocket(s3); - closesocket(s4); + /* Test binding and listening on INADDR_ANY together with 127.0.0.1. */ + s1 = socket(tests[i].domain, SOCK_STREAM, 0); + ok(s1 != INVALID_SOCKET, "got error %d.\n", WSAGetLastError()); - /* Test binding and listening on INADDR_ANY together with 127.0.0.1. */ - s1 = socket(AF_INET, SOCK_STREAM, 0); - ok(s1 != INVALID_SOCKET, "got error %d.\n", WSAGetLastError()); + rc = bind(s1, tests[i].addr_any, tests[i].addrlen); + ok(!rc, "got error %d.\n", WSAGetLastError()); - saddr.sin_addr.s_addr = htonl(INADDR_ANY); - rc = bind(s1, (struct sockaddr *)&saddr, sizeof(saddr)); - ok(!rc, "got error %d.\n", WSAGetLastError()); + rc = listen(s1, 1); + ok(!rc, "got error %d.\n", WSAGetLastError()); - rc = listen(s1, 1); - ok(!rc, "got error %d.\n", WSAGetLastError()); + s2 = socket(tests[i].domain, SOCK_STREAM, 0); + ok(s2 != INVALID_SOCKET, "got error %d.\n", WSAGetLastError()); - s2 = socket(AF_INET, SOCK_STREAM, 0); - ok(s2 != INVALID_SOCKET, "got error %d.\n", WSAGetLastError()); + rc = bind(s2, tests[i].addr_loopback, tests[i].addrlen); + todo_wine ok(!rc, "got error %d.\n", WSAGetLastError()); - saddr.sin_addr.s_addr = inet_addr("127.0.0.1"); - rc = bind(s2, (struct sockaddr *)&saddr, sizeof(saddr)); - todo_wine ok(!rc, "got error %d.\n", WSAGetLastError()); + rc = listen(s2, 1); + todo_wine ok(!rc, "got error %d.\n", WSAGetLastError()); - rc = listen(s2, 1); - todo_wine ok(!rc, "got error %d.\n", WSAGetLastError()); + s3 = socket(tests[i].domain, SOCK_STREAM, 0); + ok(s3 != INVALID_SOCKET, "got error %d.\n", WSAGetLastError()); - s3 = socket(AF_INET, SOCK_STREAM, 0); - ok(s3 != INVALID_SOCKET, "got error %d.\n", WSAGetLastError()); + rc = connect(s3, tests[i].addr_loopback, tests[i].addrlen); + ok(!rc, "got error %d.\n", WSAGetLastError()); - rc = connect(s3, (struct sockaddr *)&saddr, sizeof(saddr)); - ok(!rc, "got error %d.\n", WSAGetLastError()); + size = tests[i].addrlen; + s4 = accept(s2, &saddr, &size); + todo_wine ok(s4 != INVALID_SOCKET, "got error %d.\n", WSAGetLastError()); - size = sizeof(saddr); - s4 = accept(s2, (struct sockaddr *)&saddr, &size); - saddr.sin_port = htons(SERVERPORT + 1); - todo_wine ok(s4 != INVALID_SOCKET, "got error %d.\n", WSAGetLastError()); + closesocket(s1); + closesocket(s2); + closesocket(s3); + closesocket(s4); - closesocket(s1); - closesocket(s2); - closesocket(s3); - closesocket(s4); + /* Test binding to INADDR_ANY on two sockets. */ + s1 = socket(tests[i].domain, SOCK_STREAM, 0); + ok(s1 != INVALID_SOCKET, "got error %d.\n", WSAGetLastError()); - /* Test binding to INADDR_ANY on two sockets. */ - s1 = socket(AF_INET, SOCK_STREAM, 0); - ok(s1 != INVALID_SOCKET, "got error %d.\n", WSAGetLastError()); + rc = bind(s1, tests[i].addr_any, tests[i].addrlen); + ok(!rc, "got error %d.\n", WSAGetLastError()); - saddr.sin_addr.s_addr = htonl(INADDR_ANY); - rc = bind(s1, (struct sockaddr *)&saddr, sizeof(saddr)); - ok(!rc, "got error %d.\n", WSAGetLastError()); + s2 = socket(tests[i].domain, SOCK_STREAM, 0); + ok(s2 != INVALID_SOCKET, "got error %d.\n", WSAGetLastError()); - s2 = socket(AF_INET, SOCK_STREAM, 0); - ok(s2 != INVALID_SOCKET, "got error %d.\n", WSAGetLastError()); + rc = bind(s2, tests[i].addr_any, tests[i].addrlen); + ok(rc == SOCKET_ERROR && WSAGetLastError() == WSAEADDRINUSE, "got rc %d, error %d.\n", rc, WSAGetLastError()); - rc = bind(s2, (struct sockaddr*)&saddr, sizeof(saddr)); - ok(rc == SOCKET_ERROR && WSAGetLastError() == WSAEADDRINUSE, "got rc %d, error %d.\n", rc, WSAGetLastError()); + closesocket(s1); + closesocket(s2); - closesocket(s1); - closesocket(s2); + winetest_pop_context(); + } } #define IP_PKTINFO_LEN (sizeof(WSACMSGHDR) + WSA_CMSG_ALIGN(sizeof(struct in_pktinfo))) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1111
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/ntdll/unix/socket.c | 21 -------------------- server/sock.c | 41 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 21 deletions(-) diff --git a/dlls/ntdll/unix/socket.c b/dlls/ntdll/unix/socket.c index b502d3a03cb..7dbf03365bb 100644 --- a/dlls/ntdll/unix/socket.c +++ b/dlls/ntdll/unix/socket.c @@ -1977,27 +1977,6 @@ NTSTATUS sock_ioctl( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, void *apc case IOCTL_AFD_WINE_SET_SO_OOBINLINE: return do_setsockopt( handle, io, SOL_SOCKET, SO_OOBINLINE, in_buffer, in_size ); - case IOCTL_AFD_WINE_GET_SO_REUSEADDR: - return do_getsockopt( handle, io, SOL_SOCKET, SO_REUSEADDR, out_buffer, out_size ); - - /* BSD socket SO_REUSEADDR is not 100% compatible to winsock semantics; - * however, using it the BSD way fixes bug 8513 and seems to be what - * most programmers assume, anyway */ - case IOCTL_AFD_WINE_SET_SO_REUSEADDR: - { - int ret; - - if ((status = server_get_unix_fd( handle, 0, &fd, &needs_close, NULL, NULL ))) - return status; - - ret = setsockopt( fd, SOL_SOCKET, SO_REUSEADDR, in_buffer, in_size ); -#ifdef __APPLE__ - if (!ret) ret = setsockopt( fd, SOL_SOCKET, SO_REUSEPORT, in_buffer, in_size ); -#endif - status = ret ? sock_errno_to_status( errno ) : STATUS_SUCCESS; - break; - } - case IOCTL_AFD_WINE_SET_IP_ADD_MEMBERSHIP: return do_setsockopt( handle, io, IPPROTO_IP, IP_ADD_MEMBERSHIP, in_buffer, in_size ); diff --git a/server/sock.c b/server/sock.c index 4e57d6774a6..87a536bb363 100644 --- a/server/sock.c +++ b/server/sock.c @@ -2905,6 +2905,29 @@ static void sock_ioctl( struct fd *fd, ioctl_code_t code, struct async *async ) return; } + /* BSD socket SO_REUSEADDR is not 100% compatible to winsock semantics; + * however, using it the BSD way fixes bug 8513 and seems to be what + * most programmers assume, anyway */ + case IOCTL_AFD_WINE_SET_SO_REUSEADDR: + { + int reuse, ret; + + if (get_req_data_size() < sizeof(reuse)) + { + set_error( STATUS_BUFFER_TOO_SMALL ); + return; + } + + reuse = *(int *)get_req_data(); + ret = setsockopt( unix_fd, SOL_SOCKET, SO_REUSEADDR, &reuse, sizeof(reuse) ); +#ifdef __APPLE__ + if (!ret) ret = setsockopt( unix_fd, SOL_SOCKET, SO_REUSEPORT, &reuse, sizeof(reuse) ); +#endif + if (ret) + set_error( sock_get_ntstatus( errno ) ); + return; + } + case IOCTL_AFD_WINE_GET_SO_SNDBUF: { int sndbuf = sock->sndbuf; @@ -2992,6 +3015,24 @@ static void sock_ioctl( struct fd *fd, ioctl_code_t code, struct async *async ) return; } + case IOCTL_AFD_WINE_GET_SO_REUSEADDR: + { + int reuse; + socklen_t len = sizeof(reuse); + + if (!get_reply_max_size()) + { + set_error( STATUS_BUFFER_TOO_SMALL ); + return; + } + + if (!getsockopt( unix_fd, SOL_SOCKET, SO_REUSEADDR, &reuse, &len )) + set_reply_data( &reuse, min( sizeof(reuse), get_reply_max_size() )); + else + set_error( sock_get_ntstatus( errno ) ); + return; + } + case IOCTL_AFD_POLL: { if (get_reply_max_size() < get_req_data_size()) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1111
From: Paul Gofman <pgofman(a)codeweavers.com> --- server/sock.c | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/server/sock.c b/server/sock.c index 87a536bb363..7c5f0185fd5 100644 --- a/server/sock.c +++ b/server/sock.c @@ -233,6 +233,7 @@ struct sock unsigned int nonblocking : 1; /* is the socket nonblocking? */ unsigned int bound : 1; /* is the socket bound? */ unsigned int reset : 1; /* did we get a TCP reset? */ + unsigned int reuseaddr : 1; /* winsock SO_REUSEADDR option value */ }; static void sock_dump( struct object *obj, int verbose ); @@ -1545,6 +1546,7 @@ static struct sock *create_socket(void) sock->nonblocking = 0; sock->bound = 0; sock->reset = 0; + sock->reuseaddr = 0; sock->rcvbuf = 0; sock->sndbuf = 0; sock->rcvtimeo = 0; @@ -2720,14 +2722,8 @@ static void sock_ioctl( struct fd *fd, ioctl_code_t code, struct async *async ) if (bind( unix_fd, &bind_addr.addr, unix_len ) < 0) { - if (errno == EADDRINUSE) - { - int reuse; - socklen_t len = sizeof(reuse); - - if (!getsockopt( unix_fd, SOL_SOCKET, SO_REUSEADDR, (char *)&reuse, &len ) && reuse) - errno = EACCES; - } + if (errno == EADDRINUSE && sock->reuseaddr) + errno = EACCES; set_error( sock_get_ntstatus( errno ) ); return; @@ -2925,6 +2921,8 @@ static void sock_ioctl( struct fd *fd, ioctl_code_t code, struct async *async ) #endif if (ret) set_error( sock_get_ntstatus( errno ) ); + else + sock->reuseaddr = !!reuse; return; } @@ -3018,7 +3016,6 @@ static void sock_ioctl( struct fd *fd, ioctl_code_t code, struct async *async ) case IOCTL_AFD_WINE_GET_SO_REUSEADDR: { int reuse; - socklen_t len = sizeof(reuse); if (!get_reply_max_size()) { @@ -3026,10 +3023,8 @@ static void sock_ioctl( struct fd *fd, ioctl_code_t code, struct async *async ) return; } - if (!getsockopt( unix_fd, SOL_SOCKET, SO_REUSEADDR, &reuse, &len )) - set_reply_data( &reuse, min( sizeof(reuse), get_reply_max_size() )); - else - set_error( sock_get_ntstatus( errno ) ); + reuse = sock->reuseaddr; + set_reply_data( &reuse, min( sizeof(reuse), get_reply_max_size() )); return; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1111
From: Paul Gofman <pgofman(a)codeweavers.com> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=50955 --- server/sock.c | 211 ++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 194 insertions(+), 17 deletions(-) diff --git a/server/sock.c b/server/sock.c index 7c5f0185fd5..2156a1a12c2 100644 --- a/server/sock.c +++ b/server/sock.c @@ -91,6 +91,7 @@ #include "wsipx.h" #include "af_irda.h" #include "wine/afd.h" +#include "wine/rbtree.h" #include "process.h" #include "file.h" @@ -114,6 +115,19 @@ union win_sockaddr SOCKADDR_IRDA irda; }; +union unix_sockaddr +{ + struct sockaddr addr; + struct sockaddr_in in; + struct sockaddr_in6 in6; +#ifdef HAS_IPX + struct sockaddr_ipx ipx; +#endif +#ifdef HAS_IRDA + struct sockaddr_irda irda; +#endif +}; + static struct list poll_list = LIST_INIT( poll_list ); struct poll_req @@ -169,6 +183,13 @@ enum connection_state SOCK_CONNECTIONLESS, }; +struct bound_addr +{ + struct rb_entry entry; + union unix_sockaddr addr; + int reuse_count; +}; + #define MAX_ICMP_HISTORY_LENGTH 8 struct sock @@ -224,6 +245,7 @@ struct sock unsigned short icmp_seq; } icmp_fixup_data[MAX_ICMP_HISTORY_LENGTH]; /* Sent ICMP packets history used to fixup reply id. */ + struct bound_addr *bound_addr[2]; /* Links to the entries in bound addresses tree. */ unsigned int icmp_fixup_data_len; /* Sent ICMP packets history length. */ unsigned int rd_shutdown : 1; /* is the read end shut down? */ unsigned int wr_shutdown : 1; /* is the write end shut down? */ @@ -236,6 +258,137 @@ struct sock unsigned int reuseaddr : 1; /* winsock SO_REUSEADDR option value */ }; +static int is_tcp_socket( struct sock *sock ) +{ + return sock->type == WS_SOCK_STREAM && (sock->family == WS_AF_INET || sock->family == WS_AF_INET6); +} + +static int addr_compare( const void *key, const struct wine_rb_entry *entry ) +{ + const struct bound_addr *bound_addr = RB_ENTRY_VALUE(entry, struct bound_addr, entry); + const union unix_sockaddr *addr = key; + + if (addr->addr.sa_family != bound_addr->addr.addr.sa_family) + return addr->addr.sa_family < bound_addr->addr.addr.sa_family ? -1 : 1; + + if (addr->addr.sa_family == AF_INET) + { + if (addr->in.sin_port != bound_addr->addr.in.sin_port) + return addr->in.sin_port < bound_addr->addr.in.sin_port ? -1 : 1; + if (addr->in.sin_addr.s_addr == bound_addr->addr.in.sin_addr.s_addr) + return 0; + return addr->in.sin_addr.s_addr < bound_addr->addr.in.sin_addr.s_addr ? -1 : 1; + } + + assert( addr->addr.sa_family == AF_INET6 ); + if (addr->in6.sin6_port != bound_addr->addr.in6.sin6_port) + return addr->in6.sin6_port < bound_addr->addr.in6.sin6_port ? -1 : 1; + return memcmp( &addr->in6.sin6_addr, &bound_addr->addr.in6.sin6_addr, sizeof(addr->in6.sin6_addr) ); +} + +static int ipv4addr_from_v6( union unix_sockaddr *v4addr, const struct sockaddr_in6 *in6 ) +{ + v4addr->in.sin_family = AF_INET; + v4addr->in.sin_port = in6->sin6_port; + if (IN6_IS_ADDR_UNSPECIFIED(&in6->sin6_addr)) + { + v4addr->in.sin_addr.s_addr = INADDR_ANY; + return 1; + } + if (IN6_IS_ADDR_LOOPBACK(&in6->sin6_addr)) + { + v4addr->in.sin_addr.s_addr = INADDR_LOOPBACK; + return 1; + } + if (IN6_IS_ADDR_V4COMPAT(&in6->sin6_addr) || IN6_IS_ADDR_V4MAPPED(&in6->sin6_addr)) + { + memcpy( &v4addr->in.sin_addr.s_addr, &in6->sin6_addr.s6_addr[12], sizeof(v4addr->in.sin_addr.s_addr) ); + return 1; + } + return 0; +} + +static struct rb_tree bound_addresses_tree = { addr_compare }; + +static int should_track_conflicts_for_addr( struct sock *sock, const union unix_sockaddr *addr ) +{ + if (!is_tcp_socket( sock )) return 0; + + if (sock->family == WS_AF_INET && addr->addr.sa_family == AF_INET && addr->in.sin_port) + return 1; + else if (sock->family == WS_AF_INET6 && addr->addr.sa_family == AF_INET6 && addr->in6.sin6_port) + return 1; + + return 0; +} + +static int check_addr_usage( struct sock *sock, const union unix_sockaddr *addr, int v6only ) +{ + struct bound_addr *bound_addr; + union unix_sockaddr v4addr; + struct rb_entry *entry; + + if (!should_track_conflicts_for_addr( sock, addr )) return 0; + + if ((entry = rb_get( &bound_addresses_tree, addr ))) + { + bound_addr = WINE_RB_ENTRY_VALUE(entry, struct bound_addr, entry); + if (bound_addr->reuse_count == -1 || !sock->reuseaddr) return 1; + } + + if (sock->family != WS_AF_INET6 || v6only) return 0; + if (!ipv4addr_from_v6( &v4addr, &addr->in6 )) return 0; + if ((entry = rb_get( &bound_addresses_tree, &v4addr ))) + { + bound_addr = WINE_RB_ENTRY_VALUE(entry, struct bound_addr, entry); + if (bound_addr->reuse_count == -1 || !sock->reuseaddr) return 1; + } + return 0; +} + +static struct bound_addr *register_bound_address( struct sock *sock, const union unix_sockaddr *addr ) +{ + struct bound_addr *bound_addr; + + if (!(bound_addr = mem_alloc( sizeof(*bound_addr) ))) + return NULL; + + if (rb_put( &bound_addresses_tree, addr, &bound_addr->entry )) + { + free( bound_addr ); + bound_addr = WINE_RB_ENTRY_VALUE(rb_get( &bound_addresses_tree, addr ), struct bound_addr, entry); + if (bound_addr->reuse_count == -1) + { + if (debug_level) + fprintf( stderr, "register_bound_address: address being updated is already exclusively bound\n" ); + return NULL; + } + ++bound_addr->reuse_count; + } + else + { + bound_addr->addr = *addr; + bound_addr->reuse_count = sock->reuseaddr ? 1 : -1; + } + return bound_addr; +} + +static void update_addr_usage( struct sock *sock, const union unix_sockaddr *addr, int v6only ) +{ + union unix_sockaddr v4addr; + + assert( !sock->bound_addr[0] && !sock->bound_addr[1] ); + + if (!should_track_conflicts_for_addr( sock, addr )) return; + + sock->bound_addr[0] = register_bound_address( sock, addr ); + + if (sock->family != WS_AF_INET6 || v6only) return; + if (!ipv4addr_from_v6( &v4addr, &addr->in6 )) return; + + sock->bound_addr[1] = register_bound_address( sock, &v4addr ); +} + static void sock_dump( struct object *obj, int verbose ); static struct fd *sock_get_fd( struct object *obj ); static int sock_close_handle( struct object *obj, struct process *process, obj_handle_t handle ); @@ -297,19 +450,6 @@ static const struct fd_ops sock_fd_ops = sock_reselect_async /* reselect_async */ }; -union unix_sockaddr -{ - struct sockaddr addr; - struct sockaddr_in in; - struct sockaddr_in6 in6; -#ifdef HAS_IPX - struct sockaddr_ipx ipx; -#endif -#ifdef HAS_IRDA - struct sockaddr_irda irda; -#endif -}; - static int sockaddr_from_unix( const union unix_sockaddr *uaddr, struct WS_sockaddr *wsaddr, socklen_t wsaddrlen ) { memset( wsaddr, 0, wsaddrlen ); @@ -1493,11 +1633,21 @@ static int sock_close_handle( struct object *obj, struct process *process, obj_h static void sock_destroy( struct object *obj ) { struct sock *sock = (struct sock *)obj; + unsigned int i; assert( obj->ops == &sock_ops ); /* FIXME: special socket shutdown stuff? */ + for (i = 0; i < 2; ++i) + { + if (sock->bound_addr[i] && --sock->bound_addr[i]->reuse_count <= 0) + { + rb_remove( &bound_addresses_tree, &sock->bound_addr[i]->entry ); + free( sock->bound_addr[i] ); + } + } + if ( sock->deferred ) release_object( sock->deferred ); @@ -1552,6 +1702,7 @@ static struct sock *create_socket(void) sock->rcvtimeo = 0; sock->sndtimeo = 0; sock->icmp_fixup_data_len = 0; + sock->bound_addr[0] = sock->bound_addr[1] = NULL; init_async_queue( &sock->read_q ); init_async_queue( &sock->write_q ); init_async_queue( &sock->ifchange_q ); @@ -1740,6 +1891,13 @@ static int init_socket( struct sock *sock, int family, int type, int protocol ) sock->type = type; sock->family = family; + if (is_tcp_socket( sock )) + { + int reuse = 1; + + setsockopt( sockfd, SOL_SOCKET, SO_REUSEADDR, &reuse, sizeof(reuse) ); + } + if (sock->fd) { options = get_fd_options( sock->fd ); @@ -2671,6 +2829,7 @@ static void sock_ioctl( struct fd *fd, ioctl_code_t code, struct async *async ) union unix_sockaddr unix_addr, bind_addr; data_size_t in_size; socklen_t unix_len; + int v6only = 1; /* the ioctl is METHOD_NEITHER, so ntdll gives us the output buffer as * input */ @@ -2720,6 +2879,21 @@ static void sock_ioctl( struct fd *fd, ioctl_code_t code, struct async *async ) set_async_pending( async ); +#ifdef IPV6_V6ONLY + if (sock->family == WS_AF_INET6) + { + socklen_t len = sizeof(v6only); + + getsockopt( get_unix_fd(sock->fd), IPPROTO_IPV6, IPV6_V6ONLY, &v6only, &len ); + } +#endif + + if (check_addr_usage( sock, &bind_addr, v6only )) + { + set_error( sock->reuseaddr ? STATUS_ACCESS_DENIED : STATUS_SHARING_VIOLATION ); + return; + } + if (bind( unix_fd, &bind_addr.addr, unix_len ) < 0) { if (errno == EADDRINUSE && sock->reuseaddr) @@ -2741,6 +2915,8 @@ static void sock_ioctl( struct fd *fd, ioctl_code_t code, struct async *async ) 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 ); return; @@ -2901,9 +3077,7 @@ static void sock_ioctl( struct fd *fd, ioctl_code_t code, struct async *async ) return; } - /* BSD socket SO_REUSEADDR is not 100% compatible to winsock semantics; - * however, using it the BSD way fixes bug 8513 and seems to be what - * most programmers assume, anyway */ + /* BSD socket SO_REUSEADDR is not compatible with winsock semantics. */ case IOCTL_AFD_WINE_SET_SO_REUSEADDR: { int reuse, ret; @@ -2915,7 +3089,10 @@ static void sock_ioctl( struct fd *fd, ioctl_code_t code, struct async *async ) } reuse = *(int *)get_req_data(); - ret = setsockopt( unix_fd, SOL_SOCKET, SO_REUSEADDR, &reuse, sizeof(reuse) ); + if (is_tcp_socket( sock )) + ret = 0; + else + ret = setsockopt( unix_fd, SOL_SOCKET, SO_REUSEADDR, &reuse, sizeof(reuse) ); #ifdef __APPLE__ if (!ret) ret = setsockopt( unix_fd, SOL_SOCKET, SO_REUSEPORT, &reuse, sizeof(reuse) ); #endif -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1111
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=125458 Your paranoid android. === w10pro64_ar (64 bit report) === ws2_32: sock.c:7420: Test failed: wrong count 0 sock.c:7429: Test failed: wrong count 1 === debian11 (32 bit report) === httpapi: httpapi.c:1400: Test failed: Unexpected failure adding L"http://localhost:50000/", error 0. httpapi.c:1410: Test failed: Connecting to socket succeeded, 0. httpapi.c:1411: Test failed: Unexpected error connecting to socket, 0.
v2: - refactor is_blocking_addr(); - rename is_blocking_addr() -> should_track_conflicts_for_addr(); - set v6only in the caller and pass as input parameter; - use mem_alloc() and don't call fatal_error() on allocation failure; - add the test with setting SO_REUSEADDR on the on the second socket back (that was the only case test_soreuseaddr() was testing before my changes), also check error code in that test. I am also attaching a Windows test program with illustrates the issue which the patchset is here (wait time is ~60sec without the patches most of the time here). [win.c](/uploads/8226a5762ed3ebf5888c1bc5d08f2a74/win.c) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1111#note_12252
participants (3)
-
Marvin -
Paul Gofman -
Paul Gofman (＠gofman)