From 03006a109b04b75b01b7c3c1c27f04cd9e72ad83 Mon Sep 17 00:00:00 2001 From: David Cermak Date: Mon, 25 Feb 2019 14:29:39 +0100 Subject: [PATCH] mdns: fix possible crash when packet scheduled to transmit contained service which might have been already removed packets scheduled to transmit are pushed to action queue and removed from tx_queue_head structure, which is searched for all remaining services and while service is removed, then service questions/asnwers are also removed from this structure. This update fixes possible crash when packet is pushed to action queue, and when service is removed, its answers are removed from tx_queue_head, but not from action queue. this could lead to a crash when the packet is poped from action queue containing questions/answers to already removed (freed) service --- components/mdns/mdns.c | 24 ++++++++++++++++--- .../mdns/private_include/mdns_private.h | 1 + 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/components/mdns/mdns.c b/components/mdns/mdns.c index 179a06da6..c6dfebef4 100644 --- a/components/mdns/mdns.c +++ b/components/mdns/mdns.c @@ -3801,7 +3801,17 @@ static void _mdns_execute_action(mdns_action_t * action) _mdns_search_finish(action->data.search_add.search); break; case ACTION_TX_HANDLE: - _mdns_tx_handle_packet(action->data.tx_handle.packet); + { + mdns_tx_packet_t * p = _mdns_server->tx_queue_head; + // packet to be handled should be at tx head, but must be consistent with the one pushed to action queue + if (p && p==action->data.tx_handle.packet && p->queued) { + p->queued = false; // clearing, as the packet might be reused (pushed and transmitted again) + _mdns_server->tx_queue_head = p->next; + _mdns_tx_handle_packet(p); + } else { + ESP_LOGD(TAG, "Skipping transmit of an unexpected packet!"); + } + } break; case ACTION_RX_HANDLE: mdns_parse_packet(action->data.rx_handle.packet); @@ -3838,6 +3848,10 @@ static esp_err_t _mdns_send_search_action(mdns_action_type_t type, mdns_search_o /** * @brief Called from timer task to run mDNS responder + * + * periodically checks first unqueued packet (from tx head). + * if it is scheduled to be transmitted, then pushes the packet to action queue to be handled. + * */ static void _mdns_scheduler_run() { @@ -3845,6 +3859,10 @@ static void _mdns_scheduler_run() mdns_tx_packet_t * p = _mdns_server->tx_queue_head; mdns_action_t * action = NULL; + // find first unqueued packet + while (p && p->queued) { + p = p->next; + } if (!p) { MDNS_SERVICE_UNLOCK(); return; @@ -3852,12 +3870,12 @@ static void _mdns_scheduler_run() if ((int32_t)(p->send_at - (xTaskGetTickCount() * portTICK_PERIOD_MS)) < 0) { action = (mdns_action_t *)malloc(sizeof(mdns_action_t)); if (action) { - _mdns_server->tx_queue_head = p->next; action->type = ACTION_TX_HANDLE; action->data.tx_handle.packet = p; + p->queued = true; if (xQueueSend(_mdns_server->action_queue, &action, (portTickType)0) != pdPASS) { free(action); - _mdns_server->tx_queue_head = p; + p->queued = false; } } else { HOOK_MALLOC_FAILED; diff --git a/components/mdns/private_include/mdns_private.h b/components/mdns/private_include/mdns_private.h index cbdf830cd..da5162348 100644 --- a/components/mdns/private_include/mdns_private.h +++ b/components/mdns/private_include/mdns_private.h @@ -289,6 +289,7 @@ typedef struct mdns_tx_packet_s { mdns_out_answer_t * answers; mdns_out_answer_t * servers; mdns_out_answer_t * additional; + bool queued; } mdns_tx_packet_t; typedef struct {