Browse Source

TT#107157 fix LWS HTTP handling crash

Sequence of events:

1) HTTP request is being handled in worker thread by calling the handler
   func() from within websocket_process().
2) Handler func generates output, queues it up, and requests a
   `writeable` callback from within websocket_write_raw().
3) Main LWS thread triggers writeable callback and calls
   websocket_dequeue().
4) Output is given to LWS still within the main LWS thread, and finally
   lws_http_transaction_completed() is called to release the connection
   and ready it for the next HTTP connection.
5) LWS internally cleans up the connection and frees the user context
   (our `wc` struct).
6) The worker thread wakes up and continues to use the now invalid `wc`
   in order to clean up after it has done its job. Boom.

The solution is to handle the `drop protocol` callback, which is
triggered by LWS in the main LWS thread in step 4 from within
lws_http_transaction_completed(). We call our own connection cleanup
function websocket_conn_cleanup() which blocks until all jobs are
removed from `wc` (step 6) and only then continue, allowing LWS to
safely free the struct.

Change-Id: I596a98e9b552a96aef259f4523f16fa63c287ef4
pull/1164/head
Richard Fuchs 5 years ago
parent
commit
9653546f7c
1 changed files with 13 additions and 5 deletions
  1. +13
    -5
      daemon/websocket.c

+ 13
- 5
daemon/websocket.c View File

@ -194,6 +194,8 @@ static void websocket_process(void *p, void *up) {
gettimeofday(&rtpe_now, NULL); gettimeofday(&rtpe_now, NULL);
const char *err = wm->func(wm); const char *err = wm->func(wm);
// this may trigger a cleanup/free in another thread, which will then block until our
// job count has been decremented
websocket_message_free(&wm); websocket_message_free(&wm);
mutex_lock(&wc->lock); mutex_lock(&wc->lock);
@ -215,6 +217,7 @@ static int websocket_dequeue(struct websocket_conn *wc) {
mutex_lock(&wc->lock); mutex_lock(&wc->lock);
struct websocket_output *wo; struct websocket_output *wo;
struct lws *wsi = wc->wsi;
while ((wo = g_queue_pop_head(&wc->output_q))) { while ((wo = g_queue_pop_head(&wc->output_q))) {
// used buffer slot? // used buffer slot?
if (wo->str) { if (wo->str) {
@ -227,7 +230,7 @@ static int websocket_dequeue(struct websocket_conn *wc) {
else else
ilogs(http, LOG_DEBUG, "Writing back to LWS: '%.*s'", ilogs(http, LOG_DEBUG, "Writing back to LWS: '%.*s'",
(int) to_send, wo->str->str + wo->str_done); (int) to_send, wo->str->str + wo->str_done);
size_t ret = lws_write(wc->wsi, (unsigned char *) wo->str->str + wo->str_done,
size_t ret = lws_write(wsi, (unsigned char *) wo->str->str + wo->str_done,
to_send, wo->protocol); to_send, wo->protocol);
if (ret != to_send) if (ret != to_send)
ilogs(http, LOG_ERR, "Invalid LWS write: %lu != %lu", ilogs(http, LOG_ERR, "Invalid LWS write: %lu != %lu",
@ -243,11 +246,11 @@ static int websocket_dequeue(struct websocket_conn *wc) {
} }
g_queue_push_tail(&wc->output_q, websocket_output_new()); g_queue_push_tail(&wc->output_q, websocket_output_new());
mutex_unlock(&wc->lock);
int ret = 0; int ret = 0;
if (is_http) if (is_http)
ret = lws_http_transaction_completed(wc->wsi);
mutex_unlock(&wc->lock);
ret = lws_http_transaction_completed(wsi); // may destroy `wc`
return ret; return ret;
} }
@ -637,10 +640,15 @@ static int websocket_http(struct lws *wsi, enum lws_callback_reasons reason, voi
#endif #endif
#if LWS_LIBRARY_VERSION_MAJOR >= 3 #if LWS_LIBRARY_VERSION_MAJOR >= 3
case LWS_CALLBACK_ADD_HEADERS: case LWS_CALLBACK_ADD_HEADERS:
case LWS_CALLBACK_HTTP_DROP_PROTOCOL:
case LWS_CALLBACK_EVENT_WAIT_CANCELLED: // ? case LWS_CALLBACK_EVENT_WAIT_CANCELLED: // ?
#endif #endif
break; break;
#if LWS_LIBRARY_VERSION_MAJOR >= 3
case LWS_CALLBACK_HTTP_DROP_PROTOCOL:
ilogs(http, LOG_DEBUG, "HTTP connection reset %p", wsi);
websocket_conn_cleanup(user);
break;
#endif
case LWS_CALLBACK_FILTER_PROTOCOL_CONNECTION: case LWS_CALLBACK_FILTER_PROTOCOL_CONNECTION:
return -1; // disallow non supported websocket protocols return -1; // disallow non supported websocket protocols
case LWS_CALLBACK_GET_THREAD_ID: case LWS_CALLBACK_GET_THREAD_ID:


Loading…
Cancel
Save