From 532d57ed31b7b8e2c6b2d820e1aa17dca8bac383 Mon Sep 17 00:00:00 2001 From: Bert Belder Date: Sat, 9 May 2020 11:03:35 +0200 Subject: [PATCH 1/5] style: use clang-format version 10.x --- .clang-format | 4 +--- test/shared/test-util.h | 5 +++-- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/.clang-format b/.clang-format index 21fe245..6dbfdd5 100644 --- a/.clang-format +++ b/.clang-format @@ -12,11 +12,9 @@ BreakBeforeBraces: Attach ColumnLimit: 79 Cpp11BracedListStyle: true DerivePointerAlignment: false +IncludeBlocks: Preserve IndentWrappedFunctionNames: true MaxEmptyLinesToKeep: 1 PointerAlignment: Left SpaceAfterCStyleCast: true SpacesInContainerLiterals: false - -# Alas, not supported: -# ForceEmptyLineAtEOF: true diff --git a/test/shared/test-util.h b/test/shared/test-util.h index 1859483..709325b 100644 --- a/test/shared/test-util.h +++ b/test/shared/test-util.h @@ -38,8 +38,9 @@ void no_inline no_return check_fail(const char* message); /* Polyfill `static_assert` for some versions of clang and gcc. */ #if (defined(__clang__) || defined(__GNUC__)) && !defined(static_assert) -#define static_assert(condition, message) typedef __attribute__( \ - (__unused__)) int __static_assert_##__LINE__[(condition) ? 1 : -1] +#define static_assert(condition, message) \ + typedef __attribute__( \ + (__unused__)) int __static_assert_##__LINE__[(condition) ? 1 : -1] #endif #endif /* TEST_UTIL_H_ */ From 126c00944c05a8dce53d406b870a7151d70810cc Mon Sep 17 00:00:00 2001 From: Bert Belder Date: Sat, 9 May 2020 09:18:58 +0200 Subject: [PATCH 2/5] test: make the memory leak checker for tests work again It turns out that these static constructors declared with `__declspec(allocate(".CRT$XCU"))` don't work if you try to link them from a static library. With this patch we go back to passing 'leak-check.c' as an extra source file when compiling a test. --- CMakeLists.txt | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 86cb6ae..b5e6be8 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -27,7 +27,8 @@ file(GLOB SOURCES_INCLUDE include/*.h) file(GLOB SOURCES_SRC src/*.c src/*.h) file(GLOB SOURCES_SRC_C src/*.c) file(GLOB SOURCES_TEST test/*.c) -file(GLOB SOURCES_TEST_SHARED test/shared/*.c test/shared/*.h) +file(GLOB SOURCES_TEST_LEAK_CHECK test/shared/leak-check.*) +file(GLOB SOURCES_TEST_UTIL test/shared/test-util.*) set_property(GLOBAL PROPERTY USE_FOLDERS ON) source_group(doc FILES ${SOURCES_DOC}) @@ -35,7 +36,7 @@ source_group(config FILES ${SOURCES_CONFIG}) source_group(include FILES ${SOURCES_INCLUDE}) source_group(src FILES ${SOURCES_SRC}) source_group("" FILES ${SOURCES_TEST}) -source_group(test/shared FILES ${SOURCES_TEST_SHARED}) +source_group(test/shared FILES ${SOURCES_TEST_LEAK_CHECK} ${SOURCES_TEST_UTIL}) get_filename_component(LIB_NAME ${SOURCES_INCLUDE} NAME_WE) @@ -132,7 +133,7 @@ set(TESTING_LIB_TARGET "${LIB_NAME}-testing.lib") add_library( ${TESTING_LIB_TARGET} STATIC ${SOURCES_CONFIG_EXTERNAL_STATIC} ${SOURCES_CONFIG_INTERNAL_DEFAULT} - ${SOURCES_INCLUDE} ${SOURCES_SRC} ${SOURCES_TEST_SHARED} + ${SOURCES_INCLUDE} ${SOURCES_SRC} ${SOURCES_TEST_UTIL} ) target_include_directories( ${TESTING_LIB_TARGET} PUBLIC @@ -145,7 +146,7 @@ set_target_properties( foreach(TEST_SOURCE ${SOURCES_TEST}) get_filename_component(TEST_NAME ${TEST_SOURCE} NAME_WE) - add_executable(${TEST_NAME} ${TEST_SOURCE}) + add_executable(${TEST_NAME} ${TEST_SOURCE} ${SOURCES_TEST_LEAK_CHECK}) target_include_directories( ${TEST_NAME} PUBLIC config/external/static config/internal/default include src test/shared From 81411367b574525d5e3f79a2404cc9d5260d7ed6 Mon Sep 17 00:00:00 2001 From: Bert Belder Date: Tue, 12 May 2020 03:56:14 +0200 Subject: [PATCH 3/5] queue: rename functions to make their purpose more obvious --- src/poll-group.c | 6 +++--- src/port.c | 12 ++++++------ src/queue.c | 14 +++++++------- src/queue.h | 8 ++++---- 4 files changed, 20 insertions(+), 20 deletions(-) diff --git a/src/poll-group.c b/src/poll-group.c index e3ded8d..083400a 100644 --- a/src/poll-group.c +++ b/src/poll-group.c @@ -61,7 +61,7 @@ HANDLE poll_group_get_afd_helper_handle(poll_group_t* poll_group) { poll_group_t* poll_group_acquire(port_state_t* port_state) { queue_t* poll_group_queue = port_get_poll_group_queue(port_state); poll_group_t* poll_group = - !queue_empty(poll_group_queue) + !queue_is_empty(poll_group_queue) ? container_of( queue_last(poll_group_queue), poll_group_t, queue_node) : NULL; @@ -73,7 +73,7 @@ poll_group_t* poll_group_acquire(port_state_t* port_state) { return NULL; if (++poll_group->group_size == POLL_GROUP__MAX_GROUP_SIZE) - queue_move_first(poll_group_queue, &poll_group->queue_node); + queue_move_to_start(poll_group_queue, &poll_group->queue_node); return poll_group; } @@ -85,7 +85,7 @@ void poll_group_release(poll_group_t* poll_group) { poll_group->group_size--; assert(poll_group->group_size < POLL_GROUP__MAX_GROUP_SIZE); - queue_move_last(poll_group_queue, &poll_group->queue_node); + queue_move_to_end(poll_group_queue, &poll_group->queue_node); /* Poll groups are currently only freed when the epoll port is closed. */ } diff --git a/src/port.c b/src/port.c index afad1c8..6ecbb09 100644 --- a/src/port.c +++ b/src/port.c @@ -123,7 +123,7 @@ int port_delete(port_state_t* port_state) { poll_group_delete(poll_group); } - assert(queue_empty(&port_state->sock_update_queue)); + assert(queue_is_empty(&port_state->sock_update_queue)); DeleteCriticalSection(&port_state->lock); @@ -137,7 +137,7 @@ static int port__update_events(port_state_t* port_state) { /* Walk the queue, submitting new poll requests for every socket that needs * it. */ - while (!queue_empty(sock_update_queue)) { + while (!queue_is_empty(sock_update_queue)) { queue_node_t* queue_node = queue_first(sock_update_queue); sock_state_t* sock_state = sock_state_from_queue_node(queue_node); @@ -378,7 +378,7 @@ sock_state_t* port_find_socket(port_state_t* port_state, SOCKET socket) { void port_request_socket_update(port_state_t* port_state, sock_state_t* sock_state) { - if (queue_enqueued(sock_state_to_queue_node(sock_state))) + if (queue_is_enqueued(sock_state_to_queue_node(sock_state))) return; queue_append(&port_state->sock_update_queue, sock_state_to_queue_node(sock_state)); @@ -387,14 +387,14 @@ void port_request_socket_update(port_state_t* port_state, void port_cancel_socket_update(port_state_t* port_state, sock_state_t* sock_state) { unused_var(port_state); - if (!queue_enqueued(sock_state_to_queue_node(sock_state))) + if (!queue_is_enqueued(sock_state_to_queue_node(sock_state))) return; queue_remove(sock_state_to_queue_node(sock_state)); } void port_add_deleted_socket(port_state_t* port_state, sock_state_t* sock_state) { - if (queue_enqueued(sock_state_to_queue_node(sock_state))) + if (queue_is_enqueued(sock_state_to_queue_node(sock_state))) return; queue_append(&port_state->sock_deleted_queue, sock_state_to_queue_node(sock_state)); @@ -403,7 +403,7 @@ void port_add_deleted_socket(port_state_t* port_state, void port_remove_deleted_socket(port_state_t* port_state, sock_state_t* sock_state) { unused_var(port_state); - if (!queue_enqueued(sock_state_to_queue_node(sock_state))) + if (!queue_is_enqueued(sock_state_to_queue_node(sock_state))) return; queue_remove(sock_state_to_queue_node(sock_state)); } diff --git a/src/queue.c b/src/queue.c index 2b76e07..120f060 100644 --- a/src/queue.c +++ b/src/queue.c @@ -19,11 +19,11 @@ static inline void queue__detach_node(queue_node_t* node) { } queue_node_t* queue_first(const queue_t* queue) { - return !queue_empty(queue) ? queue->head.next : NULL; + return !queue_is_empty(queue) ? queue->head.next : NULL; } queue_node_t* queue_last(const queue_t* queue) { - return !queue_empty(queue) ? queue->head.prev : NULL; + return !queue_is_empty(queue) ? queue->head.prev : NULL; } void queue_prepend(queue_t* queue, queue_node_t* node) { @@ -40,12 +40,12 @@ void queue_append(queue_t* queue, queue_node_t* node) { queue->head.prev = node; } -void queue_move_first(queue_t* queue, queue_node_t* node) { +void queue_move_to_start(queue_t* queue, queue_node_t* node) { queue__detach_node(node); queue_prepend(queue, node); } -void queue_move_last(queue_t* queue, queue_node_t* node) { +void queue_move_to_end(queue_t* queue, queue_node_t* node) { queue__detach_node(node); queue_append(queue, node); } @@ -55,10 +55,10 @@ void queue_remove(queue_node_t* node) { queue_node_init(node); } -bool queue_empty(const queue_t* queue) { - return !queue_enqueued(&queue->head); +bool queue_is_empty(const queue_t* queue) { + return !queue_is_enqueued(&queue->head); } -bool queue_enqueued(const queue_node_t* node) { +bool queue_is_enqueued(const queue_node_t* node) { return node->prev != node; } diff --git a/src/queue.h b/src/queue.h index dc7aad9..f982572 100644 --- a/src/queue.h +++ b/src/queue.h @@ -24,11 +24,11 @@ WEPOLL_INTERNAL queue_node_t* queue_last(const queue_t* queue); WEPOLL_INTERNAL void queue_prepend(queue_t* queue, queue_node_t* node); WEPOLL_INTERNAL void queue_append(queue_t* queue, queue_node_t* node); -WEPOLL_INTERNAL void queue_move_first(queue_t* queue, queue_node_t* node); -WEPOLL_INTERNAL void queue_move_last(queue_t* queue, queue_node_t* node); +WEPOLL_INTERNAL void queue_move_to_start(queue_t* queue, queue_node_t* node); +WEPOLL_INTERNAL void queue_move_to_end(queue_t* queue, queue_node_t* node); WEPOLL_INTERNAL void queue_remove(queue_node_t* node); -WEPOLL_INTERNAL bool queue_empty(const queue_t* queue); -WEPOLL_INTERNAL bool queue_enqueued(const queue_node_t* node); +WEPOLL_INTERNAL bool queue_is_empty(const queue_t* queue); +WEPOLL_INTERNAL bool queue_is_enqueued(const queue_node_t* node); #endif /* WEPOLL_QUEUE_H_ */ From 8223744d7bd0be8fdb76e08cf2793b475f9dee6d Mon Sep 17 00:00:00 2001 From: Bert Belder Date: Tue, 12 May 2020 03:46:44 +0200 Subject: [PATCH 4/5] port: drop '_handle' suffix from 'port_(un)register_socket()' function names --- src/port.c | 10 +++++----- src/port.h | 10 +++++----- src/sock.c | 4 ++-- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/port.c b/src/port.c index 6ecbb09..7dd6eee 100644 --- a/src/port.c +++ b/src/port.c @@ -354,9 +354,9 @@ int port_ctl(port_state_t* port_state, return result; } -int port_register_socket_handle(port_state_t* port_state, - sock_state_t* sock_state, - SOCKET socket) { +int port_register_socket(port_state_t* port_state, + sock_state_t* sock_state, + SOCKET socket) { if (tree_add(&port_state->sock_tree, sock_state_to_tree_node(sock_state), socket) < 0) @@ -364,8 +364,8 @@ int port_register_socket_handle(port_state_t* port_state, return 0; } -void port_unregister_socket_handle(port_state_t* port_state, - sock_state_t* sock_state) { +void port_unregister_socket(port_state_t* port_state, + sock_state_t* sock_state) { tree_del(&port_state->sock_tree, sock_state_to_tree_node(sock_state)); } diff --git a/src/port.h b/src/port.h index 93f45d8..244de13 100644 --- a/src/port.h +++ b/src/port.h @@ -24,11 +24,11 @@ WEPOLL_INTERNAL int port_ctl(port_state_t* port_state, SOCKET sock, struct epoll_event* ev); -WEPOLL_INTERNAL int port_register_socket_handle(port_state_t* port_state, - sock_state_t* sock_state, - SOCKET socket); -WEPOLL_INTERNAL void port_unregister_socket_handle(port_state_t* port_state, - sock_state_t* sock_state); +WEPOLL_INTERNAL int port_register_socket(port_state_t* port_state, + sock_state_t* sock_state, + SOCKET socket); +WEPOLL_INTERNAL void port_unregister_socket(port_state_t* port_state, + sock_state_t* sock_state); WEPOLL_INTERNAL sock_state_t* port_find_socket(port_state_t* port_state, SOCKET socket); diff --git a/src/sock.c b/src/sock.c index e4539c3..342a1a8 100644 --- a/src/sock.c +++ b/src/sock.c @@ -90,7 +90,7 @@ sock_state_t* sock_new(port_state_t* port_state, SOCKET socket) { tree_node_init(&sock_state->tree_node); queue_node_init(&sock_state->queue_node); - if (port_register_socket_handle(port_state, sock_state, socket) < 0) + if (port_register_socket(port_state, sock_state, socket) < 0) goto err2; return sock_state; @@ -111,7 +111,7 @@ static int sock__delete(port_state_t* port_state, sock__cancel_poll(sock_state); port_cancel_socket_update(port_state, sock_state); - port_unregister_socket_handle(port_state, sock_state); + port_unregister_socket(port_state, sock_state); sock_state->delete_pending = true; } From 8dc61151279e76354c18a6c8bb48c9fa19691745 Mon Sep 17 00:00:00 2001 From: Bert Belder Date: Tue, 12 May 2020 05:06:23 +0200 Subject: [PATCH 5/5] ws: add workaround for Komodia based LSPs that break SIO_BASE_HANDLE --- src/ws.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 48 insertions(+), 9 deletions(-) diff --git a/src/ws.c b/src/ws.c index 83de065..2a97c96 100644 --- a/src/ws.c +++ b/src/ws.c @@ -1,9 +1,14 @@ #include #include "error.h" +#include "util.h" #include "win.h" #include "ws.h" +#ifndef SIO_BSP_HANDLE_POLL +#define SIO_BSP_HANDLE_POLL 0x4800001D +#endif + #ifndef SIO_BASE_HANDLE #define SIO_BASE_HANDLE 0x48000022 #endif @@ -19,20 +24,54 @@ int ws_global_init(void) { return 0; } -SOCKET ws_get_base_socket(SOCKET socket) { - SOCKET base_socket; +static inline SOCKET ws__ioctl_get_bsp_socket(SOCKET socket, DWORD ioctl) { + SOCKET bsp_socket; DWORD bytes; if (WSAIoctl(socket, - SIO_BASE_HANDLE, + ioctl, NULL, 0, - &base_socket, - sizeof base_socket, + &bsp_socket, + sizeof bsp_socket, &bytes, NULL, - NULL) == SOCKET_ERROR) - return_map_error(INVALID_SOCKET); - - return base_socket; + NULL) != SOCKET_ERROR) + return bsp_socket; + else + return INVALID_SOCKET; +} + +SOCKET ws_get_base_socket(SOCKET socket) { + SOCKET base_socket; + DWORD error; + + for (;;) { + base_socket = ws__ioctl_get_bsp_socket(socket, SIO_BASE_HANDLE); + if (base_socket != INVALID_SOCKET) + return base_socket; + + error = GetLastError(); + if (error == WSAENOTSOCK) + return_set_error(INVALID_SOCKET, error); + + /* clang-format off */ + /* Even though Microsoft documentation clearly states that LSPs should + * never intercept the `SIO_BASE_HANDLE` ioctl [1], Komodia based LSPs do + * so anyway, breaking it, with the apparent intention of preventing LSP + * bypass [2]. Fortunately they don't handle `SIO_BSP_HANDLE_POLL`, which + * we can use to obtain the socket associated with the next protocol chain + * entry. If this succeeds, loop around and call `SIO_BASE_HANDLE` again + * with the retrieved BSP socket to be sure that we actually got all the + * way to the base. + * [1] https://docs.microsoft.com/en-us/windows/win32/winsock/winsock-ioctls + * [2] https://www.komodia.com/newwiki/index.php?title=Komodia%27s_Redirector_bug_fixes#Version_2.2.2.6 + */ + /* clang-format on */ + base_socket = ws__ioctl_get_bsp_socket(socket, SIO_BSP_HANDLE_POLL); + if (base_socket != INVALID_SOCKET && base_socket != socket) + socket = base_socket; + else + return_set_error(INVALID_SOCKET, error); + } }