Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lws_vhost_destroy() crashes after lws_create_adopt_udp() fail #3225

Open
millikk opened this issue Sep 18, 2024 · 1 comment
Open

lws_vhost_destroy() crashes after lws_create_adopt_udp() fail #3225

millikk opened this issue Sep 18, 2024 · 1 comment

Comments

@millikk
Copy link
Contributor

millikk commented Sep 18, 2024

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
#include <stdio.h>
#include <arpa/inet.h>
#include <libwebsockets.h>

// 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
Copy link
Member

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);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants