fix(mdns): Fix API races while adding subtypes for services

Fixes **API race issue** (described in 8a69050) for API
mdns_service_subtype_add_for_host()

Author: david-cermak

components/mdns/mdns.c

@@ -5111,9 +5111,6 @@ static void _mdns_free_action(mdns_action_t *action)
     case ACTION_SERVICE_INSTANCE_SET:
         free(action->data.srv_instance.instance);
         break;
-    case ACTION_SERVICE_SUBTYPE_ADD:
-        free(action->data.srv_subtype_add.subtype);
-        break;
     case ACTION_SEARCH_ADD:
     //fallthrough
     case ACTION_SEARCH_SEND:
@@ -5155,9 +5152,6 @@ static void _mdns_free_action(mdns_action_t *action)
 static void _mdns_execute_action(mdns_action_t *action)
 {
     mdns_srv_item_t *a = NULL;
-    mdns_service_t *service;
-    char *subtype;
-    mdns_subtype_t *subtype_item;
 
     switch (action->type) {
     case ACTION_SYSTEM_EVENT:
@@ -5187,19 +5181,6 @@ static void _mdns_execute_action(mdns_action_t *action)
         action->data.srv_instance.service->service->instance = action->data.srv_instance.instance;
         _mdns_probe_all_pcbs(&action->data.srv_instance.service, 1, false, false);
 
-        break;
-    case ACTION_SERVICE_SUBTYPE_ADD:
-        service = action->data.srv_subtype_add.service->service;
-        subtype = action->data.srv_subtype_add.subtype;
-        subtype_item = (mdns_subtype_t *)malloc(sizeof(mdns_subtype_t));
-        if (!subtype_item) {
-            HOOK_MALLOC_FAILED;
-            _mdns_free_action(action);
-            return;
-        }
-        subtype_item->subtype = subtype;
-        subtype_item->next = service->subtype;
-        service->subtype = subtype_item;
         break;
     case ACTION_SERVICES_CLEAR:
         _mdns_send_final_bye(false);
@@ -6213,6 +6194,7 @@ esp_err_t mdns_service_txt_item_set_for_host_with_explicit_value_len(const char
     MDNS_SERVICE_LOCK();
     esp_err_t ret = ESP_OK;
     char *value = NULL;
+    mdns_txt_linked_item_t *new_txt = NULL;
     const char *hostname = host ? host : _mdns_server->hostname;
     ESP_GOTO_ON_FALSE(_mdns_server && _mdns_server->services && !_str_null_or_empty(service) && !_str_null_or_empty(proto) && !_str_null_or_empty(key) &&
                       !((!value_arg && value_len)), ESP_ERR_INVALID_ARG, err, TAG, "Invalid state or arguments");
@@ -6223,7 +6205,7 @@ esp_err_t mdns_service_txt_item_set_for_host_with_explicit_value_len(const char
     mdns_service_t *srv = s->service;
     if (value_len > 0) {
         value = (char *) malloc(value_len);
-        ESP_GOTO_ON_FALSE(s, ESP_ERR_NO_MEM, out_of_mem, TAG, "Out of memory");
+        ESP_GOTO_ON_FALSE(value, ESP_ERR_NO_MEM, out_of_mem, TAG, "Out of memory");
         memcpy(value, value_arg, value_len);
     } else {
         value_len = 0;
@@ -6239,14 +6221,14 @@ esp_err_t mdns_service_txt_item_set_for_host_with_explicit_value_len(const char
         txt = txt->next;
     }
     if (!txt) {
-        txt = (mdns_txt_linked_item_t *)malloc(sizeof(mdns_txt_linked_item_t));
-        ESP_GOTO_ON_FALSE(s, ESP_ERR_NO_MEM, out_of_mem, TAG, "Out of memory");
-        txt->key = strdup(key);
-        ESP_GOTO_ON_FALSE(txt->key, ESP_ERR_NO_MEM, out_of_mem, TAG, "Out of memory");
-        txt->value = value;
-        txt->value_len = value_len;
-        txt->next = srv->txt;
-        srv->txt = txt;
+        new_txt = (mdns_txt_linked_item_t *)malloc(sizeof(mdns_txt_linked_item_t));
+        ESP_GOTO_ON_FALSE(new_txt, ESP_ERR_NO_MEM, out_of_mem, TAG, "Out of memory");
+        new_txt->key = strdup(key);
+        ESP_GOTO_ON_FALSE(new_txt->key, ESP_ERR_NO_MEM, out_of_mem, TAG, "Out of memory");
+        new_txt->value = value;
+        new_txt->value_len = value_len;
+        new_txt->next = srv->txt;
+        srv->txt = new_txt;
     }
 
     _mdns_announce_all_pcbs(&s, 1, false);
@@ -6258,6 +6240,7 @@ esp_err_t mdns_service_txt_item_set_for_host_with_explicit_value_len(const char
     MDNS_SERVICE_UNLOCK();
     HOOK_MALLOC_FAILED;
     free(value);
+    free(new_txt);
     return ret;
 }
 
@@ -6346,46 +6329,33 @@ esp_err_t mdns_service_subtype_add_for_host(const char *instance_name, const cha
         const char *hostname, const char *subtype)
 {
     MDNS_SERVICE_LOCK();
-    if (!_mdns_server || !_mdns_server->services || _str_null_or_empty(service) || _str_null_or_empty(proto) ||
-            _str_null_or_empty(subtype)) {
-        MDNS_SERVICE_UNLOCK();
-        return ESP_ERR_INVALID_ARG;
-    }
+    esp_err_t ret = ESP_OK;
+    ESP_GOTO_ON_FALSE(_mdns_server && _mdns_server->services && !_str_null_or_empty(service) && !_str_null_or_empty(proto) &&
+                      !_str_null_or_empty(subtype), ESP_ERR_INVALID_ARG, err, TAG, "Invalid state or arguments");
+
     mdns_srv_item_t *s = _mdns_get_service_item_instance(instance_name, service, proto, hostname);
-    MDNS_SERVICE_UNLOCK();
-    if (!s) {
-        return ESP_ERR_NOT_FOUND;
-    }
+    ESP_GOTO_ON_FALSE(s, ESP_ERR_NOT_FOUND, err, TAG, "Service doesn't exist");
 
     mdns_subtype_t *srv_subtype = s->service->subtype;
     while (srv_subtype) {
-        if (strcmp(srv_subtype->subtype, subtype) == 0) {
-            // The same subtype has already been added
-            return ESP_ERR_INVALID_ARG;
-        }
+        ESP_GOTO_ON_FALSE(strcmp(srv_subtype->subtype, subtype) != 0, ESP_ERR_INVALID_ARG, err, TAG, "The same subtype has already been added");
         srv_subtype = srv_subtype->next;
     }
 
-    mdns_action_t *action = (mdns_action_t *)malloc(sizeof(mdns_action_t));
-    if (!action) {
-        HOOK_MALLOC_FAILED;
-        return ESP_ERR_NO_MEM;
-    }
-
-    action->type = ACTION_SERVICE_SUBTYPE_ADD;
-    action->data.srv_subtype_add.service = s;
-    action->data.srv_subtype_add.subtype = strdup(subtype);
+    mdns_service_t *srv = s->service;
+    mdns_subtype_t *subtype_item = (mdns_subtype_t *)malloc(sizeof(mdns_subtype_t));
+    ESP_GOTO_ON_FALSE(subtype_item, ESP_ERR_NO_MEM, err, TAG, "Out of memory");
+    subtype_item->subtype = strdup(subtype);
+    ESP_GOTO_ON_FALSE(subtype_item->subtype, ESP_ERR_NO_MEM, err, TAG, "Out of memory");
+    subtype_item->next = srv->subtype;
+    srv->subtype = subtype_item;
 
-    if (!action->data.srv_subtype_add.subtype) {
-        free(action);
-        return ESP_ERR_NO_MEM;
-    }
-    if (xQueueSend(_mdns_server->action_queue, &action, (TickType_t)0) != pdPASS) {
-        free(action->data.srv_subtype_add.subtype);
-        free(action);
-        return ESP_ERR_NO_MEM;
+err:
+    MDNS_SERVICE_UNLOCK();
+    if (ret == ESP_ERR_NO_MEM) {
+        HOOK_MALLOC_FAILED;
     }
-    return ESP_OK;
+    return ret;
 }
 
 esp_err_t mdns_service_instance_name_set_for_host(const char *instance_old, const char *service, const char *proto, const char *hostname,

components/mdns/private_include/mdns_private.h

@@ -188,7 +188,6 @@ typedef enum {
     ACTION_HOSTNAME_SET,
     ACTION_INSTANCE_SET,
     ACTION_SERVICE_INSTANCE_SET,
-    ACTION_SERVICE_SUBTYPE_ADD,
     ACTION_SERVICES_CLEAR,
     ACTION_SEARCH_ADD,
     ACTION_SEARCH_SEND,
@@ -444,10 +443,6 @@ typedef struct {
             mdns_srv_item_t *service;
             char *instance;
         } srv_instance;
-        struct {
-            mdns_srv_item_t *service;
-            char *subtype;
-        } srv_subtype_add;
         struct {
             mdns_search_once_t *search;
         } search_add;