Signed-off-by: Paul Gofman pgofman@codeweavers.com --- Supersedes 228827-228832. For reference, the full patchset (which adds more tests and fixes individual options) is here: https://github.com/gofman/wine/commits/winsock
dlls/ws2_32/socket.c | 33 ++++++++++++++++++++------------- 1 file changed, 20 insertions(+), 13 deletions(-)
diff --git a/dlls/ws2_32/socket.c b/dlls/ws2_32/socket.c index e5e29107853..30dc007167a 100644 --- a/dlls/ws2_32/socket.c +++ b/dlls/ws2_32/socket.c @@ -1395,11 +1395,18 @@ int WINAPI getsockopt( SOCKET s, int level, int optname, char *optval, int *optl TRACE( "socket %#Ix, %s, optval %p, optlen %p (%d)\n", s, debugstr_sockopt(level, optname), optval, optlen, optlen ? *optlen : 0 );
- if ((level != SOL_SOCKET || optname != SO_OPENTYPE) && - !socket_list_find( s )) + if ((level != SOL_SOCKET || optname != SO_OPENTYPE)) { - SetLastError( WSAENOTSOCK ); - return SOCKET_ERROR; + if (!socket_list_find( s )) + { + SetLastError( WSAENOTSOCK ); + return SOCKET_ERROR; + } + if (!optlen || *optlen <= 0) + { + SetLastError( WSAEFAULT ); + return -1; + } }
switch(level) @@ -1464,7 +1471,7 @@ int WINAPI getsockopt( SOCKET s, int level, int optname, char *optval, int *optl }
case SO_CONNECT_TIME: - if (!optlen || !optval) + if (!optval) { SetLastError( WSAEFAULT ); return -1; @@ -1483,7 +1490,7 @@ int WINAPI getsockopt( SOCKET s, int level, int optname, char *optval, int *optl int len = sizeof(linger); int ret;
- if (!optlen || *optlen < sizeof(BOOL)|| !optval) + if (*optlen < sizeof(BOOL)|| !optval) { SetLastError(WSAEFAULT); return SOCKET_ERROR; @@ -1500,7 +1507,7 @@ int WINAPI getsockopt( SOCKET s, int level, int optname, char *optval, int *optl /* As mentioned in setsockopt, Windows ignores this, so we * always return true here */ case SO_DONTROUTE: - if (!optlen || *optlen < sizeof(BOOL) || !optval) + if (*optlen < sizeof(BOOL) || !optval) { SetLastError(WSAEFAULT); return SOCKET_ERROR; @@ -1521,7 +1528,7 @@ int WINAPI getsockopt( SOCKET s, int level, int optname, char *optval, int *optl int size;
/* struct linger and LINGER have different sizes */ - if (!optlen || *optlen < sizeof(LINGER) || !optval) + if (*optlen < sizeof(LINGER) || !optval) { SetLastError(WSAEFAULT); return SOCKET_ERROR; @@ -1540,7 +1547,7 @@ int WINAPI getsockopt( SOCKET s, int level, int optname, char *optval, int *optl }
case SO_MAX_MSG_SIZE: - if (!optlen || *optlen < sizeof(int) || !optval) + if (*optlen < sizeof(int) || !optval) { SetLastError(WSAEFAULT); return SOCKET_ERROR; @@ -1574,9 +1581,9 @@ int WINAPI getsockopt( SOCKET s, int level, int optname, char *optval, int *optl ret = ws_protocol_info(s, optname == SO_PROTOCOL_INFOW, &infow, &size); if (ret) { - if (!optlen || !optval || *optlen < size) + if (!optval || *optlen < size) { - if(optlen) *optlen = size; + *optlen = size; ret = 0; SetLastError(WSAEFAULT); } @@ -1606,7 +1613,7 @@ int WINAPI getsockopt( SOCKET s, int level, int optname, char *optval, int *optl WSAPROTOCOL_INFOW info; int size;
- if (!optlen || *optlen < sizeof(int) || !optval) + if (*optlen < sizeof(int) || !optval) { SetLastError(WSAEFAULT); return SOCKET_ERROR; @@ -1765,7 +1772,7 @@ int WINAPI getsockopt( SOCKET s, int level, int optname, char *optval, int *optl return server_getsockopt( s, IOCTL_AFD_WINE_GET_IPV6_MULTICAST_LOOP, optval, optlen );
case IPV6_PROTECTION_LEVEL: - if (!optlen || *optlen < sizeof(UINT) || !optval) + if (*optlen < sizeof(UINT) || !optval) { SetLastError( WSAEFAULT ); return -1;
Signed-off-by: Paul Gofman pgofman@codeweavers.com --- dlls/ws2_32/socket.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/dlls/ws2_32/socket.c b/dlls/ws2_32/socket.c index 30dc007167a..2e99f415c72 100644 --- a/dlls/ws2_32/socket.c +++ b/dlls/ws2_32/socket.c @@ -2827,6 +2827,7 @@ int WINAPI setsockopt( SOCKET s, int level, int optname, const char *optval, int return server_setsockopt( s, IOCTL_AFD_WINE_SET_SO_RCVBUF, optval, optlen );
case SO_RCVTIMEO: + if (optlen < 0) optlen = 4; return server_setsockopt( s, IOCTL_AFD_WINE_SET_SO_RCVTIMEO, optval, optlen );
case SO_REUSEADDR:
Signed-off-by: Zebediah Figura zfigura@codeweavers.com
Signed-off-by: Paul Gofman pgofman@codeweavers.com --- dlls/ws2_32/socket.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/dlls/ws2_32/socket.c b/dlls/ws2_32/socket.c index 2e99f415c72..efc0a5b488a 100644 --- a/dlls/ws2_32/socket.c +++ b/dlls/ws2_32/socket.c @@ -2837,6 +2837,7 @@ int WINAPI setsockopt( SOCKET s, int level, int optname, const char *optval, int return server_setsockopt( s, IOCTL_AFD_WINE_SET_SO_SNDBUF, optval, optlen );
case SO_SNDTIMEO: + if (optlen < 0) optlen = 4; return server_setsockopt( s, IOCTL_AFD_WINE_SET_SO_SNDTIMEO, optval, optlen );
/* SO_DEBUG is a privileged operation, ignore it. */
Signed-off-by: Zebediah Figura zfigura@codeweavers.com
Signed-off-by: Paul Gofman pgofman@codeweavers.com --- dlls/ws2_32/socket.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/dlls/ws2_32/socket.c b/dlls/ws2_32/socket.c index efc0a5b488a..40d86becef9 100644 --- a/dlls/ws2_32/socket.c +++ b/dlls/ws2_32/socket.c @@ -2946,6 +2946,12 @@ int WINAPI setsockopt( SOCKET s, int level, int optname, const char *optval, int break;
case IPPROTO_IP: + if (optlen < 0) + { + SetLastError(WSAENOBUFS); + return SOCKET_ERROR; + } + switch(optname) { case IP_ADD_MEMBERSHIP:
Signed-off-by: Zebediah Figura zfigura@codeweavers.com
Signed-off-by: Paul Gofman pgofman@codeweavers.com --- dlls/ws2_32/tests/sock.c | 138 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 136 insertions(+), 2 deletions(-)
diff --git a/dlls/ws2_32/tests/sock.c b/dlls/ws2_32/tests/sock.c index 6b6267dc153..6e492ffbb61 100644 --- a/dlls/ws2_32/tests/sock.c +++ b/dlls/ws2_32/tests/sock.c @@ -1155,15 +1155,38 @@ static const LINGER linger_testvals[] = {
static void test_set_getsockopt(void) { + static struct + { + int af; + int type; + int level; + int optname; + BOOL accepts_short_len; + unsigned int sizes[3]; + DWORD values[3]; + } + test_optsize[] = + { + {AF_INET, SOCK_STREAM, SOL_SOCKET, SO_RCVTIMEO, FALSE, {1, 2, 4}}, + {AF_INET, SOCK_STREAM, SOL_SOCKET, SO_SNDTIMEO, FALSE, {1, 2, 4}}, + {AF_INET, SOCK_DGRAM, IPPROTO_IP, IP_MULTICAST_LOOP, TRUE, {1, 1, 4}}, + {AF_INET, SOCK_DGRAM, IPPROTO_IP, IP_MULTICAST_TTL, TRUE, {1, 1, 4}}, + {AF_INET, SOCK_DGRAM, IPPROTO_IP, IP_TOS, TRUE, {1, 1, 4}}, + {AF_INET, SOCK_DGRAM, IPPROTO_IP, IP_TTL, TRUE, {1, 1, 4}}, + }; SOCKET s, s2; - int i, err, lasterr; + int i, j, err, lasterr; int timeout; LINGER lingval; int size; WSAPROTOCOL_INFOA infoA; WSAPROTOCOL_INFOW infoW; char providername[WSAPROTOCOL_LEN + 1]; - DWORD value; + DWORD expected_last_error, expected_value; + int expected_err, expected_size; + DWORD value, save_value; + UINT64 value64; + struct _prottest { int family, type, proto; @@ -1375,6 +1398,117 @@ static void test_set_getsockopt(void) err, WSAGetLastError());
closesocket(s); + + /* Test option length. */ + for (i = 0; i < ARRAY_SIZE(test_optsize); ++i) + { + winetest_push_context("i %u, level %d, optname %d", + i, test_optsize[i].level, test_optsize[i].optname); + + s2 = socket( test_optsize[i].af, test_optsize[i].type, 0 ); + ok(s2 != INVALID_SOCKET, "socket() failed error %d\n", WSAGetLastError()); + + size = sizeof(save_value); + err = getsockopt(s2, test_optsize[i].level, test_optsize[i].optname, (char*)&save_value, &size); + ok(!err, "Unexpected getsockopt result %d.\n", err); + + value64 = 0xffffffff00000001; + err = setsockopt(s2, test_optsize[i].level, test_optsize[i].optname, (char *)&value64, sizeof(value64)); + ok(!err, "Unexpected setsockopt result %d.\n", err); + ok(!WSAGetLastError(), "Unexpected WSAGetLastError() %u.\n", WSAGetLastError()); + + size = sizeof(value64); + err = getsockopt(s2, test_optsize[i].level, test_optsize[i].optname, (char*)&value64, &size); + ok(!err, "Unexpected getsockopt result %d.\n", err); + ok(size == test_optsize[i].sizes[2], "Got unexpected size %d.\n", size); + /* The behaviour regarding filling the high dword is different between options without the obvious + * pattern, it is either left untouched (more often) or zeroed. Wine doesn't touch the high dword. */ + + if (test_optsize[i].sizes[2] == 1 || test_optsize[i].level != SOL_SOCKET) + { + expected_err = -1; + expected_last_error = WSAENOBUFS; + } + else + { + expected_err = 0; + expected_last_error = 0; + } + + value = 1; + err = setsockopt(s2, test_optsize[i].level, test_optsize[i].optname, (char *)&value, -1); + ok(err == expected_err, "Unexpected setsockopt result %d.\n", err); + /* Broken between Win7 and Win10 21H1. */ + ok(WSAGetLastError() == expected_last_error || broken(expected_last_error && WSAGetLastError() == WSAEFAULT), + "Unexpected WSAGetLastError() %u.\n", WSAGetLastError()); + + size = -1; + value = 0xdeadbeef; + err = getsockopt(s2, test_optsize[i].level, test_optsize[i].optname, (char*)&value, &size); + if (test_optsize[i].optname == SO_OPENTYPE) + { + ok(!err, "Unexpected getsockopt result %d.\n", err); + ok(!WSAGetLastError(), "Unexpected WSAGetLastError() %u.\n", WSAGetLastError()); + } + else + { + ok(err == -1, "Unexpected getsockopt result %d.\n", err); + ok(WSAGetLastError() == WSAEFAULT, "Unexpected WSAGetLastError() %u.\n", WSAGetLastError()); + } + ok(size == (test_optsize[i].optname == SO_OPENTYPE ? 4 : -1), "Got unexpected size %d.\n", size); + + winetest_pop_context(); + + for (j = 0; j < ARRAY_SIZE(test_optsize[i].sizes); ++j) + { + size = 1 << j; + winetest_push_context("i %u, level %d, optname %d, len %u", + i, test_optsize[i].level, test_optsize[i].optname, size); + + value = 1; + if (test_optsize[i].values[j]) + expected_value = test_optsize[i].values[j]; + else + expected_value = 0xdeadbeef; + + if (test_optsize[i].accepts_short_len || size == 4) + { + expected_err = 0; + expected_last_error = 0; + expected_size = test_optsize[i].sizes[j]; + + if (!test_optsize[i].values[j]) + memcpy(&expected_value, &value, expected_size); + } + else + { + expected_err = -1; + expected_last_error = WSAEFAULT; + expected_size = test_optsize[i].sizes[j]; + } + + SetLastError(0xdeadbeef); + err = setsockopt(s2, test_optsize[i].level, test_optsize[i].optname, (char*)&value, size); + ok(err == expected_err, "Unexpected setsockopt result %d.\n", err); + ok(WSAGetLastError() == expected_last_error, "Unexpected WSAGetLastError() %u.\n", WSAGetLastError()); + + value = 0xdeadbeef; + SetLastError(0xdeadbeef); + err = getsockopt(s2, test_optsize[i].level, test_optsize[i].optname, (char*)&value, &size); + ok(err == expected_err, "Unexpected getsockopt result %d.\n", err); + ok(WSAGetLastError() == expected_last_error, "Unexpected WSAGetLastError() %u.\n", WSAGetLastError()); + ok(value == expected_value, "Got unexpected value %#x, expected %#x.\n", value, expected_value); + ok(size == expected_size, "Got unexpected size %d, expected %d.\n", size, expected_size); + + winetest_pop_context(); + } + + err = setsockopt(s2, test_optsize[i].level, test_optsize[i].optname, + (char*)&save_value, sizeof(save_value)); + ok(!err, "Unexpected getsockopt result %d.\n", err); + closesocket(s2); + } + /* Test with the closed socket */ SetLastError(0xdeadbeef); size = sizeof(i);
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=109918
Your paranoid android.
=== debian11 (32 bit report) ===
ws2_32: sock.c:5871: Test succeeded inside todo block: expected timeout sock.c:5871: Test succeeded inside todo block: event series matches sock.c:5881: Test failed: got unexpected event 0x20 sock.c:5881: Test failed: event series matches
Signed-off-by: Zebediah Figura zfigura@codeweavers.com
On 3/5/22 13:04, Paul Gofman wrote:
winetest_pop_context();for (j = 0; j < ARRAY_SIZE(test_optsize[i].sizes); ++j){size = 1 << j;winetest_push_context("i %u, level %d, optname %d, len %u",i, test_optsize[i].level, test_optsize[i].optname, size);
Note that contexts are nestable, so you don't need to pop before the inner loop.
Signed-off-by: Paul Gofman pgofman@codeweavers.com --- dlls/ws2_32/socket.c | 13 +++++++++++++ 1 file changed, 13 insertions(+)
diff --git a/dlls/ws2_32/socket.c b/dlls/ws2_32/socket.c index 40d86becef9..dacb41a0403 100644 --- a/dlls/ws2_32/socket.c +++ b/dlls/ws2_32/socket.c @@ -2776,6 +2776,8 @@ static int server_setsockopt( SOCKET s, ULONG code, const char *optval, int optl */ int WINAPI setsockopt( SOCKET s, int level, int optname, const char *optval, int optlen ) { + DWORD value = 0; + TRACE( "socket %#Ix, %s, optval %s, optlen %d\n", s, debugstr_sockopt(level, optname), debugstr_optval(optval, optlen), optlen );
@@ -2997,6 +2999,17 @@ int WINAPI setsockopt( SOCKET s, int level, int optname, const char *optval, int return server_setsockopt( s, IOCTL_AFD_WINE_SET_IP_RECVTTL, optval, optlen );
case IP_TOS: + if (!optlen || !optval) + { + SetLastError(WSAEFAULT); + return SOCKET_ERROR; + } + memcpy( &value, optval, min( optlen, sizeof(value) )); + if (value > 0xff) + { + SetLastError(WSAEINVAL); + return SOCKET_ERROR; + } return server_setsockopt( s, IOCTL_AFD_WINE_SET_IP_TOS, optval, optlen );
case IP_TTL:
Signed-off-by: Zebediah Figura zfigura@codeweavers.com
Signed-off-by: Paul Gofman pgofman@codeweavers.com --- dlls/ws2_32/tests/sock.c | 49 +++++++++++++++++++++++++++++++++++----- 1 file changed, 43 insertions(+), 6 deletions(-)
diff --git a/dlls/ws2_32/tests/sock.c b/dlls/ws2_32/tests/sock.c index 6e492ffbb61..1fc8d132b63 100644 --- a/dlls/ws2_32/tests/sock.c +++ b/dlls/ws2_32/tests/sock.c @@ -1164,15 +1164,17 @@ static void test_set_getsockopt(void) BOOL accepts_short_len; unsigned int sizes[3]; DWORD values[3]; + BOOL accepts_large_value; + BOOL bool_value; } test_optsize[] = { - {AF_INET, SOCK_STREAM, SOL_SOCKET, SO_RCVTIMEO, FALSE, {1, 2, 4}}, - {AF_INET, SOCK_STREAM, SOL_SOCKET, SO_SNDTIMEO, FALSE, {1, 2, 4}}, - {AF_INET, SOCK_DGRAM, IPPROTO_IP, IP_MULTICAST_LOOP, TRUE, {1, 1, 4}}, - {AF_INET, SOCK_DGRAM, IPPROTO_IP, IP_MULTICAST_TTL, TRUE, {1, 1, 4}}, - {AF_INET, SOCK_DGRAM, IPPROTO_IP, IP_TOS, TRUE, {1, 1, 4}}, - {AF_INET, SOCK_DGRAM, IPPROTO_IP, IP_TTL, TRUE, {1, 1, 4}}, + {AF_INET, SOCK_STREAM, SOL_SOCKET, SO_RCVTIMEO, FALSE, {1, 2, 4}, {0}, TRUE}, + {AF_INET, SOCK_STREAM, SOL_SOCKET, SO_SNDTIMEO, FALSE, {1, 2, 4}, {0}, TRUE}, + {AF_INET, SOCK_DGRAM, IPPROTO_IP, IP_MULTICAST_LOOP, TRUE, {1, 1, 4}, {0}, TRUE, TRUE}, + {AF_INET, SOCK_DGRAM, IPPROTO_IP, IP_MULTICAST_TTL, TRUE, {1, 1, 4}, {0}, FALSE}, + {AF_INET, SOCK_DGRAM, IPPROTO_IP, IP_TOS, TRUE, {1, 1, 4}, {0}, FALSE}, + {AF_INET, SOCK_DGRAM, IPPROTO_IP, IP_TTL, TRUE, {1, 1, 4}, {0}, FALSE}, }; SOCKET s, s2; int i, j, err, lasterr; @@ -1457,6 +1459,41 @@ static void test_set_getsockopt(void) } ok(size == (test_optsize[i].optname == SO_OPENTYPE ? 4 : -1), "Got unexpected size %d.\n", size);
+ expected_size = test_optsize[i].sizes[2]; + if (expected_size == 1) + expected_value = 0xdeadbe00; + else + expected_value = test_optsize[i].bool_value ? 0x1 : 0x100; + if (test_optsize[i].accepts_large_value) + { + expected_err = 0; + expected_last_error = 0; + } + else + { + expected_err = -1; + expected_last_error = WSAEINVAL; + } + + value = 0x100; + SetLastError(0xdeadbeef); + err = setsockopt(s2, test_optsize[i].level, test_optsize[i].optname, (char*)&value, 4); + ok(err == expected_err, "Unexpected setsockopt result %d.\n", err); + ok(WSAGetLastError() == expected_last_error, "Unexpected WSAGetLastError() %u.\n", WSAGetLastError()); + + if (test_optsize[i].accepts_large_value) + { + value = 0xdeadbeef; + SetLastError(0xdeadbeef); + size = 4; + err = getsockopt(s2, test_optsize[i].level, test_optsize[i].optname, (char*)&value, &size); + ok(err == expected_err, "Unexpected getsockopt result %d.\n", err); + ok(WSAGetLastError() == expected_last_error, "Unexpected WSAGetLastError() %u.\n", WSAGetLastError()); + todo_wine_if(test_optsize[i].optname == SO_DONTROUTE || test_optsize[i].optname == SO_LINGER) + ok(value == expected_value, "Got unexpected value %#x, expected %#x.\n", value, expected_value); + ok(size == expected_size, "Got unexpected size %u, expected %u.\n", size, expected_size); + } + winetest_pop_context();
for (j = 0; j < ARRAY_SIZE(test_optsize[i].sizes); ++j)
Signed-off-by: Zebediah Figura zfigura@codeweavers.com
-
Marvin -
Paul Gofman -
Zebediah Figura