From 35a30072f43aaf815929b431e732f269720c67a7 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 Closes IDF-504 --- 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 40b2cc038f..18fe1a9938 100644 --- a/components/mdns/mdns.c +++ b/components/mdns/mdns.c @@ -3819,7 +3819,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); @@ -3856,6 +3866,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() { @@ -3863,6 +3877,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; @@ -3870,12 +3888,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 9efd23ae70..a7383c03d5 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 {