Skip to content

Commit 3be7f28

Browse files
authored
Merge pull request #986 from gabsuren/fix/websocket_race_on_stop
fix(websocket): fix use-after-free race condition in task stop sequence (IDFGH-17044)
2 parents d5db1fc + 3d37eaa commit 3be7f28

File tree

2 files changed

+27
-6
lines changed

2 files changed

+27
-6
lines changed

components/esp_websocket_client/esp_websocket_client.c

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -478,25 +478,43 @@ static void destroy_and_free_resources(esp_websocket_client_handle_t client)
478478
{
479479
if (client->event_handle) {
480480
esp_event_loop_delete(client->event_handle);
481+
client->event_handle = NULL;
481482
}
482483
if (client->if_name) {
483484
free(client->if_name);
485+
client->if_name = NULL;
484486
}
485487
esp_websocket_client_destroy_config(client);
486488
if (client->transport_list) {
487489
esp_transport_list_destroy(client->transport_list);
488490
client->transport_list = NULL;
489491
client->transport = NULL;
490492
}
491-
vSemaphoreDelete(client->lock);
493+
if (client->lock) {
494+
vSemaphoreDelete(client->lock);
495+
client->lock = NULL;
496+
}
492497
#ifdef CONFIG_ESP_WS_CLIENT_SEPARATE_TX_LOCK
493-
vSemaphoreDelete(client->tx_lock);
498+
if (client->tx_lock) {
499+
vSemaphoreDelete(client->tx_lock);
500+
client->tx_lock = NULL;
501+
}
494502
#endif
495-
free(client->tx_buffer);
496-
free(client->rx_buffer);
497-
free(client->errormsg_buffer);
503+
if (client->tx_buffer) {
504+
free(client->tx_buffer);
505+
client->tx_buffer = NULL;
506+
}
507+
if (client->rx_buffer) {
508+
free(client->rx_buffer);
509+
client->rx_buffer = NULL;
510+
}
511+
if (client->errormsg_buffer) {
512+
free(client->errormsg_buffer);
513+
client->errormsg_buffer = NULL;
514+
}
498515
if (client->status_bits) {
499516
vEventGroupDelete(client->status_bits);
517+
client->status_bits = NULL;
500518
}
501519
free(client);
502520
client = NULL;
@@ -1374,10 +1392,11 @@ static void esp_websocket_client_task(void *pv)
13741392

13751393
esp_websocket_client_dispatch_event(client, WEBSOCKET_EVENT_FINISH, NULL, 0);
13761394
esp_transport_close(client->transport);
1377-
xEventGroupSetBits(client->status_bits, STOPPED_BIT);
13781395
client->state = WEBSOCKET_STATE_UNKNOW;
13791396
if (client->selected_for_destroying == true) {
13801397
destroy_and_free_resources(client);
1398+
} else {
1399+
xEventGroupSetBits(client->status_bits, STOPPED_BIT);
13811400
}
13821401
vTaskDelete(NULL);
13831402
}

components/esp_websocket_client/include/esp_websocket_client.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -236,6 +236,7 @@ esp_err_t esp_websocket_client_stop(esp_websocket_client_handle_t client);
236236
*
237237
* Notes:
238238
* - Cannot be called from the websocket event handler
239+
* - This function cannot be called if `esp_websocket_client_destroy_on_exit` was used for the same handle.
239240
*
240241
* @param[in] client The client
241242
*
@@ -248,6 +249,7 @@ esp_err_t esp_websocket_client_destroy(esp_websocket_client_handle_t client);
248249
*
249250
* Notes:
250251
* - After event loop finished, client handle would be dangling and should never be used
252+
* - This function is mutually exclusive with `esp_websocket_client_destroy`. Do not call `esp_websocket_client_destroy` manually if this API is used.
251253
*
252254
* @param[in] client The client
253255
*

0 commit comments

Comments
 (0)