From 9237110c5c4bbe4e3a4c506622d43cc87ab981dd Mon Sep 17 00:00:00 2001 From: Piyush Shah Date: Tue, 11 Sep 2018 16:20:00 +0530 Subject: [PATCH] bugfix: mdns_service_txt_set() wasn't allocating memory for TXT records Allocation was happening later, causing possible use of stack variables of caller function, which could be invalid. Signed-off-by: Piyush Shah --- components/mdns/mdns.c | 35 ++++++++++--------- .../mdns/private_include/mdns_private.h | 3 +- 2 files changed, 20 insertions(+), 18 deletions(-) diff --git a/components/mdns/mdns.c b/components/mdns/mdns.c index ce2f5fa87..986752411 100644 --- a/components/mdns/mdns.c +++ b/components/mdns/mdns.c @@ -1741,6 +1741,17 @@ static mdns_txt_linked_item_t * _mdns_allocate_txt(size_t num_items, mdns_txt_it } return new_txt; } +static void _mdns_free_linked_txt(mdns_txt_linked_item_t *txt) +{ + mdns_txt_linked_item_t *t; + while (txt) { + t = txt; + txt = txt->next; + free((char *)t->value); + free((char *)t->key); + free(t); + } +} /** * @brief creates/allocates new service @@ -3621,14 +3632,8 @@ static void _mdns_execute_action(mdns_action_t * action) service = action->data.srv_txt_replace.service->service; txt = service->txt; service->txt = NULL; - while (txt) { - t = txt; - txt = txt->next; - free((char *)t->value); - free((char *)t->key); - free(t); - } - service->txt = _mdns_allocate_txt(action->data.srv_txt_replace.num_items, action->data.srv_txt_replace.txt); + _mdns_free_linked_txt(txt); + service->txt = action->data.srv_txt_replace.txt; _mdns_announce_all_pcbs(&action->data.srv_txt_replace.service, 1, false); break; @@ -4204,27 +4209,25 @@ esp_err_t mdns_service_txt_set(const char * service, const char * proto, mdns_tx return ESP_ERR_NOT_FOUND; } - mdns_txt_item_t * txt_copy = NULL; + mdns_txt_linked_item_t * new_txt = NULL; if (num_items){ - txt_copy = (mdns_txt_item_t *)malloc(num_items * sizeof(mdns_txt_item_t)); - if (!txt_copy) { + new_txt = _mdns_allocate_txt(num_items, txt); + if (!new_txt) { return ESP_ERR_NO_MEM; } - memcpy(txt_copy, txt, num_items * sizeof(mdns_txt_item_t)); } mdns_action_t * action = (mdns_action_t *)malloc(sizeof(mdns_action_t)); if (!action) { - free(txt_copy); + _mdns_free_linked_txt(new_txt); return ESP_ERR_NO_MEM; } action->type = ACTION_SERVICE_TXT_REPLACE; action->data.srv_txt_replace.service = s; - action->data.srv_txt_replace.num_items = num_items; - action->data.srv_txt_replace.txt = txt_copy; + action->data.srv_txt_replace.txt = new_txt; if (xQueueSend(_mdns_server->action_queue, &action, (portTickType)0) != pdPASS) { - free(txt_copy); + _mdns_free_linked_txt(new_txt); free(action); return ESP_ERR_NO_MEM; } diff --git a/components/mdns/private_include/mdns_private.h b/components/mdns/private_include/mdns_private.h index a3d9fa279..3abaa5661 100644 --- a/components/mdns/private_include/mdns_private.h +++ b/components/mdns/private_include/mdns_private.h @@ -361,8 +361,7 @@ typedef struct { } srv_port; struct { mdns_srv_item_t * service; - uint8_t num_items; - mdns_txt_item_t * txt; + mdns_txt_linked_item_t * txt; } srv_txt_replace; struct { mdns_srv_item_t * service;