From 67051a286ba60a01d4755c3682129153c2f95953 Mon Sep 17 00:00:00 2001
From: David Cermak <cermak@espressif.com>
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 {