[PATCH 1/7] ws2_32: Return WSAEFAULT from setsockopt() for IP_MULTICAST_LOOP with zero optlen.
Signed-off-by: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/ws2_32/socket.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/dlls/ws2_32/socket.c b/dlls/ws2_32/socket.c index dacb41a0403..816ca828b90 100644 --- a/dlls/ws2_32/socket.c +++ b/dlls/ws2_32/socket.c @@ -2981,6 +2981,11 @@ int WINAPI setsockopt( SOCKET s, int level, int optname, const char *optval, int return server_setsockopt( s, IOCTL_AFD_WINE_SET_IP_MULTICAST_IF, optval, optlen ); case IP_MULTICAST_LOOP: + if (!optlen) + { + SetLastError( WSAEFAULT ); + return -1; + } return server_setsockopt( s, IOCTL_AFD_WINE_SET_IP_MULTICAST_LOOP, optval, optlen ); case IP_MULTICAST_TTL: -- 2.35.1
Signed-off-by: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/ws2_32/socket.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/dlls/ws2_32/socket.c b/dlls/ws2_32/socket.c index 816ca828b90..ed2889b3eb0 100644 --- a/dlls/ws2_32/socket.c +++ b/dlls/ws2_32/socket.c @@ -2989,6 +2989,11 @@ int WINAPI setsockopt( SOCKET s, int level, int optname, const char *optval, int return server_setsockopt( s, IOCTL_AFD_WINE_SET_IP_MULTICAST_LOOP, optval, optlen ); case IP_MULTICAST_TTL: + if (!optlen) + { + SetLastError( WSAEFAULT ); + return -1; + } return server_setsockopt( s, IOCTL_AFD_WINE_SET_IP_MULTICAST_TTL, optval, optlen ); case IP_OPTIONS: -- 2.35.1
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com>
Signed-off-by: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/ws2_32/socket.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/dlls/ws2_32/socket.c b/dlls/ws2_32/socket.c index ed2889b3eb0..e708d0d2f5e 100644 --- a/dlls/ws2_32/socket.c +++ b/dlls/ws2_32/socket.c @@ -3023,6 +3023,11 @@ int WINAPI setsockopt( SOCKET s, int level, int optname, const char *optval, int return server_setsockopt( s, IOCTL_AFD_WINE_SET_IP_TOS, optval, optlen ); case IP_TTL: + if (!optlen) + { + SetLastError( WSAEFAULT ); + return -1; + } return server_setsockopt( s, IOCTL_AFD_WINE_SET_IP_TTL, optval, optlen ); case IP_UNBLOCK_SOURCE: -- 2.35.1
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com>
Signed-off-by: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/ws2_32/tests/sock.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/dlls/ws2_32/tests/sock.c b/dlls/ws2_32/tests/sock.c index 1fc8d132b63..0fab10b4d70 100644 --- a/dlls/ws2_32/tests/sock.c +++ b/dlls/ws2_32/tests/sock.c @@ -1459,6 +1459,27 @@ static void test_set_getsockopt(void) } ok(size == (test_optsize[i].optname == SO_OPENTYPE ? 4 : -1), "Got unexpected size %d.\n", size); + if (test_optsize[i].level == SOL_SOCKET && test_optsize[i].bool_value) + { + expected_err = 0; + expected_last_error = 0; + } + else + { + expected_err = -1; + expected_last_error = WSAEFAULT; + } + value = 1; + SetLastError(0xdeadbeef); + err = setsockopt(s2, test_optsize[i].level, test_optsize[i].optname, (char*)&value, 0); + ok(err == expected_err, "Unexpected setsockopt result %d.\n", err); + ok(WSAGetLastError() == expected_last_error, "Unexpected WSAGetLastError() %u.\n", WSAGetLastError()); + + size = 0; + err = getsockopt(s2, test_optsize[i].level, test_optsize[i].optname, (char*)&value, &size); + ok(err == -1, "Unexpected getsockopt result %d.\n", err); + ok(WSAGetLastError() == WSAEFAULT, "Unexpected WSAGetLastError() %u.\n", WSAGetLastError()); + expected_size = test_optsize[i].sizes[2]; if (expected_size == 1) expected_value = 0xdeadbe00; -- 2.35.1
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=110042 Your paranoid android. === w8 (32 bit report) === ws2_32: sock.c:6726: Test failed: wrong count 0 sock.c:6735: Test failed: wrong count 1 === debian11 (32 bit French report) === ws2_32: sock.c:5929: Test succeeded inside todo block: expected timeout
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com>
Signed-off-by: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/ws2_32/socket.c | 13 ++++++++++--- dlls/ws2_32/tests/sock.c | 1 + 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/dlls/ws2_32/socket.c b/dlls/ws2_32/socket.c index e708d0d2f5e..3a1b230435a 100644 --- a/dlls/ws2_32/socket.c +++ b/dlls/ws2_32/socket.c @@ -1419,7 +1419,9 @@ int WINAPI getsockopt( SOCKET s, int level, int optname, char *optval, int *optl return server_getsockopt( s, IOCTL_AFD_WINE_GET_SO_ACCEPTCONN, optval, optlen ); case SO_BROADCAST: - return server_getsockopt( s, IOCTL_AFD_WINE_GET_SO_BROADCAST, optval, optlen ); + ret = server_getsockopt( s, IOCTL_AFD_WINE_GET_SO_BROADCAST, optval, optlen ); + if (!ret && *optlen < sizeof(DWORD)) *optlen = 1; + return ret; case SO_BSP_STATE: { @@ -2794,8 +2796,13 @@ int WINAPI setsockopt( SOCKET s, int level, int optname, const char *optval, int switch(optname) { case SO_BROADCAST: - return server_setsockopt( s, IOCTL_AFD_WINE_SET_SO_BROADCAST, optval, optlen ); - + if (!optval) + { + SetLastError( WSAEFAULT ); + return SOCKET_ERROR; + } + memcpy( &value, optval, min( optlen, sizeof(value) )); + return server_setsockopt( s, IOCTL_AFD_WINE_SET_SO_BROADCAST, (char *)&value, sizeof(value) ); case SO_DONTLINGER: { struct linger linger; diff --git a/dlls/ws2_32/tests/sock.c b/dlls/ws2_32/tests/sock.c index 0fab10b4d70..a868bc946a7 100644 --- a/dlls/ws2_32/tests/sock.c +++ b/dlls/ws2_32/tests/sock.c @@ -1169,6 +1169,7 @@ static void test_set_getsockopt(void) } test_optsize[] = { + {AF_INET, SOCK_DGRAM, SOL_SOCKET, SO_BROADCAST, TRUE, {1, 1, 4}, {0, 0xdead0001, 0}, TRUE, TRUE}, {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}, -- 2.35.1
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com>
Signed-off-by: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/ws2_32/socket.c | 12 +++++++----- dlls/ws2_32/tests/sock.c | 1 + 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/dlls/ws2_32/socket.c b/dlls/ws2_32/socket.c index 3a1b230435a..e58fc21c9f2 100644 --- a/dlls/ws2_32/socket.c +++ b/dlls/ws2_32/socket.c @@ -1488,11 +1488,12 @@ int WINAPI getsockopt( SOCKET s, int level, int optname, char *optval, int *optl case SO_DONTLINGER: { - struct linger linger; + LINGER linger; int len = sizeof(linger); + BOOL value; int ret; - if (*optlen < sizeof(BOOL)|| !optval) + if (*optlen < 1 || !optval) { SetLastError(WSAEFAULT); return SOCKET_ERROR; @@ -1500,8 +1501,9 @@ int WINAPI getsockopt( SOCKET s, int level, int optname, char *optval, int *optl if (!(ret = getsockopt( s, SOL_SOCKET, SO_LINGER, (char *)&linger, &len ))) { - *(BOOL *)optval = !linger.l_onoff; - *optlen = sizeof(BOOL); + value = !linger.l_onoff; + memcpy( optval, &value, min( sizeof(BOOL), *optlen )); + *optlen = *optlen >= sizeof(BOOL) ? sizeof(BOOL) : 1; } return ret; } @@ -2805,7 +2807,7 @@ int WINAPI setsockopt( SOCKET s, int level, int optname, const char *optval, int return server_setsockopt( s, IOCTL_AFD_WINE_SET_SO_BROADCAST, (char *)&value, sizeof(value) ); case SO_DONTLINGER: { - struct linger linger; + LINGER linger; if (!optval) { diff --git a/dlls/ws2_32/tests/sock.c b/dlls/ws2_32/tests/sock.c index a868bc946a7..5200fb185b4 100644 --- a/dlls/ws2_32/tests/sock.c +++ b/dlls/ws2_32/tests/sock.c @@ -1170,6 +1170,7 @@ static void test_set_getsockopt(void) test_optsize[] = { {AF_INET, SOCK_DGRAM, SOL_SOCKET, SO_BROADCAST, TRUE, {1, 1, 4}, {0, 0xdead0001, 0}, TRUE, TRUE}, + {AF_INET, SOCK_STREAM, SOL_SOCKET, SO_DONTLINGER, TRUE, {1, 1, 4}, {0, 0xdead0001, 0}, TRUE, TRUE}, {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}, -- 2.35.1
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com>
Signed-off-by: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/ws2_32/socket.c | 17 ++++++++++++----- dlls/ws2_32/tests/sock.c | 1 + 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/dlls/ws2_32/socket.c b/dlls/ws2_32/socket.c index e58fc21c9f2..31e8be5b6b0 100644 --- a/dlls/ws2_32/socket.c +++ b/dlls/ws2_32/socket.c @@ -1511,13 +1511,14 @@ 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 < sizeof(BOOL) || !optval) + if (*optlen < 1 || !optval) { - SetLastError(WSAEFAULT); + SetLastError( WSAEFAULT ); return SOCKET_ERROR; } - *(BOOL *)optval = TRUE; - *optlen = sizeof(BOOL); + *optval = TRUE; + *optlen = 1; + SetLastError( ERROR_SUCCESS ); return 0; case SO_ERROR: @@ -2859,7 +2860,13 @@ int WINAPI setsockopt( SOCKET s, int level, int optname, const char *optval, int /* For some reason the game GrandPrixLegends does set SO_DONTROUTE on its * socket. According to MSDN, this option is silently ignored.*/ case SO_DONTROUTE: - TRACE("Ignoring SO_DONTROUTE\n"); + TRACE( "Ignoring SO_DONTROUTE.\n" ); + if (optlen <= 0) + { + SetLastError( optlen ? WSAENOBUFS : WSAEFAULT ); + return -1; + } + SetLastError( ERROR_SUCCESS ); return 0; /* Stops two sockets from being bound to the same port. Always happens diff --git a/dlls/ws2_32/tests/sock.c b/dlls/ws2_32/tests/sock.c index 5200fb185b4..3da68b0e4a8 100644 --- a/dlls/ws2_32/tests/sock.c +++ b/dlls/ws2_32/tests/sock.c @@ -1171,6 +1171,7 @@ static void test_set_getsockopt(void) { {AF_INET, SOCK_DGRAM, SOL_SOCKET, SO_BROADCAST, TRUE, {1, 1, 4}, {0, 0xdead0001, 0}, TRUE, TRUE}, {AF_INET, SOCK_STREAM, SOL_SOCKET, SO_DONTLINGER, TRUE, {1, 1, 4}, {0, 0xdead0001, 0}, TRUE, TRUE}, + {AF_INET, SOCK_STREAM, SOL_SOCKET, SO_DONTROUTE, TRUE, {1, 1, 1}, {0}, TRUE}, {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}, -- 2.35.1
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com>
On 3/8/22 03:22, Paul Gofman wrote:
Signed-off-by: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/ws2_32/socket.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/dlls/ws2_32/socket.c b/dlls/ws2_32/socket.c index dacb41a0403..816ca828b90 100644 --- a/dlls/ws2_32/socket.c +++ b/dlls/ws2_32/socket.c @@ -2981,6 +2981,11 @@ int WINAPI setsockopt( SOCKET s, int level, int optname, const char *optval, int return server_setsockopt( s, IOCTL_AFD_WINE_SET_IP_MULTICAST_IF, optval, optlen );
case IP_MULTICAST_LOOP: + if (!optlen) + { + SetLastError( WSAEFAULT ); + return -1; + } return server_setsockopt( s, IOCTL_AFD_WINE_SET_IP_MULTICAST_LOOP, optval, optlen );
case IP_MULTICAST_TTL:
Maybe I'm missing something, but given the tests in 4/7, couldn't this be simplified by handling it outside of the inner switch, like in 9744055cf4?
On 3/8/22 19:59, Zebediah Figura wrote:
On 3/8/22 03:22, Paul Gofman wrote:
Signed-off-by: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/ws2_32/socket.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/dlls/ws2_32/socket.c b/dlls/ws2_32/socket.c index dacb41a0403..816ca828b90 100644 --- a/dlls/ws2_32/socket.c +++ b/dlls/ws2_32/socket.c @@ -2981,6 +2981,11 @@ int WINAPI setsockopt( SOCKET s, int level, int optname, const char *optval, int return server_setsockopt( s, IOCTL_AFD_WINE_SET_IP_MULTICAST_IF, optval, optlen ); case IP_MULTICAST_LOOP: + if (!optlen) + { + SetLastError( WSAEFAULT ); + return -1; + } return server_setsockopt( s, IOCTL_AFD_WINE_SET_IP_MULTICAST_LOOP, optval, optlen ); case IP_MULTICAST_TTL:
Maybe I'm missing something, but given the tests in 4/7, couldn't this be simplified by handling it outside of the inner switch, like in 9744055cf4?
Possibly, but in the end of my patchset half of the options have 'optlen < sizeof(DWORD)' condition. Also, my patchset doesn't add tests for "non-DWORD" options "IP_ADD_MEMBERSHIP" and so I don't know if that condition applies there. So it isn't apparent for me if generalizing it simplifies it, it looks like at very least I'd need to add more tests for that not directly related to the initial patchset scope.
On 3/8/22 11:47, Paul Gofman wrote:
On 3/8/22 19:59, Zebediah Figura wrote:
On 3/8/22 03:22, Paul Gofman wrote:
Signed-off-by: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/ws2_32/socket.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/dlls/ws2_32/socket.c b/dlls/ws2_32/socket.c index dacb41a0403..816ca828b90 100644 --- a/dlls/ws2_32/socket.c +++ b/dlls/ws2_32/socket.c @@ -2981,6 +2981,11 @@ int WINAPI setsockopt( SOCKET s, int level, int optname, const char *optval, int return server_setsockopt( s, IOCTL_AFD_WINE_SET_IP_MULTICAST_IF, optval, optlen ); case IP_MULTICAST_LOOP: + if (!optlen) + { + SetLastError( WSAEFAULT ); + return -1; + } return server_setsockopt( s, IOCTL_AFD_WINE_SET_IP_MULTICAST_LOOP, optval, optlen ); case IP_MULTICAST_TTL:
Maybe I'm missing something, but given the tests in 4/7, couldn't this be simplified by handling it outside of the inner switch, like in 9744055cf4?
Possibly, but in the end of my patchset half of the options have 'optlen < sizeof(DWORD)' condition. Also, my patchset doesn't add tests for "non-DWORD" options "IP_ADD_MEMBERSHIP" and so I don't know if that condition applies there. So it isn't apparent for me if generalizing it simplifies it, it looks like at very least I'd need to add more tests for that not directly related to the initial patchset scope.
Okay, fair enough.
participants (3)
-
Marvin -
Paul Gofman -
Zebediah Figura