lws_vhost_destroy() crashes after lws_create_adopt_udp() fail

[millikk]

Libwebsockets version: 4.3.2

OS: Linux

Brief: If the lws_create_adopt_udp() function fails to bind a socket and returns NULL, then subsequent call of the lws_vhost_destroy() leads to crash

Details:

1. When the lws_create_adopt_udp() starts, it creates a wsi object and saves it in the vhost->vh_awaiting_socket_owner list (see the __lws_adopt_descriptor_vhost1() function). Then if the lws_create_adopt_udp() fails to bind a socket, it doen't destroy the wsi object, but leaves it in the vhost->vh_awaiting_socket_owner list.
2. When the lws_vhost_destroy() is called, it starts closing all its wsi objects in the __lws_vhost_destroy_pt_wsi_dieback_start() function. But when the last wsi is destroyed, the whole vhost is also destroyed, look at the call chain: __lws_vhost_destroy_pt_wsi_dieback_start() -> lws_close_free_wsi() -> __lws_close_free_wsi() -> __lws_close_free_wsi_final() -> __lws_free_wsi() -> __lws_vhost_unbind_wsi() -> __lws_vhost_destroy2(). After return from this chain the lws_vhost_destroy() function accesses to the already destroyed vh object and calls __lws_vhost_destroy2() again on it

How to reproduce

Here is the minimal code snippet to reproduce the problem. To trigger the error in the lws_create_adopt_udp() we firstly bind socket to the port 55505 and then call lws_create_adopt_udp() with the same port number. Then subsequent call of the lws_vhost_destroy(vhost); leads to crash. Also if we comment out the line lws_vhost_destroy(vhost); // <- here should be a crash, we will get memory leaks (see the memory sanitizer output). To build the snippet, run:

gcc -o main ./main.c -I ./lws/include -I ./tls/include -L ./lws/lib -L ./tls/lib -lwebsockets -lmbedtls -lmbedx509 -lmbedcrypto -fsanitize=address

where the ./lws/ is a folder with libwebsockets, and the ./tls/ is a folder with tls backend (in my case it's mbedtls

Minimal snippet

```c #include #include #include // a dummy callback static int on_event(struct lws * wsi, enum lws_callback_reasons reason, void * user, void * data, size_t size) { (void) wsi; (void) reason; (void) user; (void) data; (void) size; return 0; } int main(int argc, char * argv[]) { (void) argc; (void) argv; // create lws context struct lws_context_creation_info ctx_info = {}; ctx_info.options = LWS_SERVER_OPTION_EXPLICIT_VHOSTS; ctx_info.port = CONTEXT_PORT_NO_LISTEN_SERVER; struct lws_context * ctx = lws_create_context(&ctx_info); if (!ctx) { printf("failed to create context\n"); return EXIT_FAILURE; } struct lws_protocols proto = {}; struct lws_protocols * protos_list[2] = {}; proto.name = "proto"; proto.callback = on_event; protos_list[0] = &proto; // create lws vhost struct lws_context_creation_info vh_info = {}; vh_info.port = CONTEXT_PORT_NO_LISTEN_SERVER; vh_info.pprotocols = (const struct lws_protocols **) protos_list; struct lws_vhost * vhost = lws_create_vhost(ctx, &vh_info); if (!vhost) { printf("failed to create vhost\n"); lws_context_destroy(ctx); return EXIT_FAILURE; } // create socket and bind it to the port to trigger error in the lws_create_adopt_udp() const int port = 55505; int sock = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP); if (sock == -1) { printf("failed to create socket\n"); lws_vhost_destroy(vhost); lws_context_destroy(ctx); return EXIT_FAILURE; } struct sockaddr_in sockaddr = {}; sockaddr.sin_family = AF_INET; sockaddr.sin_port = htons(port); inet_pton(AF_INET, "127.0.0.1", &sockaddr.sin_addr); if (bind(sock, (struct sockaddr *) &sockaddr, sizeof(sockaddr)) != 0) { printf("failed to create socket\n"); lws_vhost_destroy(vhost); lws_context_destroy(ctx); return EXIT_FAILURE; } // adopt the udp with the same port struct lws * wsi = lws_create_adopt_udp(vhost, "127.0.0.1", port, LWS_CAUDP_BIND, proto.name, NULL, NULL, NULL, NULL, NULL ); if (!wsi) { printf("failed to adopt udp\n"); lws_vhost_destroy(vhost); // <- here should be a crash lws_context_destroy(ctx); return EXIT_FAILURE; } lws_vhost_destroy(vhost); lws_context_destroy(ctx); return EXIT_SUCCESS; } ```

[lws-team]

Not sure this is perfect, but it should solve the crash

diff --git a/lib/core-net/vhost.c b/lib/core-net/vhost.c
index a5311494..b5d9092c 100644
--- a/lib/core-net/vhost.c
+++ b/lib/core-net/vhost.c
@@ -1236,8 +1236,7 @@ __lws_vhost_destroy_pt_wsi_dieback_start(struct lws_vhost *vh)
                if (w->tsi == tsi) {

                        lwsl_vhost_debug(vh, "closing aso");
-                       lws_close_free_wsi(w, LWS_CLOSE_STATUS_NOSTATUS,
-                                          "awaiting skt");
+                       lws_wsi_close(w, LWS_TO_KILL_ASYNC);
                }

        } lws_end_foreach_dll_safe(d, d1);