Merge branch 'bugfix/mdns_add_remove_multiple_srv_master_3.3' into 'release/v3.3'

mdns: fix possible crash if tx packet contained answer to removed service (backport 3.3)

See merge request idf/esp-idf!4480

components/mdns/mdns.c

@@ -1452,20 +1452,13 @@ static void _mdns_pcb_send_bye(tcpip_adapter_if_t tcpip_if, mdns_ip_protocol_t i
 }
 
 /**
- * @brief  Send probe for particular services on particular PCB
+ * @brief  Send probe for additional services on particular PCB
  */
-static void _mdns_init_pcb_probe(tcpip_adapter_if_t tcpip_if, mdns_ip_protocol_t ip_protocol, mdns_srv_item_t ** services, size_t len, bool probe_ip)
+static void _mdns_init_pcb_probe_new_service(tcpip_adapter_if_t tcpip_if, mdns_ip_protocol_t ip_protocol, mdns_srv_item_t ** services, size_t len, bool probe_ip)
 {
     mdns_pcb_t * pcb = &_mdns_server->interfaces[tcpip_if].pcbs[ip_protocol];
     size_t services_final_len = len;
 
-    _mdns_clear_pcb_tx_queue_head(tcpip_if, ip_protocol);
-
-    if (_str_null_or_empty(_mdns_server->hostname)) {
-        pcb->state = PCB_RUNNING;
-        return;
-    }
-
     if (PCB_STATE_IS_PROBING(pcb)) {
         services_final_len += pcb->probe_services_len;
     }
@@ -1510,6 +1503,50 @@ static void _mdns_init_pcb_probe(tcpip_adapter_if_t tcpip_if, mdns_ip_protocol_t
     pcb->state = PCB_PROBE_1;
 }
 
+/**
+ * @brief  Send probe for particular services on particular PCB
+ *
+ * Tests possible duplication on probing service structure and probes only for new entries.
+ * - If pcb probing then add only non-probing services and restarts probing
+ * - If pcb not probing, run probing for all specified services
+ */
+static void _mdns_init_pcb_probe(tcpip_adapter_if_t tcpip_if, mdns_ip_protocol_t ip_protocol, mdns_srv_item_t ** services, size_t len, bool probe_ip)
+{
+    mdns_pcb_t * pcb = &_mdns_server->interfaces[tcpip_if].pcbs[ip_protocol];
+
+    _mdns_clear_pcb_tx_queue_head(tcpip_if, ip_protocol);
+
+    if (_str_null_or_empty(_mdns_server->hostname)) {
+        pcb->state = PCB_RUNNING;
+        return;
+    }
+
+    if (PCB_STATE_IS_PROBING(pcb)) {
+        // Looking for already probing services to resolve duplications
+        mdns_srv_item_t * new_probe_services[len];
+        int new_probe_service_len = 0;
+        bool found;
+        for (int j=0; j < len; ++j) {
+            found = false;
+            for (int i=0; i < pcb->probe_services_len; ++i) {
+                if (pcb->probe_services[i] == services[j]) {
+                    found = true;
+                    break;
+                }
+            }
+            if (!found) {
+                new_probe_services[new_probe_service_len++] = services[j];
+            }
+        }
+        // init probing for newly added services
+        _mdns_init_pcb_probe_new_service(tcpip_if, ip_protocol,
+                                         new_probe_service_len?new_probe_services:NULL, new_probe_service_len, probe_ip);
+    } else {
+        // not probing, so init for all services
+        _mdns_init_pcb_probe_new_service(tcpip_if, ip_protocol, services, len, probe_ip);
+    }
+}
+
 /**
  * @brief  Restart the responder on particular PCB
  */
@@ -1863,6 +1900,9 @@ static void _mdns_dealloc_scheduled_service_answers(mdns_out_answer_t ** destina
  */
 static void _mdns_remove_scheduled_service_packets(mdns_service_t * service)
 {
+    if (!service) {
+        return;
+    }
     mdns_tx_packet_t * p = NULL;
     mdns_tx_packet_t * q = _mdns_server->tx_queue_head;
     while (q) {
@@ -1951,7 +1991,6 @@ static void _mdns_free_service(mdns_service_t * service)
     if (!service) {
         return;
     }
-    _mdns_remove_scheduled_service_packets(service);
     free((char *)service->instance);
     free((char *)service->service);
     free((char *)service->proto);
@@ -3781,6 +3820,7 @@ static void _mdns_execute_action(mdns_action_t * action)
         if (_mdns_server->services == action->data.srv_del.service) {
             _mdns_server->services = a->next;
             _mdns_send_bye(&a, 1, false);
+            _mdns_remove_scheduled_service_packets(a->service);
             _mdns_free_service(a->service);
             free(a);
         } else {
@@ -3791,6 +3831,7 @@ static void _mdns_execute_action(mdns_action_t * action)
                 mdns_srv_item_t * b = a->next;
                 a->next = a->next->next;
                 _mdns_send_bye(&b, 1, false);
+                _mdns_remove_scheduled_service_packets(b->service);
                 _mdns_free_service(b->service);
                 free(b);
             }
@@ -3804,6 +3845,7 @@ static void _mdns_execute_action(mdns_action_t * action)
         while (a) {
             mdns_srv_item_t * s = a;
             a = a->next;
+            _mdns_remove_scheduled_service_packets(s->service);
             _mdns_free_service(s->service);
             free(s);
         }
@@ -3819,7 +3861,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 +3908,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 +3919,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 +3930,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;
@@ -4015,9 +4075,7 @@ static esp_err_t _mdns_service_task_start()
  */
 static esp_err_t _mdns_service_task_stop()
 {
-    MDNS_SERVICE_LOCK();
     _mdns_stop_timer();
-    MDNS_SERVICE_UNLOCK();
     if (_mdns_service_task_handle) {
         mdns_action_t action;
         mdns_action_t * a = &action;
@@ -4086,12 +4144,6 @@ esp_err_t mdns_init()
         goto free_lock;
     }
 
-    if (_mdns_service_task_start()) {
-        //service start failed!
-        err = ESP_FAIL;
-        goto free_all;
-    }
-
     uint8_t i;
     ip6_addr_t tmp_addr6;
     tcpip_adapter_ip_info_t if_ip_info;
@@ -4105,9 +4157,19 @@ esp_err_t mdns_init()
         }
     }
 
+    if (_mdns_service_task_start()) {
+        //service start failed!
+        err = ESP_FAIL;
+        goto free_all_and_disable_pcbs;
+    }
+
     return ESP_OK;
 
-free_all:
+free_all_and_disable_pcbs:
+    for (i=0; i<TCPIP_ADAPTER_IF_MAX; i++) {
+        _mdns_disable_pcb(i, MDNS_IP_PROTOCOL_V6);
+        _mdns_disable_pcb(i, MDNS_IP_PROTOCOL_V4);
+    }
     vQueueDelete(_mdns_server->action_queue);
 free_lock:
     vSemaphoreDelete(_mdns_server->lock);

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 {